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

Update NDK to r22 #5475

Merged
merged 1 commit into from
Feb 9, 2021
Merged

Update NDK to r22 #5475

merged 1 commit into from
Feb 9, 2021

Conversation

grendello
Copy link
Contributor

@grendello grendello commented Jan 7, 2021

The most important NDK changes:

  • GNU binutils are deprecated (but still used)
  • LLVM 11 is used for the toolchain

XA changes:

  • All the binutils tools are listed in a single location, Configurables.Defaults, now
  • Host NDK binutils are installed by xaprepare instead of by Xamarin.Android.Build.Tools
  • UBSAN checked builds require RTTI and exceptions now

@grendello grendello force-pushed the ndk-update branch 2 times, most recently from 1eab28b to 130d5c9 Compare January 8, 2021 10:30
@grendello grendello marked this pull request as ready for review January 8, 2021 10:30
check_include_file_cxx("linux/if_arp.h" HAVE_LINUX_IF_ARP_H)
check_include_file("linux/netlink.h" HAVE_LINUX_NETLINK_H)
check_include_file("linux/rtnetlink.h" HAVE_LINUX_RTNETLINK_H)
check_include_file("linux/if_arp.h" HAVE_LINUX_IF_ARP_H)
Copy link
Member

Choose a reason for hiding this comment

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

Just a nitpick, excessive space.

Copy link
Member

@radekdoulik radekdoulik left a comment

Choose a reason for hiding this comment

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

LGTM once green.

@radekdoulik
Copy link
Member

Looks like it is now failing with:

CMake Error at CMakeLists.txt:85 (message):
  Please set the MINGW_DEPENDENCIES_ROOT_DIR variable on command line
  (-DMINGW_DEPENDENCIES_ROOT_DIR=PATH)
   Called from: [1]	C:/a/1/s/src/monodroid/CMakeLists.txt

@grendello
Copy link
Contributor Author

There's also the cmake policy warning which is treated as an error on Windows (dunno why). I'm about to look into those issues

@jonpryor
Copy link
Member

We may need to hold off on this PR? There are lots of failing unit tests a'la https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4360715&view=ms.vss-test-web.build-test-results-tab&runId=18012466&resultId=100080&paneView=attachments

Should have no MSBuild warnings.
Expected: True
But was:  False
…/BuildHasNoWarningsFalseTrueTrueapk/Properties/AndroidManifest.xml :
warning XA4216: AndroidManifest.xml //uses-sdk/@android:minSdkVersion '19' is less than API-21, this configuration is not supported.

It appears that NDK r22 bumps the minSdkVersion value to API-21 -- likely because it no longer has a platforms/android-19 directory -- which introducing all these warnings.

I thus fear that we can't really bump to NDK r22 until we bump our minimum to API-21…which is where things get complicated. .NET 6 will have API-21 as a minimum, but we plan on concurrently shipping "legacy" installers for some period of overlapping time, and both the "legacy" and .NET 6 installers will be coming out of this repo.

How do we sanely support that?

@grendello
Copy link
Contributor Author

We also need to wait for NDK to be fixed for us before we can switch. Regarding supporting both net6 and "legacy", the only option would be to enable building with both r21 and r22 - probably different pipelines running in parallel. It will definitely require some thought and work. I'm going to leave this PR open as a draft and we can come back to it when NDK is fixed. In the meantime we can discuss how to solve the net6 vs legacy conundrum.

@grendello grendello marked this pull request as draft January 14, 2021 21:16
@grendello grendello added the do-not-merge PR should not be merged. label Jan 14, 2021
@grendello grendello force-pushed the ndk-update branch 5 times, most recently from a062261 to 5d2debd Compare January 26, 2021 22:01
@grendello grendello force-pushed the ndk-update branch 4 times, most recently from 4732d9a to 2c5aba3 Compare January 29, 2021 11:19
@grendello
Copy link
Contributor Author

Most of the issues are fixed, there's only one of AOT + LLVM. Since NDK r22 removed $arch-ld replacing it with $arch-ld.{bfd,gold}, Mono is not able to link the AOT-d code since it hardcodes ld as the linker name :( If @marek-safar and @lambdageek OK it, I will next work on fixing that bit in Mono next week.

grendello added a commit to grendello/mono that referenced this pull request Feb 2, 2021
Context: https://github.com/android/ndk/wiki/Changelog-r22#announcements
Context: dotnet/android#5475

This commit fixes a problem where the AOT compiler hard-codes the native
linker executable name to a platform-specific value but such an
executable does not exist in the toolchain being used, thus making the
AOT compiler fail to link the final binary.

The specific use case here is the new Android NDK r22 which not only
deprecates GNU binutils, but also removes architecture-prefixed `ld`
binary (e.g. `aarch64-linux-android-ld` no longer exists). Instead, the
NDK provides a non-prefixed `ld` binary and two prefixed ones:
`$arch-ld.gold` and `$arch-ld.bfd`. However, since the AOT compiler
hardcodes `ld` as the linker name on Linux systems, the AOT compilation
fails when attempting to link the final executable. Which, in turn,
breaks some Xamarin.Android AOT tests.

This commit fixes the issue by adding a new `ld-name` option to the AOT
compiler allowing one to specify just the name of the linker binary to
use. The rest of the toolchain mechanics doesn't change.
@grendello
Copy link
Contributor Author

I fixed all AOT failures, but they won't work in this PR until mono/mono#20816 is in and we can update Mono to one with the fix.

grendello added a commit to grendello/runtime that referenced this pull request Feb 2, 2021
Context: https://github.com/android/ndk/wiki/Changelog-r22#announcements
Context: dotnet/android#5475
Context: mono/mono#20814

