- Notifications
You must be signed in to change notification settings - Fork5.2k
Add non-portable RID to list of known runtime packs instead of overwriting#96858
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
7 2024 -0800
ghost commentedJan 11, 2024
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsIn#75597, we replace all the RIDs with only the 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 Part of reenabling Crossgen and ILC in arm64 crossbuild verticalsdotnet/source-build#3698
|
Directory.Build.targets Outdated
| <KnownFrameworkReferenceUpdate="@(KnownFrameworkReference | ||
| ->WithMetadataValue('Identity', 'Microsoft.NETCore.App') | ||
| ->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)'))" | ||
| RuntimePackRuntimeIdentifiers="%(RuntimePackRuntimeIdentifiers);$(PackageRID)" /> |
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.
Does that Update statement with multiple lines actually work? I never saw this before.
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.
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.
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.
Ah, it needs to be a condition, notWithMetadataValue
ViktorHoferJan 11, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
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.
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.
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-arm64With 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: OriginalRids2There 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.
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, sonet7.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: OriginalRidsWe could batch only the items with the Identity 'Microsoft.NETCore.App' onTargetFramework == 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.
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.
I think you are doing something wrong.<KnownFrameworkReference Update="@(KnownFrameworkReference->WithMetadataValue('Identity', 'Microsoft.NETCore.App')->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)')) ... works for me.
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.
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.
jtschusterJan 11, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
In the source build all the items in theKnownFrameworkReferences itemgroup with the nameMicrosoft.NETCore.App are adding thePackageRid to their metadata, no matter theTargetFramework metadata. In afewplaces 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.
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.
OK I was able to reproduce this. This seems to be an msbuild issue. Fileddotnet/msbuild#9636
Uh oh!
There was an error while loading.Please reload this page.
…iting in source-build (dotnet#96858)Indotnet#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 verticalsdotnet/source-build#3698
Uh oh!
There was an error while loading.Please reload this page.
In#75597, for source-build we replace all the RIDs with only the
PackageRIDin theKnownFrameworkReferenceforNetCurrentwhen 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
KnownFrameworkReferenceand the build failed during restore.Part of reenabling Crossgen and ILC in arm64 crossbuild verticalsdotnet/source-build#3698