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 to Android NDK r19c #8992

Closed
wants to merge 4 commits into from

Conversation

AxelNennker
Copy link

Signed-off-by: Axel Nennker axel@nennker.de

Update NOTES.Android from Android NDK 10 to the current r19c.

  • Android is about to deprecate gcc in favor of clang
  • All toolchains are now standalone
  • Compiler names now contain the Android API-level in their name e.g. aarch64-linux-android28-clang
  • Developers building for Android usually have Android Studio installed which has a build-in NDK in $HOME/Android/Sdk/ndk-bundle

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label May 26, 2019
@levitte
Copy link
Member

levitte commented May 26, 2019

Uhmmmm, seems like you sucked in a few other PRs...

Signed-off-by: Axel Nennker <axel.nennker@telekom.de>
Signed-off-by: Axel Nennker <axel.nennker@telekom.de>
@AxelNennker
Copy link
Author

oops

Uhmmmm, seems like you sucked in a few other PRs...

fixed that, thank you

@slontis
Copy link
Member

slontis commented May 27, 2019

@levitte Should this still retain some info about how to build using an older toolchain?

@slontis
Copy link
Member

slontis commented May 27, 2019

@paulidale is this considered trivial for CLA purposes?

Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

I would not consider this trivial. A CLA will be required.

@slontis
Copy link
Member

slontis commented May 27, 2019

NOTES.ANDROID Outdated
HOST_TAG=linux-x86_64 \
PATH=$NDK/toolchains/llvm/prebuilt/$HOST_TAG/bin:$PATH \
CC=$NDK/toolchains/llvm/prebuilt/$HOST_TAG/bin/aarch64-linux-android28-clang \
CXX=$NDK/toolchains/llvm/prebuilt/$HOST_TAG/bin/aarch64-linux-android28-clang++ \
Copy link
Member

Choose a reason for hiding this comment

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

Why these two? I thought clang selected its target architecture with something like -target aarch64-linux-android28?, which our android target code already handles... I understand that you want clang, but in that case, shouldn't the following be enough?

    CC=clang CXX=clang++

Copy link
Author

Choose a reason for hiding this comment

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

I think the target is without the API level aarch64-linux-android but the name of the clang compiler has now the 28 in it. If you use the ndk-bundle there is no aarch64-linux-android-clang but aarch64-linux-android16-clang to aarch64-linux-android29-clang.
If you use make-standalone-toolchain.sh the aarch64-linux-android-clang wrapper script(s) are created.
Regarding 'why not just set clang and clang++?': Configure complains if unprefixed tools are used, if I remember correctly. Also Configure falls back to gcc quite fast and so best to state what should be used.

Discussion about gcc deprecation is here: android/ndk#26
gcc deprecation timeline: https://android.googlesource.com/platform/prebuilts/clang/host/linux-x86/+/master/GCC_4_9_DEPRECATION.md

Copy link
Member

Choose a reason for hiding this comment

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

There's code in Configurations/15-android.conf that goes to quite some length to make it easier. Maybe it needs adapting?

Copy link
Author

Choose a reason for hiding this comment

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

This PR is for the NOTES only. Yes, I think it makes sense to change the Android conf as well.
I wanted to keep this small for now and do the other steps later.

Copy link
Author

Choose a reason for hiding this comment

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

I have sent the CLA to openssl legal. What is the way forward?

@petrovr
Copy link

petrovr commented Jun 2, 2019 via email

@AxelNennker
Copy link
Author

AxelNennker commented Jun 3, 2019

Axel Nennker wrote:
AxelNennker commented on this pull request. > - export ANDROID_NDK_HOME=/some/where/android-ndk-10d - PATH=$ANDROID_NDK_HOME/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/bin:$PATH - ./Configure android-arm -D__ANDROID_API__=14 + For example, to compile for target aarch64-linux-android using NDK r19c: + + export ANDROID_NDK_HOME=$HOME/Android/Sdk/ndk-bundle + NDK=$ANDROID_NDK_HOME \ + HOST_TAG=linux-x86_64 \ + PATH=$NDK/toolchains/llvm/prebuilt/$HOST_TAG/bin:$PATH \ + CC=$NDK/toolchains/llvm/prebuilt/$HOST_TAG/bin/aarch64-linux-android28-clang \ + CXX=$NDK/toolchains/llvm/prebuilt/$HOST_TAG/bin/aarch64-linux-android28-clang++ \ This PR is for the NOTES only. Yes, I think it makes sense to change the Android conf as well. I wanted to keep this small for now and do the other steps later.
Above does not look acceptable. It is enough PATH to to be : PATH=$ANDROID_NDK_HOME/toolchains/llvm/prebuilt/linux-x86_64/bin:$PATH , without all those extra hacks. Roumen

I added e.g. HOST_TAG to make it clear you have to change that if you'r developing on e.g. darwin_x86_64. Anyway, I did what you requested and added a line explaining what needs to be done if linux_x86_64 does not work.

CLA is sent to legal. Who removes the cla-needed flag?

@petrovr
Copy link

petrovr commented Jun 3, 2019 via email

@AxelNennker
Copy link
Author

Axel Nennker wrote:
I added e.g. HOST_TAG to make it clear the you have to change that if you'r developing on e.g. darwin_x86_64. Anyway, I did what you requested and added a line explaining what needs to be done if linux_x86_64 does not work.
Hmm darwin_x86_64 or linux_x86_64. I cannot see any relation between those targets and Android targets. So explanation makes no sense. Roumen

These are not the targets but where the binaries on the host system are.
Developers developing for Android usually install Android Studio which installs the Android SDK into $HOME/Android/Sdk
Depending on the host system different binaries are installed. On my Ubuntu system the binaries are in ~/Android/Sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/.... I think that on a MacOS system the binaries are in ~/Android/Sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64.
That is my interpretation of the description of the standalone toolchains directory structure
https://developer.android.com/ndk/guides/other_build_systems
That is why I initially had the HOST_TAG environment variable in this PR.

@mattcaswell
Copy link
Member

Close/reopen to kick CLA bot

@mattcaswell mattcaswell reopened this Jul 17, 2019
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Jul 17, 2019
@mattcaswell
Copy link
Member

@levitte do you have further comments on this?

Signed-off-by: Axel Nennker <axel.nennker@telekom.de>
@eighthave
Copy link

I'd love to see this move forward and get merged. What is here is better than what currently exists. I think Configurations/15-android.conf could be simplified a lot if support for the really old stuff was removed, e.g. the standalone NDK and NDK versions older than 16 or so. Both are long not supported.

For example:

--- a/Configurations/15-android.conf
+++ b/Configurations/15-android.conf
@@ -8,7 +8,7 @@
 
     my $android_ndk = {};
     my %triplet = (
-        arm    => "arm-linux-androideabi",
+        arm    => "armv7a-linux-androideabi",
         arm64  => "aarch64-linux-android",
         mips   => "mipsel-linux-android",
         mips64 => "mips64el-linux-android",
@@ -39,21 +39,27 @@
             $ndk = canonpath($ndk);
 
             my $ndkver = undef;
+            my $ndkbin = undef;
 
             if (open my $fh, "<$ndk/source.properties") {
                 local $_;
                 while(<$fh>) {
-                    if (m|Pkg\.Revision\s*=\s*([0-9]+)|) {
+                    if (m|^Pkg\.Revision\s*=\s*([0-9]+)|) {
                         $ndkver = $1;
                         last;
                     }
                 }
                 close $fh;
+                die "$ndk/source.properties is missing or corrupt!" if (!$ndkver);
+                if ($ndkver >= 19) {
+                    $ndkbin = "$ndk/toolchains/llvm/prebuilt/linux-x86_64/bin";
+                    $ENV{'PATH'} = "$ndkbin:$ENV{'PATH'}";
+                }
             }
 
             my ($sysroot, $api, $arch);
 
             $config{target} =~ m|[^-]+-([^-]+)$|;      # split on dash
             $arch = $1;
 
             if ($sysroot = $ENV{CROSS_SYSROOT}) {
@@ -88,8 +94,13 @@
             my $cflags;
             my $cppflags;
 
-            # see if there is NDK clang on $PATH, "universal" or "standalone"
-            if (which("clang") =~ m|^$ndk/.*/prebuilt/([^/]+)/|) {
+            if (which("$triarch$api-clang") =~ m|^$ndk/.*/prebuilt/([^/]+)/|) {
+               my $ndkbinbase = "$ndkbin/$triarch";
+               $user{AR} = "$ndkbinbase-ar";
+               $user{CC} = "$ndkbinbase$api-clang";
+               $user{CXX} = "$ndkbinbase$api-clang++";
+               $user{RANLIB} = "$ndkbinbase-ranlib";
+            } elsif (which("clang") =~ m|^$ndk/.*/prebuilt/([^/]+)/|) { # see if there is NDK clang on $PATH, "universal" or "standalone"

(I just emailed in my CLA).

@eighthave
Copy link

And some links to info about things to clean up which could happen in this pull request or later:

@openssl-machine
Copy link
Collaborator

This issue is in a state where it requires action by @openssl/committers but the last update was 258 days ago

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

Successfully merging this pull request may close these issues.

None yet

8 participants