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

With static graph enabled, relative path isn't resolved correctly #5898

Closed
ViktorHofer opened this issue Nov 19, 2020 · 11 comments · Fixed by #7361
Closed

With static graph enabled, relative path isn't resolved correctly #5898

ViktorHofer opened this issue Nov 19, 2020 · 11 comments · Fixed by #7361
Assignees
Labels
Area: Static Graph Issues with -graph, -isolate, and the related APIs. bug Partner request triaged
Milestone

Comments

@ViktorHofer
Copy link
Member

PS C:\git\runtime3> dotnet build src\libraries\System.Runtime\ref\System.Runtime.csproj /bl
Microsoft (R) Build Engine version 16.8.0+126527ff1 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

Some command line switches were read from the auto-response file "MSBuild.rsp". To disable this file, use the "-noAutoResponse" switch.
C:\Program Files\dotnet\sdk\5.0.100\MSBuild.dll -consoleloggerparameters:Summary -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,C:\Program Files\dotnet\sdk\5.0.100\dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,C:\Program Files\dotnet\sdk\5.0.100\dotnet.dll -graph -maxcpucount -restore -verbosity:m /bl src\libraries\System.Runtime\ref\System.Runtime.csproj
  Determining projects to restore...
  All projects are up-to-date for restore.
C:\git\runtime3\src\libraries\System.Runtime\ref\src\libraries\System.Runtime\ref\System.Runtime.csproj : error MSB4025: The project file could not be loaded. Could not find a part of the path 'C:\git\runtime3\src\libraries\System.Runtime\ref\src\libraries\System.Runtime\ref\System.Runtime.csproj'.

Build FAILED.

C:\git\runtime3\src\libraries\System.Runtime\ref\src\libraries\System.Runtime\ref\System.Runtime.csproj : error MSB4025: The project file could not be loaded. Could not find a part of the path 'C:\git\runtime3\src\libraries\System.Runtime\ref\src\libraries\System.Runtime\ref\System.Runtime.csproj'.
    0 Warning(s)
    1 Error(s)

Using the 5.0.100 SDK. Looks like msbuild concatenates the relative path passed in with the path that resolves to the project file.

cc @rainersigwald @cdmihai

@ViktorHofer ViktorHofer added bug needs-triage Have yet to determine what bucket this goes in. labels Nov 19, 2020
@rainersigwald rainersigwald added the Area: Static Graph Issues with -graph, -isolate, and the related APIs. label Nov 19, 2020
@rainersigwald rainersigwald self-assigned this Nov 19, 2020
@rainersigwald
Copy link
Member

I repro with two interesting tidbits:

  1. Only in dotnet msbuild, not msbuild.exe
  2. Not with a bootstrap debug copy of MSBuild

@rainersigwald
Copy link
Member

Same debug copy of MSBuild repros when put in a .NET 5 install.

Proximate cause is here:

 	Microsoft.Build.dll!Microsoft.Build.Shared.FileUtilities.GetFullPath(string path) Line 411	C#
 	Microsoft.Build.dll!Microsoft.Build.Shared.FileUtilities.NormalizePath(string path) Line 372	C#
 	Microsoft.Build.dll!Microsoft.Build.Graph.GraphBuilder.AddGraphBuildPropertyToEntryPoints(System.Collections.Generic.IEnumerable<Microsoft.Build.Graph.ProjectGraphEntryPoint> entryPoints) Line 390	C#
 	Microsoft.Build.dll!Microsoft.Build.Graph.GraphBuilder.GraphBuilder(System.Collections.Generic.IEnumerable<Microsoft.Build.Graph.ProjectGraphEntryPoint> entryPoints, Microsoft.Build.Evaluation.ProjectCollection projectCollection, Microsoft.Build.Graph.ProjectGraph.ProjectInstanceFactoryFunc projectInstanceFactory, Microsoft.Build.Graph.ProjectInterpretation projectInterpretation, int degreeOfParallelism, System.Threading.CancellationToken cancellationToken) Line 61	C#
 	Microsoft.Build.dll!Microsoft.Build.Graph.ProjectGraph.ProjectGraph(System.Collections.Generic.IEnumerable<Microsoft.Build.Graph.ProjectGraphEntryPoint> entryPoints, Microsoft.Build.Evaluation.ProjectCollection projectCollection, Microsoft.Build.Graph.ProjectGraph.ProjectInstanceFactoryFunc projectInstanceFactory, int degreeOfParallelism, System.Threading.CancellationToken cancellationToken) Line 422	C#
 	Microsoft.Build.dll!Microsoft.Build.Graph.ProjectGraph.ProjectGraph(System.Collections.Generic.IEnumerable<Microsoft.Build.Graph.ProjectGraphEntryPoint> entryPoints, Microsoft.Build.Evaluation.ProjectCollection projectCollection, Microsoft.Build.Graph.ProjectGraph.ProjectInstanceFactoryFunc projectInstanceFactory) Line 329	C#
>	Microsoft.Build.dll!Microsoft.Build.Execution.BuildManager.ExecuteGraphBuildScheduler(Microsoft.Build.Graph.GraphBuildSubmission submission) Line 1414	C#

because the CWD is changed to the entry-point's folder. Not sure why yet.

@rainersigwald
Copy link
Member

Something in ExecuteRestore is changing CWD. So there's a workaround:

dotnet build -graph src\MSBuild\MSBuild.csproj -restore:false

@cdmihai
Copy link
Contributor

cdmihai commented Nov 19, 2020

I repro with two interesting tidbits:

1. Only in dotnet msbuild, not msbuild.exe
2. Not with a bootstrap debug copy of MSBuild

That pretty much sums up what I tested things on, and what our CI tests against :)

@rainersigwald
Copy link
Member

I wonder if this is going to start failing when we update to target .NET 5.

@cdmihai
Copy link
Contributor

cdmihai commented Nov 19, 2020

I was wondering why the static graph tests in the sdk are passing. But when I looked into it I remembered that they got disabled and we didn't follow up into reenabling them

@rainersigwald
Copy link
Member

It looks like just doing a graph build changes the working directory--I see it when I disable restore but break in and check after the main call to ExecuteGraphBuild.

I'm going to stop looking at this for the moment but we should definitely chase this down.

@rainersigwald rainersigwald removed their assignment Nov 19, 2020
@rainersigwald rainersigwald added this to the MSBuild 16.9 milestone Nov 19, 2020
@rainersigwald rainersigwald added Partner request and removed needs-triage Have yet to determine what bucket this goes in. labels Nov 19, 2020
@rainersigwald
Copy link
Member

BTW, I think that invalidates my other findings: msbuild.exe doesn't have -restore by default, and neither does directly calling dotnet path\to\private\msbuild.dll. I bet if we added -restore there (as dotnet build does implicitly) we'd see the problem.

@marcpopMSFT
Copy link
Member

@dfederm Would you be looking into static graph issues going forward?

@dfederm
Copy link
Contributor

dfederm commented Jan 7, 2022

@marcpopMSFT Sure, will take a look.

@dfederm
Copy link
Contributor

dfederm commented Feb 2, 2022

Looks like the cwd gets changed with non-graph builds too, but paths are resolved to be absolute beforehand so it's not a problem. Likely we just need to do the same with the graph build request

Forgind pushed a commit that referenced this issue Feb 15, 2022
…restore (#7361)

Fixes #5898

Context
Executing projects changes the cwd, which makes relative paths behave unexpectedly. So when an implicit restore happens (/restore), then the cwd is different than what it was originally. For non-graph builds, this isn't a problem as the BuildRequestData gets the full path of the project file before the implicit restore, so there effectively are no relative paths to deal with. However, this was not the case for GraphBuildRequestData, so doing a build with /retore, /graph, and a relative path to a project file was erroring incorrectly.

Changes Made
The ProjectGraphEntryPoint constructor will now get the full path of the project file, similar to what BuildRequestData does today.

I also followed all uses of ProjectGraphEntryPoint.ProjectFile and removed any normalization since it's now always done already.

Testing
I tested this change calling msbuild on a relative path with graph on/off and restore on/off. It now behaves as expected.
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Static Graph Issues with -graph, -isolate, and the related APIs. bug Partner request triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants