Prefer the portable ILCompiler when publishing for the portable RID on a source-built SDK.

[tmds]

Fixes #41727.

cc @omajid @am11 @dsplaisted @baronfel @jkotas @ViktorHofer

[ViktorHofer]

LGTM but I defer to @dsplaisted who is the ProcessFrameworkReferences expert.

[baronfel]

This is reasonable to me as well. @tmds where would be the best place to test this? dotnet/source-build?

[tmds]

where would be the best place to test this? dotnet/source-build?

I recently learned about https://github.com/dotnet/scenario-tests.
The caveat with the test is that it probably expects there to be a portable ILCompiler package, but the non-portable build is not responsible for building it. I don't know if and how scenario-tests handles such cases.

We have our own test but only run it (late) on releases (including preview releases) since we know Microsoft will have published the portable ILCompiler package to nuget.org for those.

[tmds]

@baronfel is there still time to get this into preview6?

[baronfel]

It's looking like a 'no' unless we can get Daniel to take a look at this today, after talking to the build team.

[dsplaisted]

It seems to me like this type of logic should be in .targets files, and it should set an MSBuild property for the RuntimeIdentifier to use. Maybe NETCoreSdkRuntimeIdentifier should generally be the same as NETCoreSdkPortableRuntimeIdentifier.

However, if there's still time to get it in preview 6, I'm OK with this change for now.

[tmds]

Maybe NETCoreSdkRuntimeIdentifier should generally be the same as NETCoreSdkPortableRuntimeIdentifier.

I considered this. If we do this, as you say, it makes sense to apply it everywhere, and based on that I had opted to write this PR with a "localized" change instead.

I will look into this.

However, if there's still time to get it in preview 6, I'm OK with this change for now.

I think we missed the deadline. I had preferred for this to be in preview 6, but I don't consider it a blocker.

[tmds]

@dsplaisted if we set NETCoreSdkRuntimeIdentifier=linux-x64 and RuntimeIdentifier=linux-x64 then ProcessFrameworkReferences isn't ever going to resolve a non-portable RID assets. So I don't think we can change to pass NETCoreSdkPortableRuntimeIdentifier instead of NETCoreSdkRuntimeIdentifier to ProcessFrameworkReferences?

[dsplaisted]

@dsplaisted if we set NETCoreSdkRuntimeIdentifier=linux-x64 and RuntimeIdentifier=linux-x64 then ProcessFrameworkReferences isn't ever going to resolve a non-portable RID assets. So I don't think we can change to pass NETCoreSdkPortableRuntimeIdentifier instead of NETCoreSdkRuntimeIdentifier to ProcessFrameworkReferences?

What if the RuntimeIdentifier is fedora.40-x64? It should resolve the runtime pack in that case, do the tool packs not use the RuntimeIdentifier the same way?

EDIT: Basically IIUC, the default RuntimeIdentifier should be linux-x64, but there should be the option to use fedora.40-x64. So if you explicitly set the RuntimeIdentifier to fedora.40-x64, would it work?

[tmds]

The issue we're trying to fix is that for linux-x64 we want the to prefer the linux-x64 ILCompiler for the host over the non-portable ILCompiler that matches the SDK rid (even though it's a better graph match).

[tmds]

@dsplaisted I re-wrote this in a way it could be moved into the .targets file.
NETCoreSdkRuntimeIdentifier would be passed what hostRuntimeIdentifier is set to.

Do you want me to move it to the .targets file, or keep it here?

[dsplaisted]

It's fine here for now. It seems to me like we might eventually need to know which host Runtime Identifier we're using elsewhere. But until we run into a case like that I think it's good as is.