[17.13] Opt-in .sln parsing with Microsoft.VisualStudio.SolutionPersistence
#11487
Conversation
surayya-MS
changed the title
.sln parsing with SolutionPersistence.sln parsing with Microsoft.VisualStudio.SolutionPersistence
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -134,6 +134,8 @@ public Traits() | |||
|
|
|||
| public readonly bool InProcNodeDisabled = Environment.GetEnvironmentVariable("MSBUILDNOINPROCNODE") == "1"; | |||
|
|
|||
| public readonly bool SlnParsingWithSolutionPersistenceOptIn = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILD_SLN_PARSING_SOLUTIONPERSISTENCE_OPTIN")); | |||
There was a problem hiding this comment.
I like the naming style with "_" more than current, it is more readable. However, this is breaking current naming standards for environment variables, from what I see in this file. Let's discuss whether we want to allow two styles from now on. @rainersigwald your opinion?
There was a problem hiding this comment.
Actually, I see two styles already in the file, so this would be 3rd.
There was a problem hiding this comment.
thanks for the commen! i will address this in follow-up PR
There was a problem hiding this comment.
@AR-May , I used the name SlnParsingWithSolutionPersistenceOptIn similar to https://github.com/dotnet/msbuild/pull/11538/files#diff-248a4d8f08fe76eb35f1020d07a8d4300f623e28dba2706090517c8d48692dcaR147
|
For 17.13 we are reverting .slnx support - use the new parser for .sln and .slnx in #11488 We want the changes in this PR for 17.14 |
surayya-MS
changed the title
.sln parsing with Microsoft.VisualStudio.SolutionPersistence.sln parsing with Microsoft.VisualStudio.SolutionPersistence
Require opt-in (instead of -out) to
.slnparsing with the new parser fromMicrosoft.VisualStudio.SolutionPersistence.Fixes #11463
Work item (Internal use): AB#2397817
Summary
Added
MSBUILD_SLN_PARSING_SOLUTIONPERSISTENCE_OPTINas an opt-in environment variable instead of requiring changewave opt-out to return to the MSBuild-internal solution parser.Customer Impact
Three categories of problem:
NuGet.exerestores failed because they couldn't find the library (fixed in newer versions but reported via VS Feedback and Update to msbuild 17.13 breaks builds microsoft/dotnet-framework-docker#1213.NuGet.exerestores can fail if the path to 64-bit MSBuild is specified explicitlyAll manifest as build or NuGet restore breaks with no obvious workaround (but once discovered the changewave opt-out environment variable works).
Regression?
Yes, in 17.13/9.0.200 due to adopting the common SolutionPersistence library instead of our homegrown sln parser.
Testing
Changed tests to set
MSBUILD_SLN_PARSING_SOLUTIONPERSISTENCE_OPTINenvironment variable. Tested manually as well.Risk
Low, returns to 17.12 behavior unless explicitly opted in--basically inverting existing opt-out.