Add non-portable RID to list of known runtime packs instead of overwriting by jtschuster · Pull Request #96858 · dotnet/runtime

jtschuster · 2024-01-11T19:42:09Z

In #75597, for source-build we replace all the RIDs with only the PackageRID in the KnownFrameworkReference for NetCurrent when we should have only added an additional RID.

This was causing issues in crossbuilds of vertical builds. The crossgen and ilc projects that run within the build require a runtime for the build machine's RID, but it wasn't found in KnownFrameworkReference and the build failed during restore.

Part of reenabling Crossgen and ILC in arm64 crossbuild verticals dotnet/source-build#3698

7 2024 -0800

ghost · 2024-01-11T19:42:21Z

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

In #75597, we replace all the RIDs with only the PackageRID in the KnownFrameworkReference for NetCurrent when we should have only added an additional RID.

This was causing issues in crossbuilds of vertical builds. The crossgen and ilc projects that run within the build require a runtime for the build machine's RID, but it wasn't found in KnownFrameworkReference and the build failed during restore.

Part of reenabling Crossgen and ILC in arm64 crossbuild verticals dotnet/source-build#3698

Author:	jtschuster
Assignees:	-
Labels:	`area-Infrastructure-libraries`
Milestone:	-

ViktorHofer · 2024-01-11T19:53:29Z

+ <KnownFrameworkReference Update="@(KnownFrameworkReference
+ ->WithMetadataValue('Identity', 'Microsoft.NETCore.App')
+ ->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)'))"
+ RuntimePackRuntimeIdentifiers="%(RuntimePackRuntimeIdentifiers);$(PackageRID)" />


Does that Update statement with multiple lines actually work? I never saw this before.

It looks like it might be working, but I just double checked and it looks like the filtering might not be working correctly even on a single line.

Ah, it needs to be a condition, not WithMetadataValue

We shouldn't use and item batching condition here. Such are less performant. Prefer msbuild item functions when possible.
Also, this currently works before with the item functions.

