-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
.slnx support - use the new parser for .sln and .slnx #10836
base: main
Are you sure you want to change the base?
Conversation
commit suggestions Co-authored-by: Rainer Sigwald <[email protected]> Co-authored-by: Jan Krivanek <[email protected]>
… Solution_OldParser_Tests
lets see what happens if we now merge main (fingers crossed) :) |
the one failing test |
This should help identify failures like dotnet#10836 (comment).
Diagnosability on that test is quite unfortunate, but hopefully fixable (#10886). |
Co-authored-by: kasperk81 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one also probably needs a standalone insertion to VS due to potential regression risk, and @dotnet/kitten should keep a close eye on optprof data generation/collection since we're adding an assembly load on the msbuild.exe foo.sln
path.
src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs
Outdated
Show resolved
Hide resolved
This should help identify failures like dotnet#10836 (comment).
Co-authored-by: Rainer Sigwald <[email protected]>
Co-authored-by: Rainer Sigwald <[email protected]>
…/msbuild into slnx-support-followup
eng/Versions.props
Outdated
<VersionPrefix>17.14.0</VersionPrefix> | ||
<PackageValidationBaselineVersion>17.12.0</PackageValidationBaselineVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rainersigwald do you know if we should also update PackageValidationBaselineVersion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
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
Microsoft.VisualStudio.SolutionPersistence
parser to parse .slnx and .sln (applied 17.13 change wave for .sln)Testing
Moved tests from
SolutionFile_Tests
that are old parser specific to theSolutionFile_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.