[One .NET] support for multiple $(RuntimeIdentifiers) #4767

jonathanpeppers · 2020-06-04T21:05:39Z

To mirror what we have for $(AndroidSupportedAbis) in the existing
Xamarin.Android world, we need a way for .NET 6 apps to support
multiple architectures.

A HelloAndroid.csproj targeting all architectures might look like:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net5.0-android</TargetFramework>
    <RuntimeIdentifiers>android.21-x86;android.21-x64;android.21-arm;android.21-arm64</RuntimeIdentifiers>
    <OutputType>Exe</OutputType>
  </PropertyGroup>
</Project>

To achieve this, we can do a nested <MSBuild/> call:

<ItemGroup>
  <_RIDs Include="$(RuntimeIdentifiers)" />
</ItemGroup>
<MSBuild
    Projects="$(MSBuildProjectFile)"
    Targets="_ComputeFilesToPublishForRuntimeIdentifiers"
    Properties="RuntimeIdentifier=%(_RIDs.Identity)">
  <Output TaskParameter="TargetOutputs" ItemName="ResolvedFileToPublish" />
</MSBuild>

For a new target:

<Target Name="_ComputeFilesToPublishForRuntimeIdentifiers"
    DependsOnTargets="ResolveReferences;ComputeFilesToPublish"
    Returns="@(ResolvedFileToPublish)">
</Target>

This is equivalent to what would happen in the legacy
_ResolveAssemblies target.

The linker then outputs files to:

obj\Release\net5.0\$(RuntimeIdentifier)\linked\

One problem with this, is you receive multiple BCL assemblies for each
architecture:

obj\Release\net5.0\android.21-arm\linked\System.Linq.dll
obj\Release\net5.0\android.21-arm64\linked\System.Linq.dll
obj\Release\net5.0\android.21-x86\linked\System.Linq.dll
obj\Release\net5.0\android.21-x64\linked\System.Linq.dll

I created a <ProcessAssemblies/> MSBuild task that checks the mvid
of each assembly to detect duplicates. Non-duplicates will be placed
in a sub-directory within the .apk file, such as:

assemblies\HelloAndroid.dll
assemblies\System.Linq.dll
assemblies\arm64-v8a\System.Private.CoreLib.dll
assemblies\armeabi-v7a\System.Private.CoreLib.dll
assemblies\x86\System.Private.CoreLib.dll
assemblies\x86_64\System.Private.CoreLib.dll

I had to make a few build & runtime changes for these ABI
sub-directories to work.

Other notes:

We must pass @(IntermediateAssembly) from the outer build to the
nested <MSBuild/> calls. Otherwise, the inner calls look for
obj\Release\net5.0\android.21-arm\HelloAndroid.dll instead of
obj\Release\net5.0\HelloAndroid.dll that actually exists.
_ComputeFilesToPublishForRuntimeIdentifiers was running some extra
targets causing <ResolveSdks/> to run 5 times for 4 ABIs. I had to
make changes in BuildOrder.targets to fix this.
<ResolveAssemblies/> now sets %(IntermediateLinkerOutput) on
each assembly item. This allows .NET 6 builds to support multiple
linker directories.

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets

dellis1972 · 2020-06-15T12:28:14Z

@jonathanpeppers its green!

...n.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets

jonpryor · 2020-06-16T20:10:43Z

src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.targets

    <SelfContained Condition=" '$(SelfContained)' == '' And '$(AndroidApplication)' == 'true' ">true</SelfContained>
-    <RuntimeIdentifier Condition=" '$(RuntimeIdentifier)' == '' And '$(AndroidApplication)' == 'true' ">android.21-x86</RuntimeIdentifier>
+    <!-- Prefer $(RuntimeIdentifiers) plural -->
+    <RuntimeIdentifiers Condition=" '$(RuntimeIdentifier)' == '' And '$(RuntimeIdentifiers)' == '' And '$(AndroidApplication)' == 'true' ">android.21-arm;android.21-arm64</RuntimeIdentifiers>


The default $(RuntimeIdentifiers) are arm32 & arm64? Why? Especially considering that the previous default was x86, and the "normal" Xamarin.Android default -- via xamarin/monodroid, for commercial users -- is "all the ABIs!" arm64-v8a;armeabi-v7a;x86;x86_64.

Given that (1) we don't know what the "commercial integration" looks like, and (2) it would be Really Really Handy to be able to run the .apks on x86(_64?) emulators, this default seems "weird" to me.

This is the same default we have here: https://github.com/xamarin/xamarin-android/blob/5a5580cd3cce0559da5e9d097b03d356a6630c05/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L234

I can put them all, if we think that makes more sense.

This now defaults to android.21-arm64;android.21-x86.

src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs

jonpryor · 2020-06-17T01:05:42Z

src/Xamarin.Android.Build.Tasks/Tasks/RuntimeIdentifierToAbi.cs

 		public override bool RunTask ()
 		{
-			if (!string.IsNullOrEmpty (RuntimeIdentifier)) {
+			if (RuntimeIdentifiers != null && RuntimeIdentifiers.Length > 0) {


Implicit to this is that $(RuntimeIdentifiers) takes precedence over $(AndroidSupportedAbis). Is this desirable/intentional? What will be the fallout to this decision?

For example, if I have an existing App.csproj which overrides $(AndroidSupportedAbis), and then I migrate to .NET 5/6 and don't touch $(RuntimeIdentifiers)… What Happens?

Given that this PR always provides a default $(RuntimeIdentifiers) value, what I think will happen is that we'll effectively ignore $(AndroidSupportedAbis), replacing the "original overridden value" with the defaults that we provide.

Is this useful or desirable? (It might be! I don't know! I am wondering if there's been any consideration here, particularly as the commit message doesn't mention this semantic.)

Meanwhile, is the opposite possible? If $(AndroidSupportedAbis) is overridden, and $(RuntimeIdentifiers) isn't -- though making this determination may itself be difficult! -- would it be at all possible to populate $(RuntimeIdentifiers) based on the value of $(AndroidSupportedAbis)?

Eventually, I think that RuntimeIdentifier and/or RuntimeIdentifiers will be implicitly set via:

<TargetFrameworkVersion>net5.0-android</TargetFrameworkVersion>

And so, it seems like $(AndroidSupportedAbis) would always be ignored going forward? Is it worth trying to support both?

Either way, I should document something about $(RuntimeIdentifiers) being preferred in DotNet5.md.

If $(AndroidSupportedAbis) will be ignored going forward, then we should consider adding a warning if the property is set at all, as having something set which doesn't do anything will be a source of confusion.

This may complicate migrating from "long-form" to "short-form" projects, i.e. it'll be a property that needs to be removed as part of the migration, but this may be for the best.

Alternatively, we would need to ensure that if $(AndroidSupportedAbis) is set, the values within it contribute to $(RuntimeIdentifiers), so that when it's set, the specified ABIs will still appear in the resulting .apk/.aab.

Since $(RuntimeIdentifiers) is needed for Restore to even work... I think we really should require it over $(AndroidSupportedAbis).

I added XA0036 if $(AndroidSupportedAbis) is present, @brendanzagaeski you might review what I added for this, thanks.

src/Xamarin.Android.Build.Tasks/Tasks/RuntimeIdentifierToAbi.cs

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets

src/monodroid/jni/basic-android-system.cc

...marin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Linker.targets

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs

Documentation/guides/messages/README.md

Documentation/guides/messages/xa0035.md

brendanzagaeski · 2020-06-18T19:04:04Z

Documentation/guides/messages/xa0035.md

+See the Microsoft documentation on [runtime identifiers][rids] for more
+information.
+
+[rids]: https://docs.microsoft.com/dotnet/core/rid-catalog


Probably it was already in mind when adding this info, but this link might need to be updated if the eventual page that talks about Android RIDs ends up being elsewhere. I think what's written here is good for now. I'm mostly just thinking out loud about the future.

Documentation/guides/messages/xa0035.md

src/Xamarin.Android.Build.Tasks/Properties/Resources.resx

brendanzagaeski

Approved on the new warning and error messages, with one last optional idea for a small change in the XA0036 message.

The generated Resources.Designer.cs and .xlf files are all set too.

src/Xamarin.Android.Build.Tasks/Properties/Resources.resx

To mirror what we have for `$(AndroidSupportedAbis)` in the existing Xamarin.Android world, we need a way for .NET 6 apps to support multiple architectures. A `HelloAndroid.csproj` targeting all architectures might look like: <Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <TargetFramework>net5.0-android</TargetFramework> <RuntimeIdentifiers>android.21-x86;android.21-x64;android.21-arm;android.21-arm64</RuntimeIdentifiers> <OutputType>Exe</OutputType> </PropertyGroup> </Project> To achieve this, we can do a nested `<MSBuild/>` call: <ItemGroup> <_RIDs Include="$(RuntimeIdentifiers)" /> </ItemGroup> <MSBuild Projects="$(MSBuildProjectFile)" Targets="_ComputeFilesToPublishForRuntimeIdentifiers" Properties="RuntimeIdentifier=%(_RIDs.Identity)"> <Output TaskParameter="TargetOutputs" ItemName="ResolvedFileToPublish" /> </MSBuild> For a new target: <Target Name="_ComputeFilesToPublishForRuntimeIdentifiers" DependsOnTargets="ResolveReferences;ComputeFilesToPublish" Returns="@(ResolvedFileToPublish)"> </Target> This is equivalent to what would happen in the legacy `_ResolveAssemblies` target. The linker then outputs files to: obj\Release\net5.0\$(RuntimeIdentifier)\linked\ One problem with this, is you receive multiple BCL assemblies for each architecture: obj\Release\net5.0\android.21-arm\linked\System.Linq.dll obj\Release\net5.0\android.21-arm64\linked\System.Linq.dll obj\Release\net5.0\android.21-x86\linked\System.Linq.dll obj\Release\net5.0\android.21-x64\linked\System.Linq.dll I created a `<ProcessAssemblies/>` MSBuild task that checks the mvid of each assembly to detect duplicates. Non-duplicates will be placed in a sub-directory within the `.apk` file, such as: * `assemblies\HelloAndroid.dll` * `assemblies\System.Linq.dll` * `assemblies\arm64-v8a\System.Private.CoreLib.dll` * `assemblies\armeabi-v7a\System.Private.CoreLib.dll` * `assemblies\x86\System.Private.CoreLib.dll` * `assemblies\x86_64\System.Private.CoreLib.dll` I had to make a few build & runtime changes for these ABI sub-directories to work. Other notes: * We must pass `@(IntermediateAssembly)` from the outer build to the nested `<MSBuild/>` calls. Otherwise, the inner calls look for `obj\Release\net5.0\android.21-arm\HelloAndroid.dll` instead of `obj\Release\net5.0\HelloAndroid.dll` that actually exists. * `_ComputeFilesToPublishForRuntimeIdentifiers` was running some extra targets causing `<ResolveSdks/>` to run 5 times for 4 ABIs. I had to make changes in `BuildOrder.targets` to fix this. * `<ResolveAssemblies/>` now sets `%(IntermediateLinkerOutput)` on each assembly item. This allows .NET 6 builds to support multiple linker directories.

Example message should say 'warning'.

jonpryor · 2020-06-22T16:30:05Z

Squash-and-merge commit message:

For .NET 5, obsolete the `$(AndroidSupportedAbis)` MSBuild property
and replace with the `$(RuntimeIdentifiers)` MSBuild property.

The `$(AndroidSupportedAbis)` MSBuild property has long been used to
control which Android ABIs are included in a `.apk` or `.aab` file,
unless `$(AndroidCreatePackagePerAbi)`=True.

The .NET Core analogue is the [`$(RuntimeIdentifiers)` property][0],
which is a `;`-separated list of "RID" values from the
[.NET Core RID catalog][1].

The mapping between `$(AndroidSupportedAbis)` and
`$(RuntimeIdentifiers)` values is as follows:

  * armeabi-v7a: android.21-arm
  *   arm64-v8a: android.21-arm64
  *         x86: android.21-x86
  *      x86_64: android.21-x64

Within a .NET 5+/`dotnet build` context, the `$(AndroidSupportedAbis)`
property is *no longer supported*; `$(AndroidSupportedAbis)` will not
be supported in .NET 5 because `$(RuntimeIdentifiers)` may be
implicitly set based on the `$(TargetFrameworks)` property; it may not
be possible to override `$(RuntimeIdentifiers)` "early enough" to take
`$(AndroidSupportedAbis)` into consideration.

If the `$(AndroidSupportedAbis)` property present, an XA0036 warning
will be generated:

	warning XA0036: The 'AndroidSupportedAbis' MSBuild property is no longer supported.
	Edit the project file in a text editor, remove any uses of 'AndroidSupportedAbis',
	and use the 'RuntimeIdentifiers' MSBuild property instead.\

A `HelloAndroid.csproj` targeting all architectures might look like:

	<Project Sdk="Microsoft.NET.Sdk">
	  <PropertyGroup>
	    <TargetFramework>net5.0-android</TargetFramework>
	    <RuntimeIdentifiers>android.21-x86;android.21-x64;android.21-arm;android.21-arm64</RuntimeIdentifiers>
	    <OutputType>Exe</OutputType>
	  </PropertyGroup>
	</Project>

To achieve this, we can do a nested `<MSBuild/>` call:

	<ItemGroup>
	  <_RIDs Include="$(RuntimeIdentifiers)" />
	</ItemGroup>
	<MSBuild
	    Projects="$(MSBuildProjectFile)"
	    Targets="_ComputeFilesToPublishForRuntimeIdentifiers"
	    Properties="RuntimeIdentifier=%(_RIDs.Identity)">
	  <Output TaskParameter="TargetOutputs" ItemName="ResolvedFileToPublish" />
	</MSBuild>

For a new target:

	<Target Name="_ComputeFilesToPublishForRuntimeIdentifiers"
	    DependsOnTargets="ResolveReferences;ComputeFilesToPublish"
	    Returns="@(ResolvedFileToPublish)">
	</Target>

This is equivalent to what would happen in the legacy
`_ResolveAssemblies` target.

The linker then outputs files to:

	obj\Release\net5.0\$(RuntimeIdentifier)\linked\

One problem with this is that you receive multiple BCL assemblies for
each architecture:

	obj\Release\net5.0\android.21-arm\linked\System.Linq.dll
	obj\Release\net5.0\android.21-arm64\linked\System.Linq.dll
	obj\Release\net5.0\android.21-x86\linked\System.Linq.dll
	obj\Release\net5.0\android.21-x64\linked\System.Linq.dll

I created a `<ProcessAssemblies/>` MSBuild task that checks the MVID
of each assembly to detect duplicates.  Non-duplicates will be placed
in a sub-directory within the `.apk` file, such as:

  * `assemblies\HelloAndroid.dll`
  * `assemblies\System.Linq.dll`
  * `assemblies\arm64-v8a\System.Private.CoreLib.dll`
  * `assemblies\armeabi-v7a\System.Private.CoreLib.dll`
  * `assemblies\x86\System.Private.CoreLib.dll`
  * `assemblies\x86_64\System.Private.CoreLib.dll`

I had to make a few build & runtime changes for these ABI
sub-directories to work.

Other notes:

  * We must pass `@(IntermediateAssembly)` from the outer build to the
    nested `<MSBuild/>` calls.  Otherwise, the inner calls look for
    `obj\Release\net5.0\android.21-arm\HelloAndroid.dll` instead of
    `obj\Release\net5.0\HelloAndroid.dll` that actually exists.

  * `_ComputeFilesToPublishForRuntimeIdentifiers` was running some
    extra targets causing `<ResolveSdks/>` to run 5 times for 4 ABIs.
    I had to make changes in `BuildOrder.targets` to fix this.

  * `<ResolveAssemblies/>` now sets `%(IntermediateLinkerOutput)` on
    each assembly item.  This allows .NET 6 builds to support multiple
    linker directories.

[0]: https://docs.microsoft.com/en-us/dotnet/core/tools/csproj#runtimeidentifiers
[1]: https://docs.microsoft.com/en-us/dotnet/core/rid-catalog

jonathanpeppers force-pushed the dotnet-multiple-rids branch 7 times, most recently from 7521cc0 to 0d9b251 Compare June 8, 2020 20:49

jonathanpeppers mentioned this pull request Jun 9, 2020

[question] supporting multiple $(RuntimeIdentifiers) in Xamarin dotnet/sdk#11975

Closed

jonathanpeppers force-pushed the dotnet-multiple-rids branch 3 times, most recently from a915c1a to 635e99f Compare June 11, 2020 14:53

jonathanpeppers marked this pull request as ready for review June 11, 2020 14:54

jonathanpeppers requested review from dellis1972, grendello and jonpryor as code owners June 11, 2020 14:54

jonathanpeppers requested a review from radekdoulik June 11, 2020 14:54

jonathanpeppers commented Jun 11, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets Show resolved Hide resolved

dellis1972 approved these changes Jun 12, 2020

View reviewed changes

jonathanpeppers requested a review from pjcollins June 12, 2020 19:13

jonathanpeppers mentioned this pull request Jun 12, 2020

[Xamarin.Android.Build.Tasks] @(LinkDescription) should be in Inputs for the linker #4816

Merged

grendello approved these changes Jun 15, 2020

View reviewed changes

jonathanpeppers force-pushed the dotnet-multiple-rids branch from 635e99f to b25e99d Compare June 15, 2020 17:07

jonpryor reviewed Jun 16, 2020

View reviewed changes

...n.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets Show resolved Hide resolved

jonpryor reviewed Jun 16, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs Outdated Show resolved Hide resolved

jonathanpeppers force-pushed the dotnet-multiple-rids branch 2 times, most recently from 2d69d65 to 0decf45 Compare June 16, 2020 21:35

jonpryor reviewed Jun 17, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/RuntimeIdentifierToAbi.cs Outdated Show resolved Hide resolved

jonpryor reviewed Jun 17, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets Outdated Show resolved Hide resolved

jonpryor reviewed Jun 17, 2020

View reviewed changes

src/monodroid/jni/basic-android-system.cc Show resolved Hide resolved

jonathanpeppers force-pushed the dotnet-multiple-rids branch from 0decf45 to 8a1d054 Compare June 17, 2020 16:15

jonathanpeppers requested a review from brendanzagaeski as a code owner June 17, 2020 16:15

jonathanpeppers force-pushed the dotnet-multiple-rids branch from 8a1d054 to 9e1b894 Compare June 18, 2020 13:43

jonpryor reviewed Jun 18, 2020

View reviewed changes

...marin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Linker.targets Show resolved Hide resolved

jonpryor reviewed Jun 18, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs Show resolved Hide resolved

brendanzagaeski reviewed Jun 18, 2020

View reviewed changes

jonathanpeppers force-pushed the dotnet-multiple-rids branch from 9e1b894 to 4cf7fec Compare June 18, 2020 21:11

brendanzagaeski approved these changes Jun 19, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Properties/Resources.resx Outdated Show resolved Hide resolved

jonathanpeppers force-pushed the dotnet-multiple-rids branch from 4cf7fec to 6fd701c Compare June 19, 2020 14:00

jonathanpeppers force-pushed the dotnet-multiple-rids branch from 6fd701c to 28d5a02 Compare June 19, 2020 14:01

Update xa0036.md

d7c4bce

Example message should say 'warning'.

jonpryor merged commit 51fb93e into dotnet:master Jun 22, 2020

jonathanpeppers deleted the dotnet-multiple-rids branch June 22, 2020 17:12

brendanzagaeski added this to the In Preview: 11.1 (d16-8) milestone Aug 5, 2020

github-actions bot locked and limited conversation to collaborators Jan 26, 2024

[One .NET] support for multiple $(RuntimeIdentifiers) #4767

[One .NET] support for multiple $(RuntimeIdentifiers) #4767

Uh oh!

Conversation

jonathanpeppers commented Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dellis1972 commented Jun 15, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brendanzagaeski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonpryor commented Jun 22, 2020

Uh oh!

Uh oh!

jonathanpeppers commented Jun 4, 2020 •

edited

Loading