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

Look up the target OS for crossgen2 using the full target RID #45566

Open
wants to merge 1 commit into
base: release/9.0.2xx
Choose a base branch
from

Conversation

jkoritzinsky
Copy link
Member

Lookup the target OS for crossgen2 invocation based on the full RID instead of an OS-only portion of the RID. In source-build scenarios, we will likely only have a full os-arch RID in the RID graph. If we use only the os RID, we won't find a matching OS RID in the graph for the target OS. If we use the full os-arch RID, we'll be able to find our target in the graph.

This work allows us to remove the portablePlatform == null && _hostRuntimeIdentifier == _targetRuntimeIdentifier special case, as we'll now find the portable platform based on _targetRuntimeIdentifier.

Also remove the special checks for linux-musl as the linux-musl RID imports the linux RID, so we can just look for linux and get the right behavior.

…nstead of an OS-only portion of the RID

Our source-build partners only add the full OS-Arch nonportable RID to the RID graph, so the RID graph lookup would fail on an OS-only RID lookup.

Also, since the linux-musl RIDs import the linux RID, we can simplify the lookup to not look up linux-musl specifically.
var runtimeGraph = new RuntimeGraphCache(this).GetRuntimeGraph(RuntimeGraphPath);
string portablePlatform = NuGetUtils.GetBestMatchingRid(
runtimeGraph,
_targetPlatform,
new[] { "linux", "linux-musl", "osx", "win", "freebsd", "illumos" },
Copy link
Member

Choose a reason for hiding this comment

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

A bit ootl here, is Linux-Musl now bundled together with Linux? Or why are we removing it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The linux-musl RID has always had the linux RID as a parent. It's not 100% correct, but we can't really change it at this point due to back-compat.

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, we could have done this all along (just use linux for both linux and linux-musl here), we just didn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-ReadyToRun untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants