Add XML Solution (SLNX) support to MSBuildWorkspace #77326
Conversation
ghost
added
Area-IDE
untriaged
|
This should not be necessary, as we just use MSBuild to load solutions, and they've already moved to the new parser. |
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Build" /> | ||
| <PackageReference Include="Microsoft.Build.Tasks.Core" /> |
There was a problem hiding this comment.
I would expect we could remove these Microsoft.Build* package references now.
There was a problem hiding this comment.
I think we'll still have problems with the ILogger API points since we expose those as public APIs. But we should be able to reduce them in a similar way to your other PR.
| public static string Invalid => GetText("SolutionFilters.InvalidSolutionFilter.slnf"); | ||
| public static string CSharp => GetText("SolutionFilters.CSharpSolutionFilter.slnf"); | ||
| } | ||
| public static class XmlSolutions |
There was a problem hiding this comment.
nit: Newline between classes
| public static class XmlSolutions | |
| public static class XmlSolutions |
|
|
||
| <!-- SolutionPersistence --> | ||
| <PropertyGroup> | ||
| <MicrosoftVisualStudioSolutionPersistenceVersion>1.0.28</MicrosoftVisualStudioSolutionPersistenceVersion> |
There was a problem hiding this comment.
Revert changes to this file. I believe this is only being used for package which we consume through Arcade subscriptions.
| return null!; | ||
| } | ||
|
|
||
| var isSolutionFilter = SolutionFilterReader.IsSolutionFilterFilename(absoluteSolutionPath); |
There was a problem hiding this comment.
Nit: Newline after closing brace
| var isSolutionFilter = SolutionFilterReader.IsSolutionFilterFilename(absoluteSolutionPath); | |
| var isSolutionFilter = SolutionFilterReader.IsSolutionFilterFilename(absoluteSolutionPath); |
| } | ||
| catch | ||
| { | ||
|
|
| } | ||
|
|
||
| } | ||
| projectPaths = projects.ToImmutable(); | ||
| return true; | ||
|
|
||
| } |
There was a problem hiding this comment.
nit: Newline after closing branch, no empty lines before closing brace
| } | |
| } | |
| projectPaths = projects.ToImmutable(); | |
| return true; | |
| } | |
| } | |
| } | |
| projectPaths = projects.ToImmutable(); | |
| return true; | |
| } |
| var baseDirectory = Path.GetDirectoryName(filename)!; | ||
| RoslynDebug.AssertNotNull(baseDirectory); | ||
|
|
||
| solutionModel = serializer.OpenAsync(filename, CancellationToken.None).Result; |
There was a problem hiding this comment.
| solutionModel = serializer.OpenAsync(filename, CancellationToken.None).Result; | |
| var solutionModel = serializer.OpenAsync(filename, CancellationToken.None).Result; |
| // Get the serializer for the solution file | ||
| ISolutionSerializer? serializer = SolutionSerializers.GetSerializerByMoniker(filename); | ||
| SolutionModel solutionModel; | ||
| var projects = ImmutableArray.CreateBuilder<string>(); | ||
|
|
||
| if (serializer != null) | ||
| { | ||
| // The base directory for projects is the solution folder. |
There was a problem hiding this comment.
Move the solutionModel declaration down to where it is set. Move the projects declaration inside the if where it is used.
| // Get the serializer for the solution file | |
| ISolutionSerializer? serializer = SolutionSerializers.GetSerializerByMoniker(filename); | |
| SolutionModel solutionModel; | |
| var projects = ImmutableArray.CreateBuilder<string>(); | |
| if (serializer != null) | |
| { | |
| // The base directory for projects is the solution folder. | |
| // Get the serializer for the solution file | |
| ISolutionSerializer? serializer = SolutionSerializers.GetSerializerByMoniker(filename); | |
| if (serializer != null) | |
| { | |
| var projects = ImmutableArray.CreateBuilder<string>(); | |
| // The base directory for projects is the solution folder. |
| ISolutionSerializer ? serializer = SolutionSerializers.GetSerializerByMoniker(filename); | ||
| return serializer == null ? false : true; | ||
| } | ||
| //Rename the filter files and user proper namming convension |
There was a problem hiding this comment.
nit: newline following closing brace
| //Rename the filter files and user proper namming convension | |
| //Rename the filter files and user proper naming convention |
| the generators we build would load on the command line but not load in IDEs. | ||
| --> | ||
| <PackageVersion Include="Microsoft.CodeAnalysis.Common" Version="4.1.0" /> | ||
| <PackageVersion Include="Microsoft.VisualStudio.SolutionPersistence" Version="1.0.28" /> |
There was a problem hiding this comment.
Move this further up. Maybe create a MSBuildWorkspace section beneath the LanguageServer section.
@333fred Using the parsing library directly will not tie slnx support to a particular set of MSBuild tools. It will also allow us to drop the MSBuild dependency from Workspaces.MSBuild. |
If that's the direction you want to go, I don't have an issue with it, but I don't particularly get the motivation, tbh. We're still tied to MSBuild. |
|
@333fred It makes a bit more sense in the context of #76832; for that PR to ensure we better sever the MSBuild usage from the main process, we had to move to a model where we had to launch a build host just to read a solution file, which wasn't ideal from a performance/reliability perspective. I'm was also hoping we can get built-in support too for the slnf which would allow us to drop our current code reinventing that. |
|
Any update? |
6 participants
This PR adds support for the new .slnx (XML Solution) file format in the MSBuild workspace. This allows Visual Studio and other tools to work with solutions defined in this more readable and maintainable format.
Key Changes:
Microsoft.VisualStudio.SolutionPersistencelibrary. This new reader handles the XML structure and extracts project paths.SolutionReaderfor .slnx files, and to correctly handle both .sln and .slnx files. Improved error handling for invalid solution files.VisualStudioMSBuildWorkspaceTeststo verify that .slnx files are correctly loaded, including tests for both valid and invalid .slnx file structures.Microsoft.VisualStudio.SolutionPersistenceNuGet package.