Skip to content
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

Use RuntimeIdentifierGraphPath if available in runtime copy of Microsoft.NET.CrossGen.targets #91077

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

elinor-fung
Copy link
Member

The sdk (shipping) version of this target was updated to use the new trimmed graph as part of dotnet/sdk#34279. This makes the equivalent change here such that it uses the RuntimeIdentifierGraphPath if set (SDK always sets starting with .NET 8 RC1), otherwise BundledRuntimeIdentifierGraphFile.

From what I can tell, the runtime repo does not actually use this particular ResolveReadyToRunCompilers target right now. The only place I found pulling in this target file is

<Import Project="$(Crossgen2SdkOverrideTargetsPath)" Condition="'$(BuildNativeAOTRuntimePack)' != 'true' and '$(Crossgen2SdkOverrideTargetsPath)' != ''" />

but the project also pulls in ReadyToRun.targets

which overrides ResolveReadyToRunCompilers
<Target Name="ResolveReadyToRunCompilers" DependsOnTargets="ResolveRuntimeFilesFromLocalBuild">

Fixes #90977

cc @dsplaisted @tmds

@ViktorHofer
Copy link
Member

We should backport this into release/8.0.

@elinor-fung
Copy link
Member Author

Yep, will backport.

@elinor-fung
Copy link
Member Author

Failures are known: #82837, #90834

@elinor-fung elinor-fung merged commit 31e99c2 into dotnet:main Aug 24, 2023
177 of 180 checks passed
@elinor-fung elinor-fung deleted the fix90977 branch August 24, 2023 19:54
@elinor-fung
Copy link
Member Author

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5968286720

@@ -443,7 +443,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<ResolveReadyToRunCompilers RuntimePacks="@(ResolvedRuntimePack)"
Crossgen2Packs="@(ResolvedCrossgen2Pack)"
TargetingPacks="@(ResolvedTargetingPack)"
RuntimeGraphPath="$(BundledRuntimeIdentifierGraphFile)"
RuntimeGraphPath="$([MSBuild]::ValueOrDefault('$(RuntimeIdentifierGraphPath)', '$(BundledRuntimeIdentifierGraphFile)'))"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another place using BundledRuntimeIdentifierGraphFile is in pretest.proj:

RuntimeGraphFiles="$(BundledRuntimeIdentifierGraphFile)"
Can we update it as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #91102

@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft.NET.CrossGen.targets: stop using legacy runtime graph
3 participants