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

[question] Android's implementation of multiple $(RuntimeIdentifiers) #14359

Closed
jonathanpeppers opened this issue Oct 28, 2020 · 8 comments
Closed
Assignees
Milestone

Comments

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Oct 28, 2020

This is related to: #9795

Android has the requirement of building multiple architecture .apk or .aab files. Using our Android workload, you can put the following in your .csproj file:

<RuntimeIdentifiers>android.21-arm;android.21-arm64;android.21-x86;android.21-x64</RuntimeIdentifiers>

In fact, if you omit $(RuntimeIdentifiers) (singular or plural), we default to android.21-arm64;android.21-x86. This allows the project to run on most devices and emulators.

The way we implemented this was somewhat complicated, and required several "hacks".

After Build, we call a target that depends on ResolveReferences;ComputeFilesToPublish:

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

We then use @(ResolvedFileToPublish) to do other post-ILLink work, and produce the .apk or .aab file.

Some examples of "hacks" and problems we have to workaround

One issue is $(IntermediateOutputPath) is obj\Debug\net5.0-android in the outer build, and the $(RuntimeIdentifier) is appended in the inner builds.

  • We have to prevent IncrementalClean from running in the inner builds:
<CoreBuildDependsOn>
    $([MSBuild]::Unescape($(CoreBuildDependsOn.Replace('IncrementalClean;', ''))))
</CoreBuildDependsOn>

Because $(IntermediateOutputPath) changes, potentially many files would get deleted.

  • We have to fix up the @(IntermediateAssembly) and @(_DebugSymbolsIntermediatePath) item groups:
<ItemGroup>
    <IntermediateAssembly Remove="@(IntermediateAssembly)" />
    <IntermediateAssembly Include="$(_OuterIntermediateAssembly)" />
</ItemGroup>
<ItemGroup Condition=" '@(_DebugSymbolsIntermediatePath->Count())' != '0' ">
    <_DebugSymbolsIntermediatePath Remove="@(_DebugSymbolsIntermediatePath)" />
    <_DebugSymbolsIntermediatePath Include="$([System.IO.Path]::ChangeExtension ($(_OuterIntermediateAssembly), '.pdb'))" />
</ItemGroup>
  • @(ResolvedFileToPublish) contains many .NET assemblies that are completely duplicate. In fact, I had to write an MSBuild task that looks at the MVID of each assembly and deduplicate them.

See the full implementation here.

Is there a better way to accomplish what we are trying to do? Should there be some new feature in dotnet/sdk that would also address #9795?

Thanks!

/cc @dsplaisted @jonpryor

@dsplaisted dsplaisted added this to the 6.0.1xx milestone Oct 28, 2020
@dsplaisted
Copy link
Member

What assets do you need from the RID-specific builds? Is it just the contents of the runtime packs? If so it may be a lot simpler to call the ResolveRuntimePackAssets task directly rather than having inner builds with the RuntimeIdentifier specified.

@jonathanpeppers
Copy link
Member Author

We also need to run ILLink, but I'll give <ResolveRuntimePackAssets/> a try.

There is some differences in dotnet build vs dotnet publish for us:

  • build needs to produce something runnable, like an .apk file -- this means we run ILLink during build
  • publish doesn't really do anything besides copying files to $(PublishDir), the thought was we might extend it in the future to upload builds to app stores, re-sign, etc.

I think for a .NET 5 console app, only dotnet publish would run ILLink.

@dsplaisted
Copy link
Member

Do you have to run ILLink separately for each RuntimeIdentifier?

What is the structure of the output that you want? Do you want separate output folders for each RuntimeIdentifier, or a single folder that has all of the assets for all of the RuntimeIdentifiers?

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Oct 28, 2020

Yes, we are running ILLink N times right now.

What we need in the end is the layout of the .apk file:

assemblies\Foo.dll
assemblies\System.IO.dll
assemblies\armeabi-v7a\System.Private.CoreLib.dll
assemblies\arm64-v8a\System.Private.CoreLib.dll
assemblies\x86\System.Private.CoreLib.dll
assemblies\x86_64\System.Private.CoreLib.dll

Where Foo.dll and System.IO.dll are identical for all RIDs / architectures, but System.Private.CoreLib.dll is unique per RID / architecture.

We put architecture-specific assemblies in a folder that matches the Android abi name. At runtime, we have native code that looks for assemblies in the specific subdirectory.

We could have used $(RuntimeIdentifier) instead here, but we don't have this concept in our native code. It also matches how native libraries look:

lib\arm64-v8a\libSystem.IO.Compression.Native.so
lib\arm64-v8a\libSystem.Native.so
lib\arm64-v8a\libmonosgen-2.0.so
lib\x86\libSystem.IO.Compression.Native.so
lib\x86\libSystem.Native.so
lib\x86\libmonosgen-2.0.so

@jonathanpeppers
Copy link
Member Author

I forgot we had a related discussion before: #11975

@marek-safar
Copy link
Contributor

Not only ILLink needs to run for each RID also native linker should run for each RID

@dsplaisted
Copy link
Member

@jonathanpeppers, this is an old issue, is there still more needed here or any open questions? Or should we close this?

@jonathanpeppers
Copy link
Member Author

We can close this, I'm not sure if there are any plans for the dotnet/sdk to support multiple RIDs.

If that ever comes, we should use the new built-in thing instead of the solution we have now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants