Skip to content

Ensure sln load uses project absolute paths #78772

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 2, 2025
Merged

Ensure sln load uses project absolute paths #78772

merged 2 commits into from
Jun 2, 2025

Conversation

dibarbet
Copy link
Member

@dibarbet dibarbet commented May 30, 2025 •
edited
Loading

fixes issue found by integration tests in dotnet/vscode-csharp#8331

The new solution parser API gives us relative paths, but contracts in the project system require absolute paths

2025-05-29 20:42:14.888 [info] [Error - 8:42:14 PM] [solution/open] [LSP] System.InvalidOperationException: Unexpected false - file LanguageServerProjectLoader.cs line 195
   at Microsoft.CodeAnalysis.Contract.Fail(String message, Int32 lineNumber, String filePath) in /_/src/Dependencies/Contracts/Contract.cs:line 161
   at Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.LanguageServerProjectLoader.ReloadProjectAsync(ProjectToLoad projectToLoad, ToastErrorReporter toastErrorReporter, BuildHostProcessManager buildHostProcessManager, CancellationToken cancellationToken) in /_/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs:line 195
   at Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.LanguageServerProjectLoader.<>c.<<ReloadProjectsAsync>b__16_0>d.MoveNext() in /_/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs:line 158
--- End of stack trace from previous location ---
   at Microsoft.CodeAnalysis.Shared.Utilities.ProducerConsumer`1.<>c__DisplayClass11_0`3.<<RunParallelChannelAsync>b__2>d.MoveNext() in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ProducerConsumer.cs:line 256
--- End of stack trace from previous location ---
   at System.Threading.Tasks.Parallel.<>c__57`1.<<ForEachAsync>b__57_0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.CodeAnalysis.Shared.Utilities.ProducerConsumer`1.<>c__14`2.<<RunChannelAsync>b__14_3>d.MoveNext() in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ProducerConsumer.cs:line 385
--- End of stack trace from previous location ---
   at Microsoft.CodeAnalysis.Shared.Utilities.ProducerConsumer`1.PerformActionAndCloseWriterAsync[TArgs](Func`3 action, TArgs args, ChannelWriter`1 writer, CancellationToken cancellationToken) in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ProducerConsumer.cs:line 402
   at Microsoft.CodeAnalysis.Shared.Utilities.ProducerConsumer`1.RunChannelAsync[TArgs,TResult](ProducerConsumerOptions options, Func`4 produceItems, Func`4 consumeItems, TArgs args, CancellationToken cancellationToken) in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ProducerConsumer.cs:line 362
   at Microsoft.CodeAnalysis.Shared.Utilities.ProducerConsumer`1.RunParallelAsync[TSource,TArgs](IAsyncEnumerable`1 source, Func`5 produceItems, TArgs args, CancellationToken cancellationToken) in /_/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ProducerConsumer.cs:line 229
   at Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.LanguageServerProjectLoader.ReloadProjectsAsync(ImmutableSegmentedList`1 projectPathsToLoadOrReload, CancellationToken cancellationToken) in /_/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs:line 153
   at Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.LanguageServerProjectLoader.ReloadProjectsAsync(ImmutableSegmentedList`1 projectPathsToLoadOrReload, CancellationToken cancellationToken) in /_/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs:line 179
   at Microsoft.CodeAnalysis.Threading.AsyncBatchingWorkQueue`1.<>c__DisplayClass2_0.<<Convert>b__0>d.MoveNext() in /_/src/Dependencies/Threading/AsyncBatchingWorkQueue`1.cs:line 40
--- End of stack trace from previous location ---
   at Microsoft.CodeAnalysis.Threading.AsyncBatchingWorkQueue`2.ProcessNextBatchAsync() in /_/src/Dependencies/Threading/AsyncBatchingWorkQueue`2.cs:line 274
   at Microsoft.CodeAnalysis.Threading.AsyncBatchingWorkQueue`2.<AddWork>g__ContinueAfterDelayAsync|16_1(Task lastTask) in /_/src/Dependencies/Threading/AsyncBatchingWorkQueue`2.cs:line 221
   at Microsoft.CodeAnalysis.Threading.AsyncBatchingWorkQueue`2.WaitUntilCurrentBatchCompletesAsync() in /_/src/Dependencies/Threading/AsyncBatchingWorkQueue`2.cs:line 238
   at Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.LanguageServerProjectSystem.OpenSolutionAsync(String solutionFilePath) in /_/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectSystem.cs:line 65
   at Microsoft.CommonLanguageServerProtocol.Framework.QueueItem`1.StartRequestAsync[TRequest,TResponse](TRequest request, TRequestContext context, IMethodHandler handler, String language, CancellationToken cancellationToken) in /_/src/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/QueueItem.cs:line 203

@dibarbet dibarbet requested a review from a team as a code owner May 30, 2025 20:22
@dibarbet dibarbet mentioned this pull request May 30, 2025
Closed
@jasonmalinowski jasonmalinowski mentioned this pull request May 30, 2025
Merged
@dibarbet dibarbet requested a review from jasonmalinowski May 31, 2025 00:05
@Crashdummyy Crashdummyy mentioned this pull request Jun 1, 2025
Closed
jasonmalinowski
jasonmalinowski approved these changes Jun 2, 2025
View reviewed changes
...ageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectSystem.cs
@@ -57,7 +57,7 @@ public async Task OpenSolutionAsync(string solutionFilePath)
_logger.LogInformation(string.Format(LanguageServerResources.Loading_0, solutionFilePath));
ProjectFactory.SolutionPath = solutionFilePath;

var (_, projects) = await SolutionFileReader.ReadSolutionFileAsync(solutionFilePath, CancellationToken.None);
var (_, projects) = await SolutionFileReader.ReadSolutionFileAsync(solutionFilePath, DiagnosticReportingMode.Throw, CancellationToken.None);
Copy link
Member

Choose a reason for hiding this comment

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

Do we want something else here or are we going to throw exceptions and not load a solution that we could mostly load? (this question is not blocking considering the regression you're fixing here)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this generally matches prior behavior, but I'll do some tests to see if we can relax it and load slns with potentially invalid projects. Will followup.

src/Workspaces/MSBuild/Test/VisualStudioMSBuildWorkspaceTests.cs
@@ -1002,13 +1002,11 @@ public async Task TestOpenSolution_WithLockedFile_LoadsWithEmptyText()
public async Task TestOpenSolution_WithInvalidProjectPath_SkipTrue_SucceedsWithFailureEvent()
{
// when skipped we should see a diagnostic for the invalid project
Copy link
Member

Choose a reason for hiding this comment

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

So the test was never testing what it claimed to test?

Copy link
Member Author

Choose a reason for hiding this comment

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

It kind of was. But it was setting the wrong flag to allow it to create the diagnostic

@dibarbet dibarbet merged commit 729bd2f into dotnet:main Jun 2, 2025
25 checks passed
@dibarbet dibarbet deleted the fix_project_load branch June 2, 2025 19:08
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jun 2, 2025
This was referenced Jun 10, 2025
Closed
Closed
@dotnet-bot dotnet-bot mentioned this pull request Jun 17, 2025
Closed
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonmalinowski jasonmalinowski jasonmalinowski approved these changes

@RikkiGibson RikkiGibson RikkiGibson approved these changes

Assignees
No one assigned
Labels
Area-IDE VSCode
Projects
None yet
Milestone
18.0 P1
Development

Successfully merging this pull request may close these issues.
3 participants
@dibarbet @jasonmalinowski @RikkiGibson