-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update ProcessFrameworkReferences and ResolveAppHosts to not look for runtime-specific assets for the any
RID
#50421
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
Update ProcessFrameworkReferences and ResolveAppHosts to not look for runtime-specific assets for the any
RID
#50421
Conversation
… runtime-specific assets for the �ny RID
This PR is targeting |
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.
Pull Request Overview
This PR fixes issues where the .NET SDK was attempting to resolve runtime-specific assets for the platform-agnostic "any" RID, which has no associated runtime or apphost packs. The changes prevent unnecessary error reporting when using the "any" RID in multi-targeting scenarios.
- Adds checks to skip processing the "any" RID in both
ProcessFrameworkReferences
andResolveAppHosts
tasks - Prevents
UnavailableRuntimePack
errors from being reported for the platform-agnostic "any" RID - Improves the build experience for projects using the "any" RID in self-contained deployments
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Tasks/Microsoft.NET.Build.Tasks/ResolveAppHosts.cs | Adds check to skip apphost pack resolution for "any" RID |
src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs | Adds checks to skip runtime pack processing for "any" RID in two locations |
src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
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.
Thanks, this seems like a wise idea. Ideally we also get a review from someone more familiar with this area.
@@ -335,6 +335,7 @@ out List<KnownRuntimePack> knownRuntimePacksForTargetFramework | |||
var runtimeRequiredByDeployment | |||
= (SelfContained || ReadyToRunEnabled) && | |||
!string.IsNullOrEmpty(RuntimeIdentifier) && | |||
RuntimeIdentifier != "any" && | |||
selectedRuntimePack != null && |
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 DependencyContextBuilder, I wonder if we could skip the check for nativeRuntimeTargetsFiles if the RID is Any?
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 didn't quite understand what you meant here, but I did see a related change that we needed to make around a check on _isPortable
that I've updated too.
@@ -335,6 +335,7 @@ out List<KnownRuntimePack> knownRuntimePacksForTargetFramework | |||
var runtimeRequiredByDeployment | |||
= (SelfContained || ReadyToRunEnabled) && | |||
!string.IsNullOrEmpty(RuntimeIdentifier) && | |||
RuntimeIdentifier != "any" && | |||
selectedRuntimePack != null && | |||
!string.IsNullOrEmpty(selectedRuntimePack.Value.RuntimePackNamePatterns); |
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'm surprised GenerateBundle
seems to fail with Any rid. Is there a chance that ever gets called? Likely, the answer is no.
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.
GenerateBundle is only called for single-file publishing, and I believe that the combination of any
+ singlefile is blocked higher up the stack. Good call-out!
I assume this should also fix the other currently-failing scenarios, right? i.e. it should fix all of
|
@andrewlock precisely! All of those use this same Runtime Pack discovery/resolution pipeline, so from a build perspective they are all the 'same problem'. |
/backport to release/10.0.1xx |
Started backporting to release/10.0.1xx: https://github.com/dotnet/sdk/actions/runs/17177864867 |
@baronfel backporting to "release/10.0.1xx" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: update ProcessFrameworkReferences and ResolveAppHosts to not look for runtime-specific assets for the �ny RID
Applying: add a test to cover mixed self-contained and agnostic tool package generation
Applying: slightly correct the test
error: sha1 information is lacking or useless (test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0003 slightly correct the test
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
… runtime-specific assets for the `any` RID (dotnet#50421)
This fixes two more issues reported in #50312 (comment) that are in the same vein as the original issue, which we fixed in #50376.
ProcessFrameworkReferences
Task takesRuntimeIdentifiers
as an input and tries to register matching runtime packs for all RIDs inRuntimeIdentifiers
for eachFrameworkReference
the project needs. It shouldn't do this for theany
RID, as this RID has no matching runtime pack. When it can't resolve some of those, it reports them asUnavailableRuntimePack
Items, which theResolveRuntimePackAssets
Task eventually reports as errors.ResolveAppHosts
Task similarly tries to register matching apphost packs for theOtherRuntimeIdentifiers
the project needs. It too needs to just...not do that for theany
RID.Before:

After:

Need to add some tests still - I think we missed this because our existing multi-tool-package-build tests that hit the
any
RID are all framework-dependent, so no RID-specific runtime packs would ever be downloaded. Adding a single test case that covers self-contained should cover all of the reported scenarios.