Skip to content

Reënable static graph restore - #8856

Merged
JanKrivanek merged 2 commits into
mainfrom
revert-8498-turn-off-static-graph-restore
Jul 13, 2023
Merged

Reënable static graph restore#8856
JanKrivanek merged 2 commits into
mainfrom
revert-8498-turn-off-static-graph-restore

Conversation

@rainersigwald

Copy link
Copy Markdown
Member

Reverts #8498 which should no longer be necessary now that 17.6 has rolled out broadly--it's everywhere I checked (official build, hosted agents, the pre queue).

@rainersigwald

Copy link
Copy Markdown
Member Author

I think the failure may be a bad interaction with #8488. @JanKrivanek can you investigate as a kitten task (but super low pri, and feel free to delegate back to me if you're swamped :)).

@JanKrivanek

Copy link
Copy Markdown
Member

I think the failure may be a bad interaction with #8488. @JanKrivanek Jan Krivanek FTE can you investigate as a kitten task (but super low pri, and feel free to delegate back to me if you're swamped :)).

Adding to the kitten queue

@JaynieBai

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@GangWang01

GangWang01 commented Jun 21, 2023

Copy link
Copy Markdown
Member

The error in this PR as below ever appeared in #8488 (comment). It's supposed src\MSBuild.Bootstrap\RedirectNuGetConsoleProcess.After.Microsoft.Common.targets could redirect the dotnet path properly. But from binlog Build.zip the task was not executed. No idea why.

##[error]stage1\bin\bootstrap\net7.0\MSBuild\NuGet.RestoreEx.targets(19,5): error : (NETCORE_ENGINEERING_TELEMETRY=Restore) An error occurred trying to start process 'D:\a\1\s\stage1\bin\bootstrap\dotnet' with working directory 'D:\a\1\s'. The system cannot find the file specified.

@GangWang01

Copy link
Copy Markdown
Member

@dfederm is #8488 missing calling the target RedirectNuGetConsoleProcess in src\MSBuild.Bootstrap\RedirectNuGetConsoleProcess.After.Microsoft.Common.targets?

@dfederm

dfederm commented Jun 21, 2023

Copy link
Copy Markdown
Contributor

@GangWang01 The target is defined as <Target Name="RedirectNuGetConsoleProcess" BeforeTargets="Restore">, so it should be executing.

@Forgind

Forgind commented Jun 21, 2023

Copy link
Copy Markdown
Contributor

What is importing that .targets file?

@dfederm

dfederm commented Jun 21, 2023

Copy link
Copy Markdown
Contributor

What is importing that .targets file?

It's copied to Current\Microsoft.Common.targets\ImportAfter, so the common targets import it via wildcard

@JanKrivanek

Copy link
Copy Markdown
Member

Interesting that the build seems to fail in the non-bootstrapped legs - which should be independent on the other change (adressing bootstrap).
So I guess something is flipped with RestoreUseStaticGraphEvaluation that makes the restore fail here?

@JanKrivanek

JanKrivanek commented Jun 21, 2023

Copy link
Copy Markdown
Member

This: https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj#L215-L232 is getting skipped in the failing legs
Red herring

The difference is in the Restore target that's being executed. For failed case it's comming from D:\a\1\s\stage1\bin\bootstrap\net7.0\MSBuild\NuGet.RestoreEx.targets, for analogous build without the change it comes from D:\a\1\s\stage1\bin\bootstrap\net7.0\MSBuild\NuGet.targets

@rainersigwald

Copy link
Copy Markdown
Member Author

What is importing that .targets file?

It's copied to Current\Microsoft.Common.targets\ImportAfter, so the common targets import it via wildcard

That pulls it in for projects that import common.targets, but not for solutions. @GangWang01 can you please try putting a copy of that file in bin/bootstrap/net7.0/MSBuild/Current/SolutionFile/ImportAfter/Microsoft.NuGet.ImportAfter.targets in addition to its current location?

@rainersigwald rainersigwald added this to the VS 17.8 milestone Jun 22, 2023
@GangWang01

Copy link
Copy Markdown
Member

What is importing that .targets file?

It's copied to Current\Microsoft.Common.targets\ImportAfter, so the common targets import it via wildcard

That pulls it in for projects that import common.targets, but not for solutions. @GangWang01 can you please try putting a copy of that file in bin/bootstrap/net7.0/MSBuild/Current/SolutionFile/ImportAfter/Microsoft.NuGet.ImportAfter.targets in addition to its current location?

It worked. #8960 fixed the failure. Please help to merge it to the work branch in this PR.

@rainersigwald

Copy link
Copy Markdown
Member Author

Thanks @GangWang01. LGTM (but it's my change too, so I'm not going to hit merge until somebody else looks at the current state).

@JanKrivanek

Copy link
Copy Markdown
Member

LGTM. It has enough approvals - I'm merging it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants