.slnx support - use the new parser for .sln and .slnx

[surayya-MS]

Context

The .slnx support PR is split in 2 - 1. use new parser for .slnx, 2. use new parser for both .sln (under change wave) and .slnx. This is the follow-up PR.

Changes Made

• Use Microsoft.VisualStudio.SolutionPersistence parser to parse .slnx and .sln (applied 17.13 change wave for .sln)
• Throw ArgumentException if solutionFile is null or empty (applied 17.13 change wave)

Testing

• Moved tests from SolutionFile_Tests that are old parser specific to the SolutionFile_OldParser_Tests.

• Some of them already had duplicates there: BadVersionStamp, VersionTooLow, ParseSolutionFileWithDescriptionInformation, MissingNestedProject, ParseInvalidSolutionConfigurations1, ParseInvalidSolutionConfigurations2, ParseInvalidSolutionConfigurations3.

• Deleted SolutionFile_Tests.SharedProjects test because it just checks dependencies and we already have test for that. It also has "GlobalSection(SharedMSBuildProjectFiles)" section but we do not parse it in the old parser.

• Changed SolutionProjectGeneratorTests - refactored strings and used both old and new parsers in the tests (except where only the old one can be used, made a remark there about it)

• Deleted GraphConstructionShouldThrowOnMissingSolutionDependencies test because it checks specifics of how solution projects' dependencies were parsed. "SolutionParseProjectDepNotFoundError" is thrown when the project dependency does not exist. And this is handled differently in the new parser - the dependency is not added at all if it did not exist in solution.

[kasperk81]

System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.VisualStudio.SolutionPersistence, Version=1.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified. File name: 'Microsoft.VisualStudio.SolutionPersistence, Version=1.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'

is it about adding <PackageReference Include="Microsoft.VisualStudio.SolutionPersistence" /> here: https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj#L20 ?

[surayya-MS]

System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.VisualStudio.SolutionPersistence, Version=1.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified. File name: 'Microsoft.VisualStudio.SolutionPersistence, Version=1.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'

is it about adding <PackageReference Include="Microsoft.VisualStudio.SolutionPersistence" /> here:

https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj#L20

?

the message says so, but that is strange. because on my last commit the graph tests were failing because of .sln being parsed by the new parser, hence the package was found. @rainersigwald do you know what might be a problem here?

[kasperk81]

i just searched how the other dependencies are set up https://github.com/search?q=repo%3Adotnet%2Fmsbuild%20System.Configuration.ConfigurationManager&type=code maybe feature switch pulled something which made the fragile dependency graph in torn state, which means it needs to be specified explicitly to make it work in all cases.

[surayya-MS]

i just searched how the other dependencies are set up https://github.com/search?q=repo%3Adotnet%2Fmsbuild%20System.Configuration.ConfigurationManager&type=code maybe feature switch pulled something which made the fragile dependency graph in torn state, which means it needs to be specified explicitly to make it work in all cases.

maybe, let's try

[kasperk81]

entry maybe needed here as well https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/eng/Packages.props#L21

[surayya-MS]

entry maybe needed here as well

https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/eng/Packages.props#L21

we already have version here in eng/dependabot/Packages.props. dependabot updates them periodically. eng/Packages.props is for Darc/Maestro https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/eng/dependabot/Packages.props#L64

[kasperk81]

ah dependabot one is included in eng/Packages.props. the other two diffs are:

https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/src/MSBuild/MSBuild.csproj#L178 https://github.com/dotnet/msbuild/blob/b8f46eb171f3b1c7e3bddb8afaff9fbb0ba59e9d/src/Utilities/Microsoft.Build.Utilities.csproj#L27 (missing persistence package ref)

[kasperk81]

@YuliiaKovalova do you know how to add new dependency in bootstrap? i read https://github.com/dotnet/msbuild/blob/5881e053656bc8aaa890515e5d4318af39f49dc5/documentation/wiki/Bootstrap.md and tried adding Microsoft.VisualStudio.SolutionPersistence around here https://github.com/dotnet/msbuild/blob/5881e053656bc8aaa890515e5d4318af39f49dc5/eng/BootStrapMsBuild.targets#L37 but the error doesn't go away. it's about copying that dll somewhere/somehow to that one place where bootstrap script is looking for. deleting this block fix the error https://github.com/dotnet/msbuild/blob/5881e053656bc8aaa890515e5d4318af39f49dc5/eng/cibuild_bootstrapped_msbuild.sh#L59-L68 (but that's not what we want ik, just to understand where the error is coming from)

[kasperk81]

lets see what happens if we now merge main (fingers crossed) :)

[kasperk81]

the one failing test Microsoft.Build.BuildCheck.UnitTests.TaskInvocationCheckDataTests.ReportsComplexTaskParameters seems unrelated (it also failed in https://github.com/dotnet/msbuild/pull/10813)

[rainersigwald]

Diagnosability on that test is quite unfortunate, but hopefully fixable (#10886).