Minimum required sdk version is making dev wf painful

[ViktorHofer]

• This issue is blocking
• This issue is causing unreasonable pain

dotnet/runtime@e0e752a updated the minimum required SDK specified in the repo's global.json file: dotnet/runtime@e0e752a#diff-274660eb4b1b1d963a330b471e10f41cR3.

I don't see any reason why Arcade bumps both the locally aquired sdk's version and the minimum required sdk version. We have a policy in place in dotnet/runtime that forbids us to merge and infrastructure breaking changes outside of our monthly rollouts. These auto-updates break our promise.

cc @mmitche @safern @markwilkie

[markwilkie]

My understanding of the current policy is that all SDK updates go through tactics which concerns can be addressed as required. (I believe this happened in this case as well)

With the goal (principle) that the stack uses a consistent SDK - do you have a recommend @ViktorHofer for what we should do instead?

[ViktorHofer]

With the goal (principle) that the stack uses a consistent SDK

The stack's SDK is the repo local SDK:

arcade/global.json

Line 3 in dcc5ca2

"dotnet": "5.0.100-preview.6.20310.4"

.

do you have a recommend @ViktorHofer for what we should do instead?

Yes, dependency flow should only update the repo local SDK's version but not the minimum required one. Arcade doesn't even define a minimum version in their global.json and dependency flow doesn't add it so I assume that it only updates the value if it exists.

[riarenas]

Maestro does keep them both in sync using Arcade's local version as the source of truth. the scope of the policy at the time was to include the minimum required as well, both for consistency with the repo local, and I believe one additional reason was to get more usage for dogfooding.

If the policy should change, it's a simple code change to stop keeping them both in sync.

[safern]

I think updating the minimum required SDK is painful for contributors and I'm towards not updating it that "often", however, in this case if we haven't had updated it we would not have caught a very concerning MSBuild issue that would've probably slipped through Preview6 final product, so that makes me hesitant and I like that we actually dogfood the SDK as customers would do.

[dougbu]

We prefer the current state for the dotnet/aspnetcore repo. In that repo, we purposefully do not allow the SDK to roll forward and have disabled use of a globally-installed SDK. Please at least give repos the option to maintain the current behaviour.

[ViktorHofer]

Don't get me wrong here, we definitely want to dogfood the latest SDK and we already do that via the repo local one. The issue that Santi mentioned should have been caught by dotnet/installer or Microsoft/msbuild and I suspect a test hole there.

dotnet/runtime allows using a globally installed SDK if it meets the minimum required version. @dougbu why would you want to prevent that?

[dougbu]

why would you want to prevent that?

For consistency our preference remains sticking w/ the repo-local SDK. We're not going to change that.

[ViktorHofer]

I think updating the minimum required SDK is painful for contributors and I'm towards not updating it that "often", however, in this case if we haven't had updated it we would not have caught a very concerning MSBuild issue

In the past dotnet/runtime updated the SDK more often than Arcade (which sticked on 3.1.101 for several months and just recently started dogfooding 5.0 with the TFM change). We will most likely update the minimum required SDK every month with the monthly infrastructure rollout (ie dotnet/runtime#37706). The pain at the moment is that Arcade updates it outside of our rollout window.

We want to be able to opt-out of the existing behavior and have full control over the minimum required SDK version. Example of the opt-out switch:

  "tools": {
    "dotnet": "5.0.100-preview.6.20310.4",
    "updateSdkAttribute": "true"
  },

[mmitche]

I think updating the minimum required SDK is painful for contributors and I'm towards not updating it that "often", however, in this case if we haven't had updated it we would not have caught a very concerning MSBuild issue

In the past dotnet/runtime updated the SDK more often than Arcade (which sticked on 3.1.101 for several months and just recently started dogfooding 5.0 with the TFM change). We will most likely update the minimum required SDK every month with the monthly infrastructure rollout (ie dotnet/runtime#37706). The pain at the moment is that Arcade updates it outside of our rollout window.

We want to be able to opt-out of the existing behavior and have full control over the minimum required SDK version. Example of the opt-out switch:

  "tools": {
    "dotnet": "5.0.100-preview.6.20310.4",
    "updateSdkAttribute": "true"
  },

Doesn't the fact that the tools.dotnet attribute gets updated across the board (the SDK used to build the product) effectively mean that an arcade rollout that updates the dotnet version will bring in a potentially breaking change (e.g. nuget update)?

Is this primarily about dev workflow?

[ViktorHofer]

Doesn't the fact that the tools.dotnet attribute gets updated across the board (the SDK used to build the product) effectively mean that an arcade rollout that updates the dotnet version will bring in a potentially breaking change (e.g. nuget update)?

Not necessarily. As an example, you updated the Arcade SDK a month ago to benefit from improvements made in NuGet: 1a7aa80. For that change alone we would never update the minimum required SDK as there's close to zero impact for developers working on libraries.

Is this primarily about dev workflow?

Yes, the minimum required SDK version determines if a globally installed SDK can be used in the repository. Our community and our devs don't want us bumping the minimum required version up for an artificial reason as it requires them to install a new nightly version.

There are definitely cases where updating the minimum required SDK can't be avoided, e.g. for NuGet or VSTest features that we take a dependency on (ie dotnet test support or NuGet Static Graph restore).

We have helper files like our dotnet.cmd and .sh scripts or the build -vs command that open VS with the repo local SDK but that's insufficient. People still want to use their globally installed SDK (me included) and would appreciate it if the minimum required SDK version isn't bumped randomly.

EDIT: If Arcade takes a dependency on a new SDK feature then the min version should be updated, hence it makes sense for Arcade to define a minimum required version in their global.json as well.

[dougbu]

Backing up a bit, I was wrong in

For consistency our preference remains sticking w/ the repo-local SDK. We're not going to change that.

and similar comments. The perception is the aspnetcore repo is not as approachable as it could be. One step toward approachability would be to support system-wide SDK installations. We are also considering allowing roll-forward so that individual developers could try out newer SDKs, meaning CI builds aren't the main way we'd dogfood those SDKs.

So, as @Pilchie said in a recent meeting, the ideal Arcade option for 5.0 would bump the minimum version just to the latest released version of the SDK. We might not be there yet…

Put another way, opting out of SDK changes entirely seems less important than slowing the cadence down to released versions.

[Pilchie]

I thought the initial plan we had for SDK updates at the beginning of 5.0 was to update once per preview to the last released preview except when a team came to tactics with a need to do an interim update?

Also, didn't this have implications on source build?

[Pilchie]

That seems to be what's written at https://github.com/dotnet/arcade/blob/master/Documentation/Policy/ToolsetPolicy.md#net-sdk--managed-toolset

[ViktorHofer]

I don't think we differentiated between min version and target version when the doc was written.

I still believe that arcade should specify a min version rather than keeping it in sync with the target one.

[markwilkie]

I guess I'm struggling with the business value of having what amounts to a "torn state" between the dev's box and the CI build. The intent of the doc that @Pilchie referenced is deal with issues earlier rather than later in the ship cycle. Speeding up the dev workflow, only to have it get jammed in CI seems unproductive.

Also - having written this out, I have this feeling I'm missing something big and obvious. So.....could somebody help me understand how splitting min vs. target makes sense - assuming we want to "keep up" by using each release (including previews) as they come out.

[Pilchie]

Instead, I'd consider that we have several goals to serve here:

1. Make it as easy as possible for contributors to repos to get started and be successful. To enable this goal, we should use the most widely available SDK that can be installed as easily as possible. Ideally fully released and included in VS. Could (and likely have to) settle for most recently released preview release. No reason to require a local copy of the SDK, if there is a globally installed one, that just breaks workflows like opening VS without setting a bunch of env vars.
2. Make the product secure/trustworthy/etc. This goal suggests building with a Last Known Good SDK, ideally a released one.
3. Make source build easier. In the past, this motivated using the latest public preview as a way to avoid more complicated bootstrapping. I understand this least, so will defer to others for impact herre.
4. Validate the SDK. This one leans to us using the most recent possible SDK, so that if there is a problem with it, we have time to fix it.
5. Consume latest features in the SDK (includes "visible" changes like new TFMs, language features, but also optimizations to make build flow faster, etc).

Goals 1, 2, and 3, all suggest using older, more widely available SDKs. Goals 4, and 5 suggest upgrading as fast as possible.

My personal suggestion would be:

• Stick to public previews whenever possible, keeping in mind that new features may occasionally force us to want to adopt an interim SDK sooner, but do that only with Tactics approval, and do it comprehensively across the stack.
• To satisfy goal 4, we can set up validation builds of all/most/bellweather repos that use the most recently produced SDK to try to get an early read on SDK regressions.

[dougbu]

FYI

No reason to require a local copy of the SDK, if there is a globally installed one

Actually there is a requirement at the moment but we could treat that as a separate Arcade or SDK bug or enhancement. eng/common/tools.ps1 and eng/common/tools.sh have $useInstalledDotNetCli and $use_installed_dotnet_cli properties but it's ignored if the global.json file in the repo lists additional runtimes (as the one in dotnet/aspnetcore does).

@dsplaisted @wli3 is it currently possible to add additional runtimes for a C:\Program Files\dotnet\... SDK installation and have it picked up automatically without writing into that folder❔ For example can we set an environment variable pointing to additional versions of Microsoft.NETCore.App❔ I know it's possible to muck with @(KnownAppHostPack) and @(KnownFrameworkReference) items in msbuild files but that layers hacks if we want to get it working in Arcade.

[markwilkie]

Stick to public previews whenever possible, keeping in mind that new features may occasionally force us to want to adopt an interim SDK sooner, but do that only with Tactics approval, and do it comprehensively across the stack.
To satisfy goal 4, we can set up validation builds of all/most/bellweather repos that use the most recently produced SDK to try to get an early read on SDK regressions.

I agree with this approach Kevin, and I think it's inline with the spirit of the current guidance.

[garath]

Triage: @markwilkie Please update guidance.

[ViktorHofer]

@Pilchie thanks for the detailed response. What I'm missing in your summary is the developer value when updating the SDK.
1a7aa80 as an example bumped the minimum required SDK only for the purpose of improved build times (during packaging) without any local developer inner loop benefit. On the contrary, it forced our developers across the stack to update their globally installed SDK a) to be able to use VS and b) when using the globally installed SDK when interacting with our repositories (mostly for convenience as it is in the PATH).

