Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove SlnFile references #45442

Merged
merged 27 commits into from
Jan 14, 2025
Merged

Conversation

edvilme
Copy link
Member

@edvilme edvilme commented Dec 12, 2024

SlnFile is being replaced by vs-solutionpersistence as part of the efforts for .slnx support
This should be replaced in all remaining places it might be used

Contributes #40913

@@ -76,8 +76,11 @@ private static (bool isSolution, string workspacePath) FindFile(string workspace
return (isSolution, workspacePath);
}

private static IEnumerable<string> FindSolutionFiles(string basePath) => Directory.EnumerateFileSystemEntries(basePath, "*.sln", SearchOption.TopDirectoryOnly)
.Concat(Directory.EnumerateFileSystemEntries(basePath, "*.slnf", SearchOption.TopDirectoryOnly));
private static IEnumerable<string> FindSolutionFiles(string basePath) => [
Copy link
Contributor

Choose a reason for hiding this comment

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

we can return string[] and use the array all the way to avoid .ToList in the end. it makes an unnecessary copy in FindMatchingFile to find count

@edvilme edvilme requested a review from a team January 8, 2025 22:13
@edvilme edvilme marked this pull request as ready for review January 8, 2025 22:13
@edvilme edvilme requested review from a team as code owners January 8, 2025 22:13
@edvilme edvilme changed the title (Draft) Remove SlnFile Remove SlnFile references Jan 8, 2025
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Lot of deleted code! I guess it was all SlnFile-specific?

I think this at least mostly accomplishes your goal. I feel like we should better support slnf, as that's basically a sln*.

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Signing off for the MSBuildWorkspaceFinder.cs change which is owned by roslyn-ide; the rest is unreviewed.

@edvilme
Copy link
Member Author

edvilme commented Jan 10, 2025

@dotnet/source-build I have removed project Mcrosoft.DotNet.Cli.Sln.Internal but am getting source build errors. Should I be doing an additional step? Thanks!

@ellahathaway
Copy link
Member

@dotnet/source-build I have removed project Mcrosoft.DotNet.Cli.Sln.Internal but am getting source build errors. Should I be doing an additional step? Thanks!

Error is: /__w/1/s/artifacts/sb/src/source-build.slnf(1,1): error MSB4025: The project file could not be loaded. Data at the root level is invalid. Line 1, position 1.

I believe you will have to update source-build.slnf to reflect that you've removed Microsoft.DotNet.Cli.Sln.Internal

@edvilme edvilme enabled auto-merge (squash) January 13, 2025 23:05
@edvilme edvilme merged commit 4795c3e into dotnet:release/9.0.2xx Jan 14, 2025
29 of 31 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants