Project SDKs resolution order

[MarcoRossignoli]

I'm doing some tests with the project sdks and I want to override the sdk used specifying the version inside the project file. Reading the documentation https://learn.microsoft.com/en-us/visualstudio/msbuild/how-to-use-project-sdk?view=vs-2022#how-project-sdks-are-resolved I expected that the override should work, but when I try it in VS I get this error

 warning MSB4241: The SDK reference "MSTest.Sdk" version "3.2.1-dev" was resolved to version "8.0.100" instead.  You could be using a different version than expected if you do not update the referenced version to match.
 warning MSB4240: Multiple versions of the same SDK "MSTest.Sdk" cannot be specified. The previously resolved SDK version "8.0.100" from location "...\Contoso.Tests.csproj" will be used and the version "3.2.1-dev" will be ignored.
 warning MSB4240: Multiple versions of the same SDK "MSTest.Sdk" cannot be specified. The previously resolved SDK version "8.0.100" from location "...\Contoso.Tests.csproj" will be used and the version "3.2.1-dev" will be ignored.

custom SDK is installed in C:\Program Files\dotnet\sdk\8.0.100\Sdks\MSTest.Sdk and I've specified <Project Sdk="MSTest.Sdk/3.2.1-dev"> in the project

Is it expected?

I don't see the same issue if I build using dotnet build in the command line and I see the package(3.2.1-dev) correctly restored with nuget.

cc: @rainersigwald @ladipro @Evangelink

[rainersigwald]

I'm doing some tests with the project sdks and I want to override the sdk used specifying the version inside the project file.

This is not something that's expected to work smoothly since each SDK name is expected to be resolved by only one resolver.

Multiple versions of the same SDK "MSTest.Sdk" cannot be specified.

This is accurate. Did you modify the SDK definition while VS was open? If so it may have persisted across the edit. Do you see the same problem in msbuild.exe?

[Evangelink]

This is accurate. Did you modify the SDK definition while VS was open? If so it may have persisted across the edit. Do you see the same problem in msbuild.exe?

dotnet build was good and issue was only appearing in VS which would match what you just wrote.

[MarcoRossignoli]

This is accurate. Did you modify the SDK definition while VS was open? If so it may have persisted across the edit. Do you see the same problem in msbuild.exe?

like @Evangelink said with dotnet build in console I don't see problem

This is not something that's expected to work smoothly since each SDK name is expected to be resolved by only one resolver.

So the only viable solution to support the override is to build a custom resolver? Maybe have one that call the others in the correct expected order(nuget and workload one)?

[ladipro]

This is not something that's expected to work smoothly since each SDK name is expected to be resolved by only one resolver.

I hope I'm not misunderstanding but I believe that the system is designed to handle the case where multiple resolvers resolve the same SDK spec. Resolvers are asked in priority order and the first one returning an SDK wins. The order should be consistent between command line and VS. I can see where it may go wrong with multi-project solutions because the SDK cache would be shared but this doesn't seem to be the case here.

[ladipro]

So in dotnet build, we use the following resolvers:

{Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.WorkloadSdkResolver} {Microsoft.Build.NuGetSdkResolver.NuGetSdkResolver} {Microsoft.Build.BackEnd.SdkResolution.DefaultSdkResolver}

and WorkloadSdkResolver does not claim the SDK so it is resolved from NuGet.

In MSBuild.exe, we use the following resolvers:

{Microsoft.DotNet.MSBuildSdkResolver.DotNetMSBuildSdkResolver} {Microsoft.Build.NuGetSdkResolver.NuGetSdkResolver} {Microsoft.Build.BackEnd.SdkResolution.DefaultSdkResolver}

and DotNetMSBuildSdkResolver claims the SDK by unifying it with the on-disk version 8.0.100.

@dsplaisted is this a bug or do the two behave differently by design?

[ladipro]

Also, and I'm putting it in a separate comment so I will convert it to another issue, the reason why it appeared to work with MSBuild.exe on the command line for me is actually a Terminal Logger bug. Warnings thrown during evaluation are silently swallowed.

[dsplaisted]

is this a bug or do the two behave differently by design?

This is by design. When running in MSBuild.exe to resolve the SDKs from the .NET SDK we have to first figure out what version of the .NET SDK should be used (using global.json if necessary), and find that SDK. Then we look in the Sdks subfolder of that SDK. The DotNetMSBuildSdkResolver handles this.

When we're running in the .NET CLI (dotnet), we are already running in the context of a given SDK. Any global.json resolution has already happened to load the correct version of the CLI. So the DefaultSdkResolver, which just looks in the Sdks subfolder of the MSBuild tools directory, is sufficient to resolve these SDKs.

Note that in order to minimize DLL loads in Visual Studio, the DotNetMSBuildSdkResolver also has the workload SDK resolution logic that is included in the WorkloadSdkResolver for the .NET CLI.

I don't remember myself, but probably as Rainer says the design was that each SDK would only be resolved by a single resolver, so the order shouldn't matter.

It's probably best to use or create a different hook to replace the project SDK for testing. For this scenario, it would be nice if the DotNetMSBuildSdkResolver would ignore SDK imports that have a version specified. But it's hard to know if that would break people somehow, and I don't think there's even a good mechanism that would let you opt out of the new behavior.

[MarcoRossignoli]

It's probably best to use or create a different hook to replace the project SDK for testing.

@dsplaisted do you mean a custom resolver? Reading the doc looks like if we use the ResolvableSdkPattern(https://github.com/dotnet/msbuild/blob/main/documentation/specs/sdk-resolvers-algorithm.md) it should take the precedence, we could have something for our MSTest sdk where we apply our ordering.

[dsplaisted]

I was thinking of something like an environment variable that you could specify. There already are variables where you can redirect where the SDKs are located, but you would probably need to also have all the other SDKs next to yours. Another route would be to have your Sdk.props and Sdk.targets files be simple shells that import the real props/targets from either the same folder, or from the path specified in a special property / environment variable if it is defined.

[ladipro]

I don't remember myself, but probably as Rainer says the design was that each SDK would only be resolved by a single resolver, so the order shouldn't matter.

So even this user detectable difference aside, the order has an impact on perf. Basically by having regular in-box SDKs resolved by the last resolver in the chain, we spend some non-trivial time loading up the NuGet machinery. I really think dotnet build should be optimized for the common case and do a search in <Version>\Sdks first.

[ladipro]

Making a simple measurement with 8.0.100, it looks like WorkloadSdkResolver and NuGetSdkResolver combined take up >20% of evaluation CPU just to return "I don't know what Microsoft.NET.Sdk is".

[MarcoRossignoli]

I was thinking of something like an environment variable that you could specify. There already are variables where you can redirect where the SDKs are located, but you would probably need to also have all the other SDKs next to yours. Another route would be to have your Sdk.props and Sdk.targets files be simple shells that import the real props/targets from either the same folder, or from the path specified in a special property / environment variable if it is defined.

@dsplaisted maybe the shell could be a good compromise but I don't know if can work well for users, let me explain what we need here. Historically we had a huge problem to move users to the new versions of our testing infra because references are in csproj and users don't always use tools like dependabot or they don't want to bump at all. We'd like to revert this behavior and "force" the default policy that every time you bump the sdk you get the last tooling with all the new features, performance and sometimes also "breaking" changes.

For this reason have a built-in sdk is a good solution, users won't have to specify nothing in their csproj and we can take full control over our dependencies giving us great flexibilty.

What's needed anyway is a way for the user to "force" an old version in case for some reason something went wrong and we broke them. For this reason the idea to have the resolution order like [user project, built-in sdk] was good, we can add code analyzers and inform users about possible breaking changes, fixes etc...

So it's fine to me the shell idea but it means that we need to give them some prop like <MSTestVersion>somever</MSTestVersion> and we need at built time to "download" the nuget and import manually, is it possible from the "shell"? Build a custom resolver that combo the available ones can be also doable as last resort, if we can find something easier I'm all ears.

[dsplaisted]

We'd like to revert this behavior and "force" the default policy that every time you bump the sdk you get the last tooling with all the new features, performance and sometimes also "breaking" changes.

That sounds great.

Do you need to use a project SDK at all for this though? You could just have the built-in targets automatically imported when a property such as UseMSTest or something is set. To support older versions, you could continue to use the NuGet package references.

I think you do need to support referencing older versions of the MSTest SDK you already shipped, but once you switch to version with the .NET SDK, I don't know if you need to support rolling back to those newer versions separately from the SDK itself. It would make things simpler if you didn't have to support that.

[MarcoRossignoli]

You could just have the built-in targets automatically imported when a property such as UseMSTest or something is set.

Can you clarify what you mean?