This commit fixes a problem where the AOT compiler hard-codes the native
linker executable name to a platform-specific value but such an
executable does not exist in the toolchain being used, thus making the
AOT compiler fail to link the final binary.

The specific use case here is the new Android NDK r22 which not only
deprecates GNU binutils, but also removes architecture-prefixed `ld`
binary (e.g. `aarch64-linux-android-ld` no longer exists). Instead, the
NDK provides a non-prefixed `ld` binary and two prefixed ones:
`$arch-ld.gold` and `$arch-ld.bfd`. However, since the AOT compiler
hardcodes `ld` as the linker name on Linux systems, the AOT compilation
fails when attempting to link the final executable. Which, in turn,
breaks some Xamarin.Android AOT tests.

This commit fixes the issue by adding a new `ld-name` option to the AOT
compiler allowing one to specify just the name of the linker binary to
use. The rest of the toolchain mechanics doesn't change.
marek-safar pushed a commit to mono/mono that referenced this pull request Feb 3, 2021
Context: https://github.com/android/ndk/wiki/Changelog-r22#announcements
Context: dotnet/android#5475

This commit fixes a problem where the AOT compiler hard-codes the native
linker executable name to a platform-specific value but such an
executable does not exist in the toolchain being used, thus making the
AOT compiler fail to link the final binary.

The specific use case here is the new Android NDK r22 which not only
deprecates GNU binutils, but also removes architecture-prefixed `ld`
binary (e.g. `aarch64-linux-android-ld` no longer exists). Instead, the
NDK provides a non-prefixed `ld` binary and two prefixed ones:
`$arch-ld.gold` and `$arch-ld.bfd`. However, since the AOT compiler
hardcodes `ld` as the linker name on Linux systems, the AOT compilation
fails when attempting to link the final executable. Which, in turn,
breaks some Xamarin.Android AOT tests.

This commit fixes the issue by adding a new `ld-name` option to the AOT
compiler allowing one to specify just the name of the linker binary to
use. The rest of the toolchain mechanics doesn't change.
marek-safar pushed a commit to mono/mono that referenced this pull request Feb 3, 2021
Context: https://github.com/android/ndk/wiki/Changelog-r22#announcements
Context: dotnet/android#5475

This commit fixes a problem where the AOT compiler hard-codes the native
linker executable name to a platform-specific value but such an
executable does not exist in the toolchain being used, thus making the
AOT compiler fail to link the final binary.

The specific use case here is the new Android NDK r22 which not only
deprecates GNU binutils, but also removes architecture-prefixed `ld`
binary (e.g. `aarch64-linux-android-ld` no longer exists). Instead, the
NDK provides a non-prefixed `ld` binary and two prefixed ones:
`$arch-ld.gold` and `$arch-ld.bfd`. However, since the AOT compiler
hardcodes `ld` as the linker name on Linux systems, the AOT compilation
fails when attempting to link the final executable. Which, in turn,
breaks some Xamarin.Android AOT tests.

This commit fixes the issue by adding a new `ld-name` option to the AOT
compiler allowing one to specify just the name of the linker binary to
use. The rest of the toolchain mechanics doesn't change.
marek-safar pushed a commit to dotnet/runtime that referenced this pull request Feb 3, 2021
Context: https://github.com/android/ndk/wiki/Changelog-r22#announcements
Context: dotnet/android#5475
Context: mono/mono#20814

This commit fixes a problem where the AOT compiler hard-codes the native
linker executable name to a platform-specific value but such an
executable does not exist in the toolchain being used, thus making the
AOT compiler fail to link the final binary.

The specific use case here is the new Android NDK r22 which not only
deprecates GNU binutils, but also removes architecture-prefixed `ld`
binary (e.g. `aarch64-linux-android-ld` no longer exists). Instead, the
NDK provides a non-prefixed `ld` binary and two prefixed ones:
`$arch-ld.gold` and `$arch-ld.bfd`. However, since the AOT compiler
hardcodes `ld` as the linker name on Linux systems, the AOT compilation
fails when attempting to link the final executable. Which, in turn,
breaks some Xamarin.Android AOT tests.

This commit fixes the issue by adding a new `ld-name` option to the AOT
compiler allowing one to specify just the name of the linker binary to
use. The rest of the toolchain mechanics doesn't change.
@grendello grendello force-pushed the ndk-update branch 5 times, most recently from d35ac3c to 05c942e Compare February 4, 2021 11:56
@grendello grendello marked this pull request as ready for review February 4, 2021 15:10
@grendello
Copy link
Contributor Author

We are ready for NDK r22 :) Now we need to wait for new NDK release which fixes the detection issue

@grendello grendello force-pushed the ndk-update branch 2 times, most recently from e3f849d to 43fb082 Compare February 8, 2021 09:09
@grendello grendello removed the do-not-merge PR should not be merged. label Feb 8, 2021
Context: https://github.com/android/ndk/wiki/Changelog-r22

The most important NDK changes:
  * GNU binutils are deprecated (but still used)
  * LLVM 11 is used for the toolchain
  * LLD is now the default linker

XA changes:
  * All the binutils tools are listed in a single location, `Configurables.Defaults`, now
  * Host NDK binutils are installed by xaprepare instead of by Xamarin.Android.Build.Tools
  * `UBSAN` checked builds require `RTTI` and exceptions now
@jonpryor jonpryor merged commit accc846 into dotnet:master Feb 9, 2021
@grendello grendello deleted the ndk-update branch February 9, 2021 20:00
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants