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

It's not possible for a workload to set a default RuntimeIdentifier #18354

Closed
rolfbjarne opened this issue Jun 18, 2021 · 7 comments · Fixed by #18639
Closed

It's not possible for a workload to set a default RuntimeIdentifier #18354

rolfbjarne opened this issue Jun 18, 2021 · 7 comments · Fixed by #18639
Assignees
Labels
untriaged Request triage from a team member

Comments

@rolfbjarne
Copy link
Member

Description

In Xamarin workloads, we'd like to set a default RuntimeIdentifier for each platform we support.

The problem is that the Microsoft.NET.RuntimeIdentifierInference.targets file is loaded before our WorkloadManifest.targets file, and executes some logic depending on whether the RuntimeIdentifier is set or not, which means that it's too late if we try to set RuntimeIdentifer in our WorkloadManifest.targets file.

And we can't set it earlier (AutoImports.props), because that would be a global default, not a platform-specific default.

The workaround I've found is to replicate the behavior of Microsoft.NET.RuntimeIdentifierInference.targets that we want in our WorkloadManifest.targets, which comes down to something like this:

  <!-- First set the default RuntimeIdentifier if not already specified. -->
  <PropertyGroup Condition=" '$(RuntimeIdentifier)' == '' And '$(RuntimeIdentifiers)' == '' ">
    <RuntimeIdentifier Condition="'$(_PlatformName)' == 'iOS'">iossimulator-x64</RuntimeIdentifier>
    <RuntimeIdentifier Condition="'$(_PlatformName)' == 'tvOS'">tvossimulator-x64</RuntimeIdentifier>
    <RuntimeIdentifier Condition="'$(_PlatformName)' == 'macOS'">osx-x64</RuntimeIdentifier>
    <RuntimeIdentifier Condition="'$(_PlatformName)' == 'MacCatalyst'">maccatalyst-x64</RuntimeIdentifier>
    <!-- Workaround/hack:
        The Microsoft.NET.RuntimeIdentifierInference.targets file is loaded
        before this file, and executes some logic depending on whether the
        RuntimeIdentifier is set or not. Since RuntimeIdentifier isn't set at
        that point (we're setting it here), we need to replicate the logic in
        the Microsoft.NET.RuntimeIdentifierInference.targets file to make sure
        things work as expected.
    -->
    <SelfContained>true</SelfContained>
    <_RuntimeIdentifierUsesAppHost>false</_RuntimeIdentifierUsesAppHost>
    <UseAppHost>false</UseAppHost>
    <IntermediateOutputPath>$(IntermediateOutputPath)$(RuntimeIdentifier)\</IntermediateOutputPath>
    <OutputPath>$(OutputPath)$(RuntimeIdentifier)\</OutputPath>
  </PropertyGroup>

Something more appropriate would be nice...

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@akoeplinger akoeplinger transferred this issue from dotnet/runtime Jun 18, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Jun 18, 2021
@akoeplinger
Copy link
Member

/cc @dsplaisted

@rolfbjarne
Copy link
Member Author

rolfbjarne commented Jun 25, 2021

Another point is that it's not possible to rewrite code like this to have a condition on the RuntimeIdentifier (to only include the ios-arm or the ios-arm64 version), because that WorkloadManifest.targets might be loaded after we've had a chance to set a default RuntimeIdentifier.

https://github.com/dotnet/runtime/blob/c139d00640485e8c231c54d1573dc02d7c9e8daf/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets#L21-L25

@jonathanpeppers
Copy link
Member

@rolfbjarne let's assume you get multiple RIDs working (not sure how close you are to that).

Android defaults to <RuntimeIdentifiers>android-arm64;android-x86</RuntimeIdentifiers>. Maybe you could use $(RuntimeIdentifiers) plural but only list a single RID?

@rolfbjarne
Copy link
Member Author

let's assume you get multiple RIDs working (not sure how close you are to that).

Just merged the PR right now 😄

Maybe you could use $(RuntimeIdentifiers) plural but only list a single RID?

Yes, I thought about that, and actually tried it. I ran into problems with library projects, which wouldn't build correctly - iirc things got built twice, and some parts of the build got confused. It might have been something fairly simple to fix, but:

  • It would have been a perf hit (run MSBuild again for the entire project) - and in in a common scenario too.
  • It makes the binlog harder to work with (searching for anything I'm interested in shows results from the build I'm not interested in too)
  • The build problems with library projects looked cumbersome to fix / work around.

So I decided to try to set a default RuntimeIdentifier instead, and that turned out much easier to implement (except the fact that I had to set private variables too).

@dsplaisted
Copy link
Member

I submitted #18639 for this

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

Successfully merging a pull request may close this issue.

5 participants