[vcpkg] Enhancement android triplet by holmesconan · Pull Request #21847 · microsoft/vcpkg

holmesconan · 2021-12-04T10:04:44Z

fix vcpkg_configure_make for Android newest NDK.
fix $ANDROID_PLATFORM set in $VCPKG_ROOT/scripts/toolchains/android.cmake by parse the Android NDK revision file.

What does your PR fix?

Fixes New Android NDK support request #21345 [android] Android NDK r23 and clang support #20844 [icu:arm-android] build failure #18941
Which triplets are supported/not supported? Have you updated the CI baseline?

android, No
Does your PR follow the maintainer guide?

Yes

…iles.

holmesconan · 2021-12-04T10:06:38Z

cc @dg0yt @m-kuhn

m-kuhn · 2021-12-04T10:08:06Z

See also #21831

dg0yt · 2021-12-04T10:15:14Z

Too many PRs on vcpkg_configure_make at the same time...

m-kuhn · 2021-12-04T10:37:32Z

If #21831 is acceptable / has the potential to be merged, I'd propose to focus on that one because it's smaller and other things from in here can follow up. If #21831 is oversimplified, I'm also very open to let it go.

dg0yt · 2021-12-04T10:55:28Z

In any case, I don't want to repeat PR comments.
ATM it is not even clear to me if the CMake-based ports (e.g. zlib (C), geos (C++)) build with the right build flags for the desired spectrum of host platforms and of Android NDKs and platforms. Anything broken there can't be fixed in vcpkg_configure_make.

m-kuhn · 2021-12-04T13:54:06Z

ATM it is not even clear to me if the CMake-based ports (e.g. zlib (C), geos (C++)) build with the right build flags for the desired spectrum of host platforms and of Android NDKs and platforms. Anything broken there can't be fixed in vcpkg_configure_make.

I'm building those successfully with an older version branched off #15605

Actually the complete gdal stack + some qt based libs with precompiled qt libraries.

dg0yt · 2021-12-04T14:13:28Z

I'm building those successfully with an older version branched off #15605

Actually the complete gdal stack + some qt based libs with precompiled qt libraries.

I understand this ("branched") as:
CMake-based ports (e.g. zlib (C), geos (C++)) don't build with the right build flags ATM, at least for some hosts or Android targets. This must be fixed first.

m-kuhn · 2021-12-04T14:34:17Z

I didn't specifically patch anything for cmake base ports. The cumbersome ones were always the others. If I had to guess I'd say they are fine already but never put this on test.

dg0yt · 2021-12-04T15:36:09Z

The build or install log files should reflect the android API level and the chosen C++ library.

m-kuhn · 2021-12-04T16:35:02Z

Here's a geos build log produced with #21831 , default community triplet arm64-android and qbsbuild/qbsdev:focal-android-ndk-r21e-0 .
--target=aarch64-none-linux-android21 corresponds to the default api level (21).

…CTED_CMAKE_(C/CXX)_FLAGS_* instead

holmesconan · 2021-12-06T03:33:42Z

Too many PRs on vcpkg_configure_make at the same time...

Sorry for that. Since @m-kuhn did not mention my issue, I don't know he/she has already made a PR. But after testint that PR, I thought this one should be the primary solution to work with...

holmesconan · 2021-12-06T03:47:25Z

I'm building those successfully with an older version branched off #15605
Actually the complete gdal stack + some qt based libs with precompiled qt libraries.

I understand this ("branched") as: CMake-based ports (e.g. zlib (C), geos (C++)) don't build with the right build flags ATM, at least for some hosts or Android targets. This must be fixed first.

I have read the $VCPKG_ROOT/scripts/toolchains/android.cmake and checked it with the Android NDK documentation. It should say that except the weird $ANDROID_PLATFORM and $ANDROID_NATIVE_API_LEVEL which is chaged from reversion to reversion, anything else matched the guidance of Android NDK usage. And this PR contains a small patch that parses the Android NDK reversion file to determine the actual NDK version instead of the CMAKE_SYSTEM_VERSION. That could refine the $ANDROID_PLATFORM setting more precisely.

scripts/cmake/vcpkg_configure_make.cmake

m-kuhn · 2021-12-06T06:48:26Z

I'm building those successfully with an older version branched off #15605
Actually the complete gdal stack + some qt based libs with precompiled qt libraries.

I understand this ("branched") as: CMake-based ports (e.g. zlib (C), geos (C++)) don't build with the right build flags ATM, at least for some hosts or Android targets. This must be fixed first.

I have read the $VCPKG_ROOT/scripts/toolchains/android.cmake and checked it with the Android NDK documentation. It should say that except the weird $ANDROID_PLATFORM and $ANDROID_NATIVE_API_LEVEL which is chaged from reversion to reversion, anything else matched the guidance of Android NDK usage. And this PR contains a small patch that parses the Android NDK reversion file to determine the actual NDK version instead of the CMAKE_SYSTEM_VERSION. That could refine the $ANDROID_PLATFORM setting more precisely.

