Use an MSBuild Task for RID selection instead of shelling out

[baronfel]

Description

This is mostly a perf optimization. Today, during every restore, we shell out to a binary that takes ~120ms:

This PR replaces this with a native MSBuild Task which takes ~3ms:

This handles the packaging too - this is such a simple task that we can reuse a lot of the previous mechanisms with minimal structural changes.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • Yes
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
      • No
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
    • If yes, have you done a threat model and had a security review?
      • Yes
      • No
  • No
• Does the change require an update in our Aspire docs?
  • Yes
    • Is this introducing a breaking change?
      • Yes
        • Link to aspire-docs issue (please use this breaking-change template):
      • No
        • Link to aspire-docs issue (please use this doc-idea template):
  • No

[baronfel]

this is a nitpick kind of thing - when the items are declared inline like this they show as one 'thing' in the binlog viewer:

this is easier to look at than N separate items.

[baronfel]

none of this is necessary if we're not spawning binaries anymore in the SDK-delivered targets

[baronfel]

I chose ns2.0 so that we should work in VS and in CLI. I've tested both and that does seem to be the case.

[baronfel]

I logged dotnet/sdk#48352 to track the SDK providing a Task to do this in-box, so that eventually you could remove this one.

[tmds]

Could NETCoreSdkPortableRuntimeIdentifier be an alternative to doing a rid graph lookup?

[baronfel]

maybe? I was mostly focused on translating the behaviors that existed before. I like than an explicit lookup will have a clearer error message than a missing implicitly-defined PackageReference.

[tmds]

Most use cases are expressed with portable rids.

Either the portable rid comes from the user, or it's the one from the SDK.

For the latter, rather than mapping NETCoreSdkRuntimeIdentifier via the rid graph, NETCoreSdkPortableRuntimeIdentifier can be used.

[tmds]

On a linux-musl-x64 system, should this use linux-x64 assets? Or would those be binary incompatible?

[joperezr]

We can't use NETCoreSdkPortableRuntimeIdentifier as there are platforms where that won't be correct. This is how we originally had it, where we'd just use that to infer the package name but it didn't work for some distros where that is more specific and we don't cross-compile for it. As an example of the types of things that would show up if we did this are #5486 which this logic was specifically fixing.

[tmds]

@joperezr are you sure you were using NETCoreSdkPortableRuntimeIdentifier? (Note: this contains: Portable.)

I'm looking at #5695 and I don't see it.

[eerhardt]

I believe @joperezr made this out of proc to not have to depend on the NuGet version used by the SDK. See #5695 (comment)

cc @dsplaisted

[baronfel]

If that's the concern, couldn't we just use a lower version of the nuget package as a compile time only asset, letting the ambient one from whichever SDK was in use be the one that is actually executed at runtime?

[eerhardt]

couldn't we just use a lower version of the nuget package as a compile time only asset, letting the ambient one from whichever SDK was in use be the one that is actually executed at runtime?

That would work if the NuGet libraries never make binary breaking changes, which they have in the past.

[joperezr]

As @eerhardt, it was intentional to make this run out-of-proc in order to avoid having to fix dependency versions. We are okay with doing this as the plan is temporary, because the intent is to move all of this logic to the SDK instead, and therefore not have the dependency management concerns.

[dsplaisted]

What dependencies does this PR actually add? I think when @joperezr was originally trying this out there were a bunch of dependencies that had to be added. But I don't really see any in this PR. Is that because they're coming in transitively from an MSBuild pakcage?

If we don't want to make this change in Aspire, then the ideal solution is to add a GetBestMatchingRid task to the SDK, and then when Aspire requires a version of the SDK that has that task it can switch to using it.