-
Notifications
You must be signed in to change notification settings - Fork 67
[release/8.0] Backport MSBuild 17.8.3 #932
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
Conversation
* Add MSBuild 17.8.3 + dependencies ... necessary for dotnet/runtime#98006 * Add manual pragma warning disable as in dotnet/runtime
…8.3 (dotnet#886) * Add Microsoft.Build.Tasks.Core and Microsoft.Build.Utilities.Core 17.8.3 Required for dotnet/runtime#98006 Follow-up on dotnet#885 * Update unrelated files * Manual change because of attribute filtering by GenAPI * Remove dependency from nuspec as well
|
@ViktorHofer this is still failing with a pre-built for |
The problem here is that when restoring Microsoft.Build.Tasks.Core, the dependent Microsoft.Build.Utilities.Core project gets restored from dotnet-public instead of from the local @NikolaMilosavljevic @MichaelSimons could this be related to the change in NuGet.Client that made you switch to package source mappings in the VMR? |
You're exactly right. The change to use package source mappings was only done for the VMR orchestrator, not for the source build implementation of Arcade that runs for each individual repo. I'm honestly surprised this hasn't exposed more issues in other repos. |
|
I think this only impacts release/8.0 because main is still on an SDK that doesn't have the underlying NuGet change:
My guess is that as soon as we update the SDK in main, source-build will start failing non-deterministically in the repo source-build legs. |
|
This is easier to solve here. All of the feeds in the NuGet.config are not necessary for the source-build. They are only needed for the generate tooling. The build should only be using the CurrentRepoSourceBuiltNupkgCacheDir. |
I'm confused. Why is this behavior occurring at all? This is using 8.0 SDK which shouldn't have the change in NuGet that added the indeterminism. |
|
Taking a step back and looking at the PR, I don't think this is valid. The new reference packages are targeting the current TFM (net 8.0) and are bringing in the 8.0 targeting pack which we don't want. The 8.0 branch should not target net8.0. I conjecture, for source-build runtime should be picking up the latest/n-1. |
AFAIK NuGet double inserts into main and 8.0. |
|
I only opened this because you said it was needed in dotnet/runtime#100595 (comment). Seems like that's no longer the case. I'll close this for now and we can discuss what the best solution is in that other PR. |
@mthalman @ViktorHofer @MichaelSimons
Not sure if this is enough, but giving it a try.