cc @stephentoub @jkotas

[jkotas]

Stick to public previews whenever possible, keeping in mind that new features may occasionally force us to want to adopt an interim SDK sooner

Do we have a data about how frequent is "occasionally" given our track record? For dotnet/runtime, my feeling is that it would happen way too often to be helpful.

The approchable contributor experience has to be reliable and predictable. I think it is better to say in the contribution guidelines what you have to do to make VS work with SDK version required by the repo; than to explain the situations when it just works vs. when you need the more complex setup.

[markwilkie]

Perhaps the frequency of updates should be revisited in tactics where the goals @Pilchie referenced can be weighed against each other - as several are competing.

For reference, the current guidance is here: https://github.com/dotnet/arcade/blob/master/Documentation/Policy/ToolsetPolicy.md#net-sdk--managed-toolset

[markwilkie]

Changed title to reflect the developer workflow focus. Also, added it to my epic (business priority).

[ViktorHofer]

Every time we update the SDK in Arcade in master we need to watch our for the dependency update PR in dotnet/runtime to then downgrade the SDK upgrade as we don't allow it outside of our monthly infra rollout window. This is completely manual, error prone and annoying. I hope we can come to an agreement soon.

[safern]

Could we consider making in it an opt-out at the DARC subscription level?

[markwilkie]

Do y'all view this as a technical or policy challenge?

[ViktorHofer]

It's definitely a policy challenge. I think we should start defining the minimum required version of the SDK which can be different from the targeting version that we already define. It's not a technical challenge as we already follow that approach in dotnet/runtime and the actual change is a one liner in Arcade's global.json file.

[ViktorHofer]

Given that we depend on the .NET SDK these days in more ways, we actually benefit from the minimum sdk version updates and people got used to monthly updates. Closing as not a priority anymore.