Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider specifying ResolvableSdkPattern in MSBuild sdk resolvers' manifests. #26009

Open
AR-May opened this issue Jun 14, 2022 · 2 comments
Open

Comments

@AR-May
Copy link
Member

AR-May commented Jun 14, 2022

Is your feature request related to a problem? Please describe.

This feature improves performance. MSBuild would skip loading additional assemblies with sdk resolvers when they are not needed.

Describe the solution you'd like

After merging this msbuild PR it is possible to add <ResolvableSdkPattern> to manifests of msbuild sdk resolvers. Consider to use it for resolvers Microsoft.DotNet.MSBuildSdkResolver and Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver which live in this repo.

Note that further we would need to keep the pattern up to date. Suggested pattern is
Microsoft\.NET.* | <some exceptional cases>

Additional context

See this page for more details about sdk resolution and the recent change of the algorithm in MSBuild.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-SdkResolvers untriaged Request triage from a team member labels Jun 14, 2022
@marcpopMSFT marcpopMSFT self-assigned this Jun 29, 2022
@marcpopMSFT marcpopMSFT added this to the Backlog milestone Jul 5, 2022
@marcpopMSFT marcpopMSFT removed the untriaged Request triage from a team member label Jul 5, 2022
@marcpopMSFT marcpopMSFT removed their assignment Jul 5, 2022
@marcpopMSFT marcpopMSFT modified the milestones: Backlog, 8.0.1xx Jul 27, 2022
@marcpopMSFT marcpopMSFT modified the milestones: 8.0.1xx, Backlog Sep 15, 2023
@baronfel
Copy link
Member

We should do this ASAP because it massively improves the user experience for SDK resolution in scenarios like NuGet/Home#13471 - adding this data would effectively remove the workload and 'normal' SDK resolvers from the list of resolvers used for NuGet SDK resolution, leaving only the most likely error messages.

@baronfel
Copy link
Member

Specifically:

  • for the Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver we need to add the line <ResolvableSdkPattern>Microsoft.NET.SDK.WorkloadAutoImportPropsLocator|Microsoft.NET.SDK.WorkloadManifestTargetsLocator</ResolvableSdkPattern> to the existing xml file.
  • for the Microsoft.DotNet.MSBuildSdkResolver we don't seem to create such an xml file and the story is a bit more muddled. This resolver seems to orchestrate a few other resolvers? So we'd need to create a regex with the superset of all of the expected names for the 'nested' resolvers (which right now looks like the workload resolver and the 'normal' SDK path resolver?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants