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

Properly handle empty solution and project paths in SDK resolver #24333

Conversation

jeffkl
Copy link
Contributor

@jeffkl jeffkl commented Mar 11, 2022

Fixes #24320

In some cases, MSBuild project evaluations are in-memory and so no path context is passed to the SDK resolver. Additionally, some evaluations in Visual Studio specify an empty string for the solution file path which causes the SDK resolver to throw an ArgumentException error when passing that to Path.GetDirectoryName().

This change checks for null and empty/whitespace values to know that the solution file path shouldn't be used. Additionally, if an in-memory project is being evaluated and there is no project file path to fall back to, the Environment.CurrentDirectory is used instead as a best effort starting directory for global.json. This could result in the wrong global.json being used but its more likely that no global.json will be found and SDK resolution will work like normal.

Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. If you want this to ship in 17.2, then it needs to be retargeted to the release/6.0.3xx branch.

@jeffkl
Copy link
Contributor Author

jeffkl commented Mar 18, 2022

@drewnoakes do you have a sense of how urgent this fix is? It will pretty much fix the bug reported in dotnet/msbuild#7453, not sure if you were seeing it a lot?

@drewnoakes
Copy link
Member

How hard would it be to retarget this to 17.2? 17.2 is LTS, so it would be good to fix this.

@jeffkl jeffkl changed the base branch from main to release/6.0.3xx March 18, 2022 21:50
@jeffkl jeffkl requested review from donJoseLuis, a team and vijayrkn as code owners March 18, 2022 21:50
@jeffkl jeffkl force-pushed the fix-argument-exception-in-sdkresolver branch from b2664c5 to 2efe146 Compare March 18, 2022 21:53
@jeffkl
Copy link
Contributor Author

jeffkl commented Mar 18, 2022

I have retargeted to release/6.0.3xx for now to see if we can get this for 17.2/6.0.3xx

@jeffkl
Copy link
Contributor Author

jeffkl commented Mar 21, 2022

@dsplaisted Let me know if you want to get this in 6.0.3xx

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

Successfully merging this pull request may close these issues.

MSBuildSdkResolver throws ArgumentException when solution path is an empty string
3 participants