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

Detect Corrupt Solution File in Dotnet Sln Add #28811

Merged
merged 3 commits into from
Jan 12, 2023

Conversation

nagilson
Copy link
Member

Resolves #28155 (comment), see for more context.

A solution file can contain solution folders which aren't actually on disk and are just a solution file construct. These folders are written via child -> parent hashtable mappings in a preSolution configuration. During manual editing, merge conflicts, and or external editors, these mappings may become invalid. At the same time, solution project folders may mistakenly not include EndProject tags. Visual Studio accounts for these invalid files and lets the user get away with it, and upon saving can repair the solution file. For us and Mono, we more strictly enforce the file format and this adds an error to describe what's wrong with the file and how to fix it.

Before:

~/programming/Umbraco-CMS> dotnet sln add src/Umbraco.Cms.Persistence.PostgreSQL/Umbraco.Cms.Persistence.PostgreSQL.csproj
Unhandled exception: System.InvalidOperationException: Sequence contains no elements
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
   at Microsoft.DotNet.Tools.Common.SlnFileExtensions.GetSolutionFolderPaths(SlnFile slnFile, SlnPropertySet nestedProjects)
   at Microsoft.DotNet.Tools.Common.SlnFileExtensions.AddSolutionFolders(SlnFile slnFile, SlnProject slnProject, IList`1 solutionFolders)
   at Microsoft.DotNet.Tools.Common.SlnFileExtensions.AddProject(SlnFile slnFile, String fullProjectPath, IList`1 solutionFolders)
   at Microsoft.DotNet.Tools.Sln.Add.AddProjectToSolutionCommand.Execute()
   at Microsoft.DotNet.Cli.SlnAddParser.<>c.<ConstructCommand>b__5_0(ParseResult parseResult)
   at Microsoft.DotNet.Cli.ParseResultCommandHandler.Invoke(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass4_0.<<BuildInvocationChain>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass12_0.<<UseHelp>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c.<<UseSuggestDirective>b__18_0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass16_0.<<UseParseDirective>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass8_0.<<UseExceptionHandler>b__0>d.MoveNext()

After:
image

src/Cli/dotnet/SlnFileExtensions.cs Outdated Show resolved Hide resolved
src/Cli/dotnet/CommonLocalizableStrings.resx Outdated Show resolved Hide resolved
@danmoseley
Copy link
Member

Just curious, MSBuild has its own solution parser code, is there any relationship/sharing?

@nagilson
Copy link
Member Author

Just curious, MSBuild has its own solution parser code, is there any relationship/sharing?

I don't think this code is shared by MSBuild, I couldn't find something like it. And I think this code is basically doing what Mono does. That's a good question though.

cc @rainersigwald to potentially add his thoughts.

@kasperk81
Copy link
Contributor

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Just curious, MSBuild has its own solution parser code, is there any relationship/sharing?

I don't think this code is shared by MSBuild, I couldn't find something like it. And I think this code is basically doing what Mono does. That's a good question though.

This is how MSBuild tolerates projects malformed in this specific way:

https://github.com/dotnet/msbuild/blob/3777dcaf7edb3e86a070037ba53e742dd1872873/src/Build/Construction/Solution/SolutionFile.cs#L849-L857

The MSBuild SolutionFile object is read-only so probably not useful as-is to the CLI, since you'll want to mutate the solution.

It is of course unfortunate that we have this duplication all over everything. I'd really like a public, open-source solution API from the Visual Studio team, but that's low on their priority list at the moment.

src/Cli/dotnet/CommonLocalizableStrings.resx Outdated Show resolved Hide resolved
@build-analysis build-analysis bot mentioned this pull request Nov 4, 2022
2 tasks
@nagilson
Copy link
Member Author

@baronfel PTAL at the error again when available, Marc wanted your approval before signing off on this

Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

Sorry aout the delay! Message reads well and is actionable 👍

@nagilson nagilson merged commit 52b9653 into dotnet:release/7.0.2xx Jan 12, 2023
@nagilson
Copy link
Member Author

nagilson commented Dec 2, 2024

It is of course unfortunate that we have this duplication all over everything. I'd really like a public, open-source solution API from the Visual Studio team, but that's low on their priority list at the moment.

Hey it actually happened :)

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.

6 participants