remove .NET SDK version from global.json

[zivkan]

Bug

Fixes: infrastructure

Description

VS often warns me I'm missing the .NET 9.0 SDK, I'm guessing because it sees global.json, even though I have the .NET 10 SDK installed and the product builds fine. Also, our internal mirror is getting dependabot updates for global.json even though our build doesn't care what version of the SDK is used.

Our build used to work this way (without the SDK version in the global.json) before https://github.com/NuGet/NuGet.Client/pull/5764. That PR added the SDK version into global.json, but our build scripts install a new SDK, so I don't feel like global.json is a good way to enforce a minimum SDK version given all the undesirable side effects it causes.

I validated that the dotnet VMR builds correctly with this change: https://github.com/dotnet/dotnet/pull/3105

PR Checklist

• [x] Meaningful title, helpful description ~and a linked NuGet/Home issue~
• [x] ~Added tests~ N/A
• [x] ~Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.~ N/A

[akoeplinger]

hm we actually are going the opposite way of making sure every global.json has an sdk:version key since otherwise Component Governance misdetects the actually used sdk version and raises S360 items...

[nkolev92]

I feel like the CG misdetection needs to be addressed regardless of what we do here. Right now, it validates what you're enforcing, rather than what you're using.

[akoeplinger]

@nkolev92 the way the detection works is that it basically runs dotnet --version next to global.json and reports whatever that returns. Some 1ES steps inject a dotnet into PATH and so that's what will be reported even though the arcade build scripts use the one from "tools { dotnet: XXX }" instead. There's currently no other way for them to determine which dotnet was actually used to build since that isn't recorded anywhere.

shouldn't we rather fix VS to not report a warning in this case? we set rollForward and allowPrerelease after all

[zivkan]

hm we actually are going the opposite way of making sure every global.json has an sdk:version key since otherwise Component Governance misdetects the actually used sdk version and raises S360 items...

NuGet solves this by having our configure script use the dotnet-install script with this argument: https://github.com/NuGet/NuGet.Client/blob/875a604c20baad06c4dcd728dee552f7da4b6550/build/DotNetSdkVersions.txt#L2

since it's asking for the latest build of a particular channel, it will always get the latest security fix, without needing ongoing updates to global.json. Therefore CG running dotnet --version will be fine. Personally, I feel like this is a better solution not only for the CG issue, but also Dependabot and other internal security tools incorrectly flagging our internal mirror as out of compliance or creating pull requests on branches we will never merge.

However, for some reason with my changes pack isn't finding some loc satellite assemblies, and I can't figure out why. I can't reproduce the error locally in WSL2. Comparing binlogs, I can see there's quite a lot of loc related stuff that is happening differently locally vs on CI, but I can't figure out why. I see the satellite assembly is compiled in the obj/ directory, but it looks like it might not be getting copied to the bin/ directory. It's also weird because our official builds don't pack the satellite assemblies, so I'm not sure why source build is. If anyone can help me investigate, that'd be greatly appreciated.

[akoeplinger]

NuGet solves this by having our configure script use the dotnet-install script with this argument:

ok but we'd need to make the same change in arcade for it to have an effect when building in the VMR

[nkolev92]

Aren't most of the arcade projects just using a local SDK that CG can't even figure out? https://github.com/dotnet/sdk/pull/49627

[zivkan]

ok but we'd need to make the same change in arcade for it to have an effect when building in the VMR

Just getting back from vacation and catching up on notifications.

The VMR already has a global.json in its root: https://github.com/dotnet/dotnet/blob/main/global.json

Will that be used when the src/nuget.client directory's global.json no longer has an sdk version?

and as the PR description said, I tested a PR in the VMR to ensure that the VMR build still works: https://github.com/dotnet/dotnet/pull/3105

So, I tried to do basic due diligence to make sure I won't break the VMR with this change.

If this PR does cause a problem in the VMR we can revert quickly. But I still think that removing the SDK from global.json gives us the best Developer Experience (DX) and least compliance issues.

[ViktorHofer]

With all due respect, I think this change is not an improvement. There are numerous situations when you want to enforce a minimum required version of the .NET SDK to build a given component / repository. The above linked PR from the top post is a great example. The native sourcelink integration of the SDK was depended on in that PR and that required enforcing a minimum SDK version. Without that, devs building the repository with a not recent enough SDK would produce an incorrect product or in the best case run into build warnings / errors.

Removing the option to enforce a minimum SDK version won't work in your (NuGet devs') favor.

While you already pass in a specific version from YML or the repo build scripts, that doesn't help with independent dotnet invocations, i.e. when building inside Visual Studio or just doing a dotnet X on the command-line.

I think there are better options to mitigate the UX issues that you called out (dependabot / Visual Studio). IIRC @ericstj worked with the Dependabot team on the feature that flags out-of-support SDKs. He might be able to share a better solution or return feedback to the responsible team to refine dependabot's detection mechanism.

Regarding Visual Studio, I'm surprised that it warns you that you are missing the 9.0.3xx SDK if you have a newer one installed. That shouldn't be the case and that sounds like VS bug that would be great to get fixed.

[ericstj]

Adding to global.json is about stating the minimum required. You should be using it for the same reasons you recommend folks use NuGet PackageReferences with Versions.

For folks like us dotnet teams which acquire it locally and don't put it on the path, we can explicitly state our requirement in global.json to ensure that CG falls back to that. There was a bug in CG where it was running in host instead of container, which is being fixed -- that had caused some misdetection of SDK in the absence of global.json. At a limit these will be fixed and shouldn't impact decisions here.

Built-in acquisition tools like UseDotNet read the value from global.json and ensure it's installed. IMO the happy path to avoiding compliance issues is to state it in global.json and let Dependabot update it. That gives enforcement (both for official builds and contributors) and acquisition.

-Channel 9.0.3xx since it's asking for the latest build of a particular channel

This is like using * versions. I wouldn't recommend it if you care about determinism. We try very hard to avoid breaking changes, but I still don't suggest that folks float their official build infrastructure like this. You also need to "remember" to update it for major versions/bands, whereas if you can have dependabot do that automatically.

shouldn't we rather fix VS to not report a warning in this case? we set rollForward and allowPrerelease after all

Yeah, this stuck out to me too. If some VS tool is not honoring the rollForward setting we need to get that fixed. For better or worse global.json is the tool we ship to customers for specifying minimum SDK version, and if we are not happy with the developer experience of that we should be fixing it rather than doing some custom thing.

[dotnet-policy-service[bot]]

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 7 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.