IIUC this is independent from the rest of the work in here.
Would it make sense to create a separate PR and discuss it in the scope of #21465 ?

dg0yt · 2021-12-06T09:01:31Z

scripts/cmake/vcpkg_configure_make.cmake

+    if(VCPKG_DETECTED_${cmake_var})
+        get_filename_component(cmd_PATH "${VCPKG_DETECTED_${cmake_var}}" DIRECTORY)
+        get_filename_component(cmd_PROGRAM "${VCPKG_DETECTED_${cmake_var}}" NAME)
+        set(ENV{${env_name}} "${cmd_PROGRAM}")


Why not set ENV{${env_name}} to the full path?

It is just some kind of OCD problem, :-(. Just make the log looks good instead of run well.

IMO changing PATH invites side effects. Program name lookup after updating the path will behave different. Apart from the PATH becoming longer and longer. And this isn't even visible in most logs.

m-kuhn · 2021-12-07T07:58:14Z

If #21831 is acceptable / has the potential to be merged, I'd propose to focus on that one because it's smaller and other things from in here can follow up. If #21831 is oversimplified, I'm also very open to let it go.

@dg0yt @holmescn @JackBoosY Given the recent increased communication in this PR I wonder if I should close mine. I am happy to help here too. Opinions?

scripts/cmake/vcpkg_configure_make.cmake

JackBoosY · 2022-03-29T02:03:41Z

Can you please merge to master and resolve the file conflict?

…ncement-android-triplet

holmesconan · 2022-03-30T03:39:49Z

@jwtowner the new PR won't append the ANDROID_NATIVE_API_LEVEL when it is there, could you please test it?
@m-kuhn the controversial code is disabled but not removed. I remember it may trigger some errors, but you can test it. If no more errors, I could remove it later.
@JackBister files merged.

jwtowner · 2022-03-30T17:51:19Z

@jwtowner the new PR won't append the ANDROID_NATIVE_API_LEVEL when it is there, could you please test it?

I can confirm it works. Thanks.

m-kuhn · 2022-03-31T15:41:58Z

Successfully built with r21e

nirvn · 2022-04-07T10:49:41Z

@holmescn , FYI, when trying to build using this PR against Android NDK r23 (23.1.7779620), the following error is thrown while configuring libiconv:

configure:4212: checking whether the C compiler works
configure:4234: /usr/local/lib/android/sdk/ndk/23.1.7779620/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -fPIC --target=aarch64-none-linux-android  -avoid-version -L/home/runner/builddir/vcpkg_installed/arm64-android/lib -L/home/runner/builddir/vcpkg_installed/arm64-android/lib/manual-link  conftest.c -latomic -lm >&5
clang-12: warning: argument unused during compilation: '-avoid-version' [-Wunused-command-line-argument]
ld: error: cannot open crtbegin_dynamic.o: No such file or directory
ld: error: cannot open crtend_android.o: No such file or directory
clang-12: error: linker command failed with exit code 1 (use -v to see invocation)

JackBoosY · 2022-04-11T02:27:52Z

Ping @holmescn for reply.

holmesconan · 2022-04-13T10:28:33Z

@holmescn , FYI, when trying to build using this PR against Android NDK r23 (23.1.7779620), the following error is thrown while configuring libiconv:

configure:4212: checking whether the C compiler works
configure:4234: /usr/local/lib/android/sdk/ndk/23.1.7779620/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -fPIC --target=aarch64-none-linux-android  -avoid-version -L/home/runner/builddir/vcpkg_installed/arm64-android/lib -L/home/runner/builddir/vcpkg_installed/arm64-android/lib/manual-link  conftest.c -latomic -lm >&5
clang-12: warning: argument unused during compilation: '-avoid-version' [-Wunused-command-line-argument]
ld: error: cannot open crtbegin_dynamic.o: No such file or directory
ld: error: cannot open crtend_android.o: No such file or directory
clang-12: error: linker command failed with exit code 1 (use -v to see invocation)

@nairaner libiconv seems hacked the autotools scripts, you need an overlay port to make it works. I have an overlay port here

Beside the overlay port, you need to comment $VCPKG_ROOT/scripts/cmake/vcpkg_configure_make.cmake line 767, where add the -avoid-version option caused configuration failed.

strega-nil-ms · 2022-04-14T14:50:31Z

I need to see resolutions for the comments I made before this is marked info:reviewed; to be clear, that means:

the if(VCPKG_TARGET_IS_OSX) line needs to be explained
z_vcpkg_setup_detected_env needs to be a function
VCPKG_ANDROID_NATIVE_API_LEVEL needs to not be an environment variable set in the triplet

JackBoosY · 2022-05-07T06:25:52Z

Ping for response.

JackBoosY · 2022-06-17T08:34:52Z

Ping @holmescn again.

m-kuhn · 2022-06-24T09:23:13Z

#25009 implements a core part of this PR and answers the question about the OSX exception raised here https://github.com/microsoft/vcpkg/pull/21847/files?w=1#r776056939 while #25201 tackles the root problem of the OSX exception.

JackBoosY · 2022-07-15T05:36:41Z

Convert this PR to draft since there is no progress.

holmesconan added 3 commits December 4, 2021 07:22

add VCPKG_ANDROID_NATIVE_API_LEVEL environment variable into triple f…

c279bf2

…iles.

fix: ANDROID_PLATFORM

70b610f

fix: vcpkg_configure_make cross-compile for Android.

b92e645

JonLiu1993 assigned JackBoosY Dec 6, 2021

JackBoosY added the category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. label Dec 6, 2021

fix: remove CFLAGS_target, CXXFLAGS_target, just patch the VCPKG_DETE…

d3e7ecd

…CTED_CMAKE_(C/CXX)_FLAGS_* instead

holmesconan added 2 commits December 6, 2021 04:49

fix: check command PATH exists in current environment

32fe1d9

fix: regression error

44b16b8

dg0yt reviewed Dec 6, 2021

View reviewed changes

scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved

dg0yt reviewed Dec 6, 2021

View reviewed changes

PhoebeHui changed the title ~~Enhancement android triplet~~ [vcpkg] Enhancement android triplet Dec 6, 2021

holmesconan mentioned this pull request Dec 6, 2021

[android] support "make" based ports #21831

Closed

holmesconan added 2 commits December 7, 2021 02:12

fix: ensure the toolchain path is the first

47cb665

fix: #21465 based on the NDK version

fa44824

JackBoosY suggested changes Dec 7, 2021

View reviewed changes

scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved

JackBoosY added the requires:author-response label Dec 7, 2021

holmesconan added 3 commits March 30, 2022 11:17

Merge branch 'master' of https://github.com/microsoft/vcpkg into enha…

759f0f2

…ncement-android-triplet

disabled the controversial code

199b0d4

don't append ANDROID_NATIVE_API_LEVEL when it is there.

5d3e0e8

github-actions bot approved these changes Mar 31, 2022

View reviewed changes

JackBoosY added the requires:author-response label Apr 1, 2022

JackBoosY added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Apr 14, 2022

strega-nil-ms added requires:author-response and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Apr 14, 2022

JackBoosY linked an issue May 10, 2022 that may be closed by this pull request

[icu:arm-android] build failure #18941

Closed

Neumann-A mentioned this pull request Jun 9, 2022

[jemalloc|vcpkg_configure_make] update and some script fixes #25009

Merged

JackBoosY added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. requires:author-response and removed requires:author-response labels Jun 13, 2022

ras0219-msft dismissed JackBoosY’s stale review via 5d3e0e8 June 23, 2022 19:58

JackBoosY removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jul 15, 2022

JackBoosY marked this pull request as draft July 15, 2022 05:36

m-kuhn mentioned this pull request Aug 28, 2022

[vcpkg script] Add support for Android to make and meson #26570

Closed

This pull request was closed.

Conversation

holmesconan commented Dec 4, 2021 • edited by JackBoosY Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

Uh oh!

holmesconan commented Dec 4, 2021

Uh oh!

m-kuhn commented Dec 4, 2021

Uh oh!

dg0yt commented Dec 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m-kuhn commented Dec 4, 2021

Uh oh!

dg0yt commented Dec 4, 2021

Uh oh!

m-kuhn commented Dec 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg0yt commented Dec 4, 2021

Uh oh!

m-kuhn commented Dec 4, 2021

Uh oh!

dg0yt commented Dec 4, 2021

Uh oh!

m-kuhn commented Dec 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

holmesconan commented Dec 6, 2021

Uh oh!

holmesconan commented Dec 6, 2021

Uh oh!

Uh oh!

m-kuhn commented Dec 6, 2021

Uh oh!

dg0yt Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

holmesconan Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

dg0yt Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

m-kuhn commented Dec 7, 2021

Uh oh!

Uh oh!

JackBoosY commented Mar 29, 2022

Uh oh!

holmesconan commented Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwtowner commented Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m-kuhn commented Mar 31, 2022

Uh oh!

nirvn commented Apr 7, 2022

Uh oh!

JackBoosY commented Apr 11, 2022

Uh oh!

holmesconan commented Apr 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

strega-nil-ms commented Apr 14, 2022

Uh oh!

JackBoosY commented May 7, 2022

Uh oh!

JackBoosY commented Jun 17, 2022

Uh oh!

m-kuhn commented Jun 24, 2022

Uh oh!

JackBoosY commented Jul 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

holmesconan commented Dec 4, 2021 •

edited by JackBoosY

Loading

dg0yt commented Dec 4, 2021 •

edited

Loading

m-kuhn commented Dec 4, 2021 •

edited

Loading

m-kuhn commented Dec 4, 2021 •

edited

Loading

holmesconan commented Mar 30, 2022 •

edited

Loading

jwtowner commented Mar 30, 2022 •

edited

Loading

holmesconan commented Apr 13, 2022 •

edited

Loading