In my repro project the item functions don't seem to work and will add the RID to each KnownFrameworkReference regardless of TFM or Identity. I'll see if there's a different way to do it with msbuild items functions.
<Project> <PropertyGroup> <NetCoreAppCurrent>net8.0</NetCoreAppCurrent> <PackageRid>linux-arm64</PackageRid> </PropertyGroup> <ItemGroup> <KnownFrameworkReferenceInclude="Microsoft.NETCore.App"TargetFramework="net8.0"RuntimePackRuntimeIdentifiers="OriginalRids" /> <KnownFrameworkReferenceInclude="Microsoft.NETCore.App.NativeAOT"TargetFramework="net8.0"RuntimePackRuntimeIdentifiers="OriginalRids" /> <KnownFrameworkReferenceInclude="Microsoft.NETCore.App"TargetFramework="net7.0"RuntimePackRuntimeIdentifiers="OriginalRids2" /> <KnownFrameworkReferenceInclude="Microsoft.NETCore.App.NativeAOT"TargetFramework="net7.0"RuntimePackRuntimeIdentifiers="OriginalRids2" /> <KnownCrossgen2PackInclude='Microsoft.NETCore.App.Crossgen2'Crossgen2RuntimeIdentifiers="OriginalRids"TargetFramework="net8.0" /> <KnownCrossgen2PackInclude='Microsoft.NETCore.App.Crossgen3'Crossgen2RuntimeIdentifiers="OriginalRids"TargetFramework="net8.0" /> <KnownCrossgen2PackInclude='Microsoft.NETCore.App.Crossgen2'Crossgen2RuntimeIdentifiers="OriginalRids2"TargetFramework="net7.0" /> <KnownCrossgen2PackInclude='Microsoft.NETCore.App.Crossgen3'Crossgen2RuntimeIdentifiers="OriginalRids2"TargetFramework="net7.0" /> </ItemGroup> <TargetName="MyModifyingTarget"> <ItemGroupCondition="'$(NoBatch)' != 'true'"> <KnownFrameworkReferenceUpdate="@(KnownFrameworkReference)"Condition="'%(Identity)' == 'Microsoft.NETCore.App' AND '%(TargetFramework)' == '$(NetCoreAppCurrent)'"RuntimePackRuntimeIdentifiers="%(RuntimePackRuntimeIdentifiers);$(PackageRID)" /> <KnownCrossgen2PackUpdate="@(KnownCrossgen2Pack)"Condition="'%(Identity)' == 'Microsoft.NETCore.App.Crossgen2' AND '%(TargetFramework)' == '$(NetCoreAppCurrent)'"Crossgen2RuntimeIdentifiers="%(Crossgen2RuntimeIdentifiers);$(PackageRID)"/> </ItemGroup> <ItemGroupCondition="'$(NoBatch)' == 'true'"> <KnownFrameworkReferenceUpdate="@(KnownFrameworkReference)->WithMetadataValue('Identity', 'Microsoft.NETCore.App')->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)')"RuntimePackRuntimeIdentifiers="%(RuntimePackRuntimeIdentifiers);$(PackageRID)" /> <KnownCrossgen2PackUpdate="@(KnownCrossgen2Pack->WithMetadataValue('Identity', 'Microsoft.NETCore.App.Crossgen2')->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)'))"Crossgen2RuntimeIdentifiers="%(Crossgen2RuntimeIdentifiers);$(PackageRID)"/> </ItemGroup> <MessageImportance="High"Text="KnownFrameworkReference:%0a%09Name: %(KnownFrameworkReference.Identity)%0a%09TFM: %(KnownFrameworkReference.TargetFramework)%0a%09RIDs: %(KnownFrameworkReference.RuntimePackRuntimeIdentifiers)" /> <MessageImportance="High"Text="KnownCrossgen2Pack:%0a%09Name: %(KnownCrossgen2Pack.Identity)%0a%09TFM: %(KnownCrossgen2Pack.TargetFramework)%0a%09RIDs: %(KnownCrossgen2Pack.Crossgen2RuntimeIdentifiers)" /> </Target> </Project>
With NoBatch=true
KnownFrameworkReference: Name: Microsoft.NETCore.App TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownFrameworkReference: Name: Microsoft.NETCore.App.NativeAOT TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownFrameworkReference: Name: Microsoft.NETCore.App TFM: net7.0 RIDs: OriginalRids2;linux-arm64 KnownFrameworkReference: Name: Microsoft.NETCore.App.NativeAOT TFM: net7.0 RIDs: OriginalRids2;linux-arm64 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen2 TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen3 TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen2 TFM: net7.0 RIDs: OriginalRids2;linux-arm64 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen3 TFM: net7.0 RIDs: OriginalRids2;linux-arm64
With NoBatch=false
KnownFrameworkReference: Name: Microsoft.NETCore.App TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownFrameworkReference: Name: Microsoft.NETCore.App.NativeAOT TFM: net8.0 RIDs: OriginalRids KnownFrameworkReference: Name: Microsoft.NETCore.App TFM: net7.0 RIDs: OriginalRids2 KnownFrameworkReference: Name: Microsoft.NETCore.App.NativeAOT TFM: net7.0 RIDs: OriginalRids2 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen2 TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen3 TFM: net8.0 RIDs: OriginalRids KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen2 TFM: net7.0 RIDs: OriginalRids2 KnownCrossgen2Pack: Name: Microsoft.NETCore.App.Crossgen3 TFM: net7.0 RIDs: OriginalRids2

Ah, that half fixed it, but it still looks like it matches on the name of each item only, so it will update all 'Microsoft.NETCore.App' items in the group, regardless of the TFM, so net7.0 item will also have the new RID.
KnownFrameworkReference: Name: Microsoft.NETCore.App TFM: net8.0 RIDs: OriginalRids;linux-arm64 KnownFrameworkReference: Name: Microsoft.NETCore.App.NativeAOT TFM: net8.0 RIDs: OriginalRids KnownFrameworkReference: Name: Microsoft.NETCore.App TFM: net7.0 RIDs: OriginalRids;linux-arm64 KnownFrameworkReference: Name: Microsoft.NETCore.App.NativeAOT TFM: net7.0 RIDs: OriginalRids
We could batch only the items with the Identity 'Microsoft.NETCore.App' on TargetFramework == NetCurrent to reduce the overhead, or assume it won't be an issue if we haven't already hit an issue where a previous TFM has the additional RID.

I think you are doing something wrong. <KnownFrameworkReference Update="@(KnownFrameworkReference->WithMetadataValue('Identity', 'Microsoft.NETCore.App')->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)')) ... works for me.

Okay, I might be doing something wrong in the repro, I'll try running a source build and check the binlog to test the change.

In the source build all the items in the KnownFrameworkReferences itemgroup with the name Microsoft.NETCore.App are adding the PackageRid to their metadata, no matter the TargetFramework metadata. In a few places we do something like this:
<KnownFrameworkReferenceUpdate="Microsoft.NETCore.App"> <RuntimePackRuntimeIdentifiersCondition="'%(TargetFramework)' == '$(NetCoreAppCurrent)'">%(RuntimePackRuntimeIdentifiers);$(PackageRID)</RuntimePackRuntimeIdentifiers> </KnownFrameworkReference> <KnownCrossgen2PackUpdate="Microsoft.NETCore.App.Crossgen2"> <Crossgen2RuntimeIdentifiersCondition="'%(TargetFramework)' == '$(NetCoreAppCurrent)'">%(Crossgen2RuntimeIdentifiers);$(PackageRID)</Crossgen2RuntimeIdentifiers> </KnownCrossgen2Pack>
It uses some batching, but I can't find another way to only update the item with the latest TFM.

OK I was able to reproduce this. This seems to be an msbuild issue. Filed dotnet/msbuild#9636

…iting in source-build (dotnet#96858) In dotnet#75597, for source-build we replace all the RIDs with only the `PackageRID` in the `KnownFrameworkReference` for `NetCurrent` when we should have only added an additional RID. This was causing issues in crossbuilds of vertical builds. The crossgen and ilc projects that run within the build require a runtime for the build machine's RID, but it wasn't found in `KnownFrameworkReference` and the build failed during restore. Part of reenabling Crossgen and ILC in arm64 crossbuild verticals dotnet/source-build#3698

Add to metadata, don't overwrite
0d70086
7 2024 -0800

jtschuster requested review from ViktorHofer and steveisok January 11, 2024 19:42

ghost added the area-Infrastructure-libraries label Jan 11, 2024

ghost assigned jtschuster Jan 11, 2024

ViktorHofer reviewed Jan 11, 2024
View reviewed changes

am11 mentioned this pull request Jan 11, 2024
Use runtime packs for NativeAOT on .NET 10 or higher dotnet/sdk#37872
Closed

Don't use multiline Update string
51f8e92

ViktorHofer reviewed Jan 11, 2024
View reviewed changes

Comment threadDirectory.Build.targets Outdated

ViktorHofer approved these changes Jan 11, 2024
View reviewed changes

Use a little batching to only change the current framework reference
d3714c4

build-analysisBot mentioned this pull request Jan 12, 2024
CollectionsMarshal.AsSpan assertion failure in System.Text.Json tests #96876
Closed

ViktorHofer approved these changes Jan 12, 2024
View reviewed changes

jtschuster mentioned this pull request Jan 12, 2024
BasicEventSourceTests.TestsManifestGeneration.Test_EventSource_EtwManifestGeneration intermittent test failures #96904
Closed

jtschuster merged commit 5e24539 into dotnet:mainJan 12, 2024

jtschuster mentioned this pull request Jan 18, 2024
Don't disable crossgen in crossbuild verticals #97163
Merged

github-actionsBot locked and limited conversation to collaborators Feb 12, 2024

Navigation Menu

Uh oh!

Add non-portable RID to list of known runtime packs instead of overwriting - #96858

Add non-portable RID to list of known runtime packs instead of overwriting#96858
jtschuster merged 3 commits into
dotnet:mainfrom
jtschuster:ReenableCrossgenAndIlcInUB

jtschuster commented Jan 11, 2024•
edited
Loading

Uh oh!

ghost commented Jan 11, 2024

Uh oh!

ViktorHoferJan 11, 2024

Uh oh!

jtschusterJan 11, 2024

Uh oh!

jtschusterJan 11, 2024

Uh oh!

ViktorHoferJan 11, 2024•
edited
Loading

Uh oh!

jtschusterJan 11, 2024

Uh oh!

jtschusterJan 11, 2024

Uh oh!

ViktorHoferJan 11, 2024

Uh oh!

jtschusterJan 11, 2024

Uh oh!

jtschusterJan 11, 2024•
edited
Loading

Uh oh!

ViktorHoferJan 12, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jtschuster commented Jan 11, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jan 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ViktorHoferJan 11, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtschusterJan 11, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jtschuster commented Jan 11, 2024•
edited
Loading

ViktorHoferJan 11, 2024•
edited
Loading

jtschusterJan 11, 2024•
edited
Loading