Change assembler to clang in android MonoAOT by jkurdek · Pull Request #110393

Change assembler to clang in android MonoAOT by jkurdek · Pull Request #110393 · dotnet/runtime

Conversation

Android NDK 27 does not ship with an assembler. The recommendation is to use clang. This PR makes MonoAOT compiler use clang assembler when building android.

Related PR bumping version of NDK to 27: dotnet/dotnet-buildtools-prereqs-docker#1278

Changes were verified here: #110196

This was referenced

Dec 4, 2024

/azp run runtime-android, runtime-androidemulator

Azure Pipelines successfully started running 2 pipeline(s).

/azp run runtime-android, runtime-androidemulator

Azure Pipelines successfully started running 2 pipeline(s).

		<AndroidLibraryMinApiLevel Condition="'$(AndroidLibraryMinApiLevel)' == ''">21</AndroidLibraryMinApiLevel>
		</PropertyGroup>

		<NdkToolFinderTask

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you happen to test library mode with this change?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 :) ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had an offline discussion with @steveisok about this. We agree that this should be handled as a follow up: #110757

This was referenced

Dec 5, 2024

Android fails are unrelated :)

Comment on lines +115 to +126

		<PropertyGroup>
		<_Triple Condition="'$(TargetArchitecture)' == 'arm'">armv7-linux-gnueabi</_Triple>
		<_Triple Condition="'$(TargetArchitecture)' == 'arm64'">aarch64-linux-android</_Triple>
		<_Triple Condition="'$(TargetArchitecture)' == 'x86'">i686-linux-android</_Triple>
		<_Triple Condition="'$(TargetArchitecture)' == 'x64'">x86_64-linux-android</_Triple>
		</PropertyGroup>

		<PropertyGroup>
		<_LdName>clang</_LdName>
		<_LdOptions>-fuse-ld=lld</_LdOptions>
		<_AsName>clang</_AsName>
		</PropertyGroup>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extract these settings to a common .props file so that we don't specify them in the AndroidSampleApp again?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

		<AndroidLibraryMinApiLevel Condition="'$(AndroidLibraryMinApiLevel)' == ''">21</AndroidLibraryMinApiLevel>
		</PropertyGroup>

		<NdkToolFinderTask

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 :) ?

When testing, did we run runtime-extra-platforms?
Just want to make sure that running runtime-android and runtime-androidemulator have the same coverage?

/azp run runtime-extra-platforms

Azure Pipelines successfully started running 1 pipeline(s).

/azp run runtime-extra-platforms, runtime-android, runtime-androidemulator

Azure Pipelines successfully started running 3 pipeline(s).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

PS Having a sample app reuse AndroidBuild.targets configuration can be done in a separate PR

/ba-g test failures unrelated, android lines pass

/backport to release/9.0-staging

/backport to release/8.0-staging

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels