Skip to content

Conversation

@ViktorHofer
Copy link
Member

MSBuild's build ifnrastructure overwrites the CustomBeforeMicrosoftCommonTargets property which results in Arcade's entry to get dropped.

Arcade adds to that property to import common files.

This fixes static graph restore not working in this repository when using Arcade's ExcludeFromBuild properties.

MSBuild's build ifnrastructure overwrites the `CustomBeforeMicrosoftCommonTargets` property which results in Arcade's entry to get dropped.

Arcade adds to that property to import common files.
@ViktorHofer ViktorHofer requested a review from a team as a code owner December 15, 2023 20:53
@ViktorHofer ViktorHofer changed the title Fix Arcade's BeforeCommon imports not working Fix Arcade's BeforeCommon imports not being respected Dec 15, 2023
@ViktorHofer
Copy link
Member Author

ViktorHofer commented Dec 15, 2023

Oh wow. I'm not sure how this change could introduce new prebuilts. cc @dotnet/source-build-contrib

@mmitche
Copy link
Member

mmitche commented Dec 15, 2023

Okay this change got a lot scarier!

@mthalman
Copy link
Member

This change appears to be causing StringTools.Benchmark.csproj to be restored but it wasn't prior to these changes.

@mthalman
Copy link
Member

Here's what the binlogs show.

Prior to these changes

Restoring packages for
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Tasks/Microsoft.Build.Tasks.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/MSBuild/MSBuild.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Package/Localization/Localization.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Utilities/Microsoft.Build.Utilities.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for .NETStandard,Version=v2.0...
    Restoring packages for .NETStandard,Version=v2.0...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Build/Microsoft.Build.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/StringTools/StringTools.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for .NETStandard,Version=v2.0...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Framework/Microsoft.Build.Framework.csproj...
    Restoring packages for .NETCoreApp,Version=v8.0...
    Restoring packages for .NETStandard,Version=v2.0...

With these changes

Restoring packages for
    Restoring packages for /tmp/NuGetScratchcloudtest_azpcontainer/089e434fec2f4c5aabf403ec1ac89ce3/42cf21743cb940e187140eb6bb86ec8f.proj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Build/Microsoft.Build.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Utilities/Microsoft.Build.Utilities.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/MSBuild/MSBuild.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Xunit.NetCore.Extensions/Xunit.NetCore.Extensions.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/StringTools.Benchmark/StringTools.Benchmark.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/MSBuild.Bootstrap/MSBuild.Bootstrap.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Package/Localization/Localization.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Framework/Microsoft.Build.Framework.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/Tasks/Microsoft.Build.Tasks.csproj...
    Restoring packages for /__w/1/s/artifacts/source-build/self/src/src/StringTools/StringTools.csproj...

Note that this comes from different restore tasks. In the first, it uses RestoreTask. The second uses RestoreTaskEx.

@ViktorHofer
Copy link
Member Author

This seems to be related to #7058. I can reproduce this locally as well. Static graph restore doesn't respect slnf files.

@ViktorHofer
Copy link
Member Author

I can reproduce this in a sample application. Filed a nuget issue for it: NuGet/Home#13097

Updated my PR not change the restore engine when building from source.

@ViktorHofer
Copy link
Member Author

This should be ready to be merged in and fixes an issue with msbuild's Arcade integration.

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.

7 participants