make the workload resolver only light up for the specific entrypoint SDKs we control #41268

baronfel · 2024-05-30T14:02:21Z

This is part of #26009

This makes NuGet/Home#13471 easier to diagnose because it removes the workload resolver from the list of resolvers - you can see the before and after here:

before:

D:\temp\test01\test01.csproj : error : Could not resolve SDK "MSTest.Sdk". Exactly one of the probing messages below indicates why we could not resolve the SDK. Investigate and resolve that message to correctly specify the SDK.
D:\temp\test01\test01.csproj : error :   SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" returned null.
D:\temp\test01\test01.csproj : error :   Unable to resolve 'MSTest.Sdk (= 3.3.1)' for '.NETStandard,Version=v0.0'. PackageSourceMapping is enabled, the following source(s) were not considered: Microsoft Visual Studio Offline Packages, NuGet official package source.
D:\temp\test01\test01.csproj : error MSB4236: The SDK 'MSTest.Sdk' specified could not be found.

after:

E:\Code\Scratch\test-app\test-app.csproj : error : Could not resolve SDK "MSTest.Sdk". Exactly one of the probing messages below indicates why we could not resolve the SDK. Investigate and resolve that message to correctly specify the SDK.
E:\Code\Scratch\test-app\test-app.csproj : error :   Unable to resolve 'MSTest.Sdk (= 3.3.1)' for '.NETStandard,Version=v0.0'. PackageSourceMapping is enabled, the following source(s) were not considered: nuget.
E:\Code\Scratch\test-app\test-app.csproj : error MSB4236: The SDK 'MSTest.Sdk/3.3.1' specified could not be found.

We save one line here, and with additional work on the MSBuild side we may be able to reduce the MSBuild resolver wrapper noise in this message.

…SDKs we control

…full framework resolver

baronfel · 2025-03-07T20:26:24Z

src/VSMSBuildExtensions/VSMSBuildExtensions.proj

+      <_VSMSBuildExtensionsContentDestinations Include="@(VSMSBuildExtensionsContent->'%(DestinationPath)')" />
+    </ItemGroup>
+
+    <Copy SourceFiles="@(VSMSBuildExtensionsContent)" DestinationFiles="@(_VSMSBuildExtensionsContentDestinations)" />


This reduces the number of Copy task calls from num_content_files to 1.

src/VSMSBuildExtensions/VSMSBuildExtensions.proj

baronfel · 2025-03-07T20:31:19Z

Here's the content from the generated SWR after this change:

folder "InstallDir:\MSBuild\Current\Bin\SdkResolvers\Microsoft.DotNet.MSBuildSdkResolver\"
  file source="$(PkgVS_Redist_Common_Net_Core_SDK_MSBuildExtensions)\MSBuildSdkResolver\Microsoft.Deployment.DotNet.Releases.dll" vs.file.ngenApplications="[installDir]\Common7\IDE\vsn.exe"
  file source="$(PkgVS_Redist_Common_Net_Core_SDK_MSBuildExtensions)\MSBuildSdkResolver\Microsoft.DotNet.MSBuildSdkResolver.dll" vs.file.ngenApplications="[installDir]\Common7\IDE\vsn.exe"
  file source="$(PkgVS_Redist_Common_Net_Core_SDK_MSBuildExtensions)\MSBuildSdkResolver\Microsoft.DotNet.MSBuildSdkResolver.xml"

and the relevant nuget package layout:

The layout matches the expected file paths for both the dll and the xml, so the resolver xml config should be consumed by MSBuild.exe/VS when injected. The next step would be to somehow inject this into a VS instance to test.

rainersigwald · 2025-03-07T20:51:21Z

The next step would be to somehow inject this into a VS instance to test.

Worst-case is "unzip the .vsix and copy-with-overwrite the content folder over the VS install root, I think.

src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj

src/VSMSBuildExtensions/VSMSBuildExtensions.proj

joeloff · 2025-03-07T21:08:52Z

Compared to the previous .swr, are we running NGEN on anything new? Just asking in case it sets off a DDRIT tripwire.

joeloff · 2025-03-07T21:10:16Z

The next step would be to somehow inject this into a VS instance to test.

Worst-case is "unzip the .vsix and copy-with-overwrite the content folder over the VS install root, I think.

That's easy to do and if we don't test with a proper insertion, we may miss some of the NGEN actions and perf impacts. I can help Chet to do this.

baronfel · 2025-03-07T21:14:34Z

notes:

probably need to change the content of the

Compared to the previous .swr, are we running NGEN on anything new? Just asking in case it sets off a DDRIT tripwire.

no - the net change here should just be a XML file in the VS insertion next to an already-existing dll. That xml file is read by MSBuild itself, after assembly loading has occurred.

baronfel · 2025-03-07T22:10:52Z

Ok, this is looking good in my local testing (laying the xml file down in a VS install root and then using MSbuild.exe):

❯ msbuild.exe /restore /bl /tl
E:\code\scratch\resolver-test\resolver-test.csproj : error :
  Could not resolve SDK "MSTest.Sdk". Exactly one of the probing messages below indicates why we could not resolve the SDK. Investigate and resolve that message to correctly specify the S
  DK.
    The NuGetSdkResolver did not resolve this SDK because there was no version specified in the project or global.json.
    MSB4276: The default SDK resolver failed to resolve SDK "MSTest.Sdk" because directory "C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Sdks\MSTest.Sdk\Sdk" did not ex
  ist.
    E:\code\scratch\resolver-test\resolver-test.csproj : error MSB4236: The SDK 'MSTest.Sdk' specified could not be found.

Restore failed with 2 error(s) in 0.3s

Things to follow up on:

MSBuild's Default SDK Resolver is coming into play here, is there a way to make it not? The .NET SDK resolver does the same thing here? cc @rainersigwald
MSBuild could make an enhancement to its' overall SDK resolution error reporting to remove this wrapper line if there was only 1 SDK resolver error reported.
@joeloff happy to work with you async to try an actual VS test run

rainersigwald · 2025-03-07T23:17:06Z

MSBuild's Default SDK Resolver is coming into play here, is there a way to make it not?

I don't think so today.

nagilson · 2025-04-15T18:01:32Z

Should we be merging this @baronfel?

baronfel · 2025-04-15T18:45:50Z

It would be great to work on this, but we need to schedule some time with @joeloff to do a full-VS run to make sure I didn't break anything. Would be great to partner with an SDK engineer to help push this forward - SDK resolver perf and error message cleanups are a pain point for users.

joeloff · 2025-04-15T18:50:02Z

It would be great to work on this, but we need to schedule some time with @joeloff to do a full-VS run to make sure I didn't break anything. Would be great to partner with an SDK engineer to help push this forward - SDK resolver perf and error message cleanups are a pain point for users.

You mean an insertion to trigger the tests VS runs? You'll need to stage in insertion, or that the very least push your branch to internal, then manually upload the packages to the feed. The latter part may not be possible any longer.

Forgind · 2025-04-15T19:06:42Z

src/VSMSBuildExtensions/VSMSBuildExtensions.proj

    </ItemGroup>

-    <Copy SourceFiles="@(VSMSBuildExtensionsContent)" DestinationFiles="%(VSMSBuildExtensionsContent.DestinationPath)" />
+    <Copy SourceFiles="@(VSMSBuildExtensionsContent)" DestinationFiles="@(VSMSBuildExtensionsContent->'%(DestinationPath)')" />


Why this change?

This coalesces the N Copy Task calls into a single call - this is way faster and the task internally handles up to date checking and such.

So many people unintentionally batch over this Task, we should probably have a build check for it!

Hmm...batching was my first thought, but I thought it would batch in either case. Batching is confusing.

dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels May 30, 2024

baronfel mentioned this pull request May 30, 2024

[Feature Request]: Reduce the wrapping around SDK Resolver messages when only one resolver is actually invoked dotnet/msbuild#10191

Open

baronfel force-pushed the ResolvableSdkPattern-for-workloadresolver branch from 773d911 to 8618076 Compare August 23, 2024 20:20

baronfel requested review from a team, ericstj, joperezr, tmat, arunchndr, AntonLapounov, sharwell, lewing, radical, pavelsavara, maraf and vijayrkn as code owners August 23, 2024 20:20

baronfel changed the base branch from release/8.0.4xx to main August 23, 2024 20:21

baronfel removed request for a team August 23, 2024 20:21

baronfel added 7 commits March 6, 2025 18:13

make the workload resolver only light up for the specific entrypoint …

38a7a62

…SDKs we control

update pattern to account for workload packs from all known workloads

62c28c2

use simpler, broader patterns

cbb1a6b

make sure the netfx resolver xml configs get the updates too

ab7ca61

use pattern that combines both workloads and known-sdk names for the …

6fee29d

…full framework resolver

Update patterns in generated netframework resolver

8cd126f

try to include the generated xml file in the vs extension

aeb2680

baronfel force-pushed the ResolvableSdkPattern-for-workloadresolver branch from afed12e to aeb2680 Compare March 7, 2025 00:13

baronfel added 2 commits March 7, 2025 13:43

fix Copy task batching

b22233a

handle the oddbal sdk resolver xml file

930a84f

baronfel commented Mar 7, 2025

View reviewed changes

src/VSMSBuildExtensions/VSMSBuildExtensions.proj Outdated Show resolved Hide resolved

rainersigwald reviewed Mar 7, 2025

View reviewed changes

apply suggestions

07fb03a

Rewrite relative path in VS-injected resolver xml

c612bc9

baronfel marked this pull request as ready for review March 7, 2025 22:06

baronfel added Area-SdkResolvers DO NOT MERGE and removed Area-Infrastructure labels Mar 8, 2025

Forgind reviewed Apr 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make the workload resolver only light up for the specific entrypoint SDKs we control #41268

make the workload resolver only light up for the specific entrypoint SDKs we control #41268

baronfel commented May 30, 2024 •

edited

Loading

baronfel Mar 7, 2025

baronfel commented Mar 7, 2025

rainersigwald commented Mar 7, 2025

joeloff commented Mar 7, 2025

joeloff commented Mar 7, 2025

baronfel commented Mar 7, 2025

baronfel commented Mar 7, 2025

rainersigwald commented Mar 7, 2025

nagilson commented Apr 15, 2025

baronfel commented Apr 15, 2025

joeloff commented Apr 15, 2025

Forgind Apr 15, 2025

baronfel Apr 16, 2025

Forgind Apr 16, 2025

make the workload resolver only light up for the specific entrypoint SDKs we control #41268

Are you sure you want to change the base?

make the workload resolver only light up for the specific entrypoint SDKs we control #41268

Conversation

baronfel commented May 30, 2024 • edited Loading

baronfel Mar 7, 2025

Choose a reason for hiding this comment

baronfel commented Mar 7, 2025

rainersigwald commented Mar 7, 2025

joeloff commented Mar 7, 2025

joeloff commented Mar 7, 2025

baronfel commented Mar 7, 2025

baronfel commented Mar 7, 2025

rainersigwald commented Mar 7, 2025

nagilson commented Apr 15, 2025

baronfel commented Apr 15, 2025

joeloff commented Apr 15, 2025

Forgind Apr 15, 2025

Choose a reason for hiding this comment

baronfel Apr 16, 2025

Choose a reason for hiding this comment

Forgind Apr 16, 2025

Choose a reason for hiding this comment

baronfel commented May 30, 2024 •

edited

Loading