-
Notifications
You must be signed in to change notification settings - Fork 689
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
Embed NuGet files in binlog after restore #5042
Conversation
test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/TaskLoggingQueueTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good!
One small suggestion about handling packages.config projects too.
If we're doing it now, might as well get it done fully :)
src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs
Outdated
Show resolved
Hide resolved
0936da1
to
93f8eb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add the files to be included for each case as part of the PR description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ this feature!
src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @zivkan had some feedback, but looks great other than that.
test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/BuildTasksUtilityTests.cs
Outdated
Show resolved
Hide resolved
8206cce
to
f8bd8a9
Compare
Bug
Fixes: NuGet/Home#12374
Regression? No Last working version:
Description
This is a follow-up of #5016 but now it includes the files from all projects that took part in restore.
This updates the Restore task to embed NuGet files in the binlog after Restore is complete. Even if restore fails, any files that exist will be embedded:
In this case the package name is wrong and the
.dgspec
is still in the binlog.For static graph-based restore, the output paths are logged back to the RestoreTaskEx so it can embed the files in the binlog at the end of the actual build. I could not find a way to embed them in the
nuget.binlog
that contains evaluation...Related to dotnet/sdk#16840
PR Checklist
PR has a meaningful title
PR has a linked issue.
Described changes
Tests
Documentation