[android] vcpkg_configure_make correct set flags for android build #15605

xandox · 2021-01-12T15:20:15Z

vcpkg_configure_make properly set CFLAGS, CXXFLAGS, LDFLAG, CC and CXX env variables now for android cross compilation.

Which triplets are supported/not supported? Mostly android triplets,
Have you updated the CI baseline? - no
Does your PR follow the maintainer guide?
yes

scripts/cmake/vcpkg_configure_make.cmake

scripts/get_cmake_vars/CMakeLists.txt

…e_crosscompiling_android

JackBoosY · 2021-01-18T02:14:43Z

Regression when building mlpack:x86-windows:

armadillo.lib(wrapper2.cpp.obj) : error LNK2019: unresolved external symbol _sgetrf_ referenced in function _wrapper2_sgetrf_
armadillo.lib(wrapper2.cpp.obj) : error LNK2019: unresolved external symbol _dgetrf_ referenced in function _wrapper2_dgetrf_
armadillo.lib(wrapper2.cpp.obj) : error LNK2019: unresolved external symbol _cgetrf_ referenced in function _wrapper2_cgetrf_
armadillo.lib(wrapper2.cpp.obj) : error LNK2019: unresolved external symbol _zgetrf_ referenced in function _wrapper2_zgetrf_

JackBoosY · 2021-01-18T06:06:17Z

Regression is gone.

JackBoosY · 2021-01-18T06:06:44Z

@Neumann-A Can you please review again?

Thanks.

JackBoosY · 2021-01-18T09:45:34Z

Fixes #15255.

scripts/cmake/vcpkg_configure_make.cmake

scripts/get_cmake_vars/CMakeLists.txt

JackBoosY · 2021-01-21T03:00:54Z

Seems have many regressions when building x64-osx, please get failure logs here.

…e_crosscompiling_android

venabled · 2021-01-25T21:07:15Z

@xandox Thanks for the contribution. I've been playing around with this PR to try and build 'libiconv' the past two days, and I've found a few things that have prevented that port from building.

I think most of it was that --gcc-toolchain' and '--sysroot' are getting pulled into the VCPKG_DETECTED_CMAKE_C*FLAGS*` variables.

And according to @DanAlbert post in the NDK repo and the NDK integration with other build systems documentation those really shouldn't be in there.

I guess this isn't really a problem with vcpkg_configure_make as it's an issue in Meson ports too. These aren't set as C/CPP flags in the Android toolchain CMake file, so Ill have to keep digging.

xandox · 2021-01-25T21:47:00Z

@venabled hi. I am not sure what you mean. I am able to build libiconv with 21d and 22 ndk. What is you problem?

According to the article you provided - yes, looks like --sysroot and --gcc-toolchain not necessary here, but they are not a problem. They came from android.toolchain.cmake and have correct paths.

venabled · 2021-01-25T21:51:19Z

I guess I should have been more specific, I had to modify 'libiconv' port as the current port only builds from source if you're on Windows.

xandox · 2021-01-25T22:04:07Z

Yes, sure. See #15606.

DanAlbert · 2021-01-26T01:24:57Z

@venabled hi. I am not sure what you mean. I am able to build libiconv with 21d and 22 ndk. What is you problem?

According to the article you provided - yes, looks like --sysroot and --gcc-toolchain not necessary here, but they are not a problem. They came from android.toolchain.cmake and have correct paths.

Possibly superfluous context here, but in case it's helpful:

While they're not necessary to build correctly, as long as they are set to the appropriate values they shouldn't harm anything. The toolchain file still configures them because (iirc), CMake's find_* behaviors depend on CMAKE_SYSROOT. There were also old versions of our gradle plugin that depended on that value being present.

-gcc-toolchain I think has done nothing useful since r19 :)

The problem in the NDK bug linked was that folks had set --sysroot's value to the wrong path, and that can definitely break things.

…e_crosscompiling_android

strega-nil-ms · 2021-07-19T19:06:12Z

Cool, @JackBoosY could you review today?

dg0yt · 2021-07-20T06:11:42Z

ports/libiconv/portfile.cmake

@@ -1,4 +1,4 @@
-if(NOT VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_TARGET_IS_ANDROID)
+if(NOT VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_CROSSCOMPILING)


What is the point of this change? Port libiconv could be empty when the target has iconv, but it must be built

when the target is Windows.

when the target is ...

But it doesn't matter if we are cross compiling.

If I am building libiconv on linux-x64 for linux-arm64 without VCPKG_CROSSCOMPILING it will let empty, for example.

If I am building libiconv on linux-x64 for linux-arm64 without VCPKG_CROSSCOMPILING it will let empty, for example.

IIUC that's just the intention of that code block: "iconv is empty unless built for Windows or Android." Probably this assumes iconv system libraries on linux, osx or bsd, and maybe this is not 100% right. But it doesn't matter if you compile from a different platform or natively.

Can you just remove the iconv changes from this PR? IMO they seem to be unrelated to "vcpkg_configure_make flags for android".

JackBoosY · 2021-07-21T02:31:17Z

Can you please resolve the file conflicts?

…e_crosscompiling_android

xandox · 2021-07-21T08:19:21Z

Can you please resolve the file conflicts?

done

…use system headers

dg0yt

I would like to note that the get_cmake_vars project is transforming into a unit which no longer reports the actual cmake variables. This is probably beneficial for some use cases, but it can fail miserably for other uses.
My preference would be a clearer separation of concerns, e.g. by using different variables for computed effective properties, and/or by offering a separate entry point for getting these properties.

dg0yt · 2021-07-21T10:05:01Z

scripts/get_cmake_vars/CMakeLists.txt

+foreach(_lang IN ITEMS C CXX)
+    foreach(INCDIR ${CMAKE_${_lang}_IMPLICIT_INCLUDE_DIRECTORIES})
+        string(APPEND CMAKE_${_lang}_FLAGS " ${CMAKE_INCLUDE_SYSTEM_FLAG_${_lang}}${INCDIR}")
+    endforeach()


I don't see the benefit here. Implicit directories are needed to be known in order to find a file in build system configuration steps, but they do not need to be given explicitly to the compiler.

Unfortunately for some cases it needed to be explicitly provided. Especially for macos native compiler.

Unfortunately for some cases it needed to be explicitly provided. Especially for macos native compiler.

Now I wonder how I can build autotools projects without vcpkg/cmake.

(All really meant constructive. I just want to avoid pitfalls from my personal cross-platform cmake experience.)

Yep. good catch.
I am not good in autotools or macos, so some explanation could be not accurate. But what I saw is. There are two different compile system in macos (how I understand this). First one which cmake uses usually placed in /Library/..... The compiler in this system unusable without all this flags and include directories. And cmake provide them to it. The second one placed in /usr/bin and if you just run configure.sh for some autotools package very probably that it will select second one system. And it is what happen right now in master when you install some autotools package by vcpkg. So there is some inconsistent.

When I just start this PR I tried to leave this behaviour unchanged. But guys decided it's not good idea. And now I force autotools use the same compiler as cmake

When I just start this PR I tried to leave this behaviour unchanged. But guys decided it's not good idea. And now I force autotools use the same compiler as cmake

Yes, this is a good thing.

I am not good in autotools or macos

I wouldn't claim to be an expert. This discussion is also an invitation for others to help with feedback.

xandox · 2021-07-21T10:27:35Z

I would like to note that the get_cmake_vars project is transforming into a unit which no longer reports the actual cmake variables.

Not sure what you mean. Which variable it reports now?

My preference would be a clearer separation of concerns, e.g. by using different variables for computed effective properties, and/or by offering a separate entry point for getting these properties.

It reports all properties, consumer could use only what they need.

Neumann-A · 2021-07-21T10:29:36Z

Why do we need host cmake vars? VCPKG should not build for the host but only the target! host binaries are provided by host dependencies! Otherwise patch the buildsystem

xandox · 2021-07-21T10:34:29Z

Why do we need host cmake vars? VCPKG should not build for the host but only the target!

Host cmake vars needed for host tools used during compilation (file generators and so on).

host binaries are provided by host dependencies! Otherwise patch the buildsystem

It's good idea but not for all ports in single pull request. Let's merge this one which works, and then everyone who would like to patch every single autotools port to separate host-build-tools from main package could do it in their own rithm.

dg0yt · 2021-07-21T10:57:00Z

I would like to note that the get_cmake_vars project is transforming into a unit which no longer reports the actual cmake variables.

Not sure what you mean. Which variable it reports now?

All the added lines which look like string(APPEND CMAKE_...) transform the original value of CMAKE_....

xandox · 2021-07-21T11:04:23Z

I would like to note that the get_cmake_vars project is transforming into a unit which no longer reports the actual cmake variables.

Not sure what you mean. Which variable it reports now?

All the added lines which look like string(APPEND CMAKE_...) transform the original value of CMAKE_....

So, you suggest to make another one project like get_cmake_vars_ex? Is get_cmake_vars project needed only for other buildsystems to provide correct options?

dg0yt · 2021-07-21T16:56:01Z

I would like to note that the get_cmake_vars project is transforming into a unit which no longer reports the actual cmake variables.

Not sure what you mean. Which variable it reports now?

All the added lines which look like string(APPEND CMAKE_...) transform the original value of CMAKE_....

So, you suggest to make another one project like get_cmake_vars_ex? Is get_cmake_vars project needed only for other buildsystems to provide correct options?

I didn't want to jump into proposing a particular implementation.

Following the single responsibility principle and the principle of least surprise, get_cmake_vars should do just that: report cmake variables. Still, output may vary by platform, to cover special properties.
And then another script/function can compose the variables to form new variables. It doesn't even need another CMake project, it will simply use the output of get_cmake_vars. If the purpose is providing configuration information for build system, I would probably choose a name like get_build_configuration.

Disclaimer: This is just personal opinion of an external contributor.

m-kuhn · 2021-08-27T07:54:55Z

Gentle ping, an integration of this would be much appreciated (it's really nice work)

strega-nil-ms · 2021-09-08T00:13:05Z

This does look good, but it needs to be updated for the new style of z_vcpkg_get_cmake_vars; @xandox are you interested in updating the PR for the new world order; otherwise, @m-kuhn do you want to take over this PR?

xandox · 2021-09-08T09:06:18Z

@strega-nil-ms @m-kuhn Sorry, but right now I have no time to work on this PR. If someone would take over this I will be appreciated.

m-kuhn · 2021-09-10T14:36:54Z

I am afraid this pull request is outside my comfort zone and I don't have the time available to expand this zone at the moment.

Even if it was within the zone, what I think this needs currently is:

a core maintainer to comment on [android] vcpkg_configure_make correct set flags for android build #15605 (comment) - which as far as I can see is the only open question at the moment
a rebase / merge of master

and since number 2 depends on number 1 it will be hard to take over for anyone at the moment

JackBoosY · 2022-01-06T16:44:19Z

If you want to continue this PR, please reopen it.

[android] vcpkg_configure_make correct set flags for android build

fd8f515

Neumann-A reviewed Jan 12, 2021

View reviewed changes

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

Neumann-A reviewed Jan 12, 2021

View reviewed changes

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

Neumann-A reviewed Jan 12, 2021

View reviewed changes

scripts/get_cmake_vars/CMakeLists.txt Outdated Show resolved Hide resolved

JackBoosY self-assigned this Jan 13, 2021

JackBoosY added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Jan 13, 2021

Ilya Arzhannikov added 2 commits January 15, 2021 17:03

Merge remote-tracking branch 'origin/master' into vcpkg_configure_mak…

2887740

…e_crosscompiling_android

extend [c,cxx,ld] flags with crosscompiling arguments

489fc1e

JackBoosY approved these changes Jan 18, 2021

View reviewed changes

JackBoosY linked an issue Jan 18, 2021 that may be closed by this pull request

Get ports working for android that use vcpkg_configure_make / vcpkg_install_make #15255

Closed

ras0219-msft requested changes Jan 20, 2021

View reviewed changes

JackBoosY added the requires:author-response label Jan 20, 2021

review fixes

d8569dd

xandox requested a review from ras0219-msft January 20, 2021 15:40

xandox and others added 4 commits January 21, 2021 12:07

set CC env if crosscompiling or not target is osx

b116f00

Merge remote-tracking branch 'origin/master' into vcpkg_configure_mak…

8c21e55

…e_crosscompiling_android

fix build on OSX

09d2be8

Merge remote-tracking branch 'origin/master' into vcpkg_configure_mak…

ccd8931

…e_crosscompiling_android

xandox requested a review from JackBoosY January 25, 2021 13:58

venabled mentioned this pull request Jan 26, 2021

Extra Flags in Android Builds #15876

Closed

Merge remote-tracking branch 'origin/master' into vcpkg_configure_mak…

446d46e

…e_crosscompiling_android

dg0yt reviewed Jul 20, 2021

View reviewed changes

xandox added 3 commits July 21, 2021 10:10

revert libiconv

bce4cfe

fix vcpkg package version

0697b3f

Merge remote-tracking branch 'origin/master' into vcpkg_configure_mak…

53690ac

…e_crosscompiling_android

use VCPKG_INSTALLED_PATH include dir as first include dir to prevent …

a39d18b

…use system headers

dg0yt reviewed Jul 21, 2021

View reviewed changes

JackBoosY marked this pull request as draft September 9, 2021 02:32

mathisloge mentioned this pull request Sep 24, 2021

vcpkg dependency example tangrams/tangram-es#2274

Draft

m-kuhn mentioned this pull request Oct 20, 2021

[android] Android NDK r23 and clang support #20844

Closed

This was referenced Nov 12, 2021

Fix android ndk clang #21222

Closed

New Android NDK support request #21345

Open

PhoebeHui linked an issue Nov 12, 2021 that may be closed by this pull request

New Android NDK support request #21345

Open

This was referenced Dec 3, 2021

[android] support "make" based ports #21831

Closed

[vcpkg] Enhancement android triplet #21847

Closed

JackBoosY closed this Jan 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[android] vcpkg_configure_make correct set flags for android build #15605

[android] vcpkg_configure_make correct set flags for android build #15605

xandox commented Jan 12, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 21, 2021

venabled commented Jan 25, 2021

xandox commented Jan 25, 2021

venabled commented Jan 25, 2021

xandox commented Jan 25, 2021

DanAlbert commented Jan 26, 2021

strega-nil-ms commented Jul 19, 2021

dg0yt Jul 20, 2021

xandox Jul 20, 2021

dg0yt Jul 20, 2021

xandox Jul 21, 2021

JackBoosY commented Jul 21, 2021

xandox commented Jul 21, 2021

dg0yt left a comment

dg0yt Jul 21, 2021

xandox Jul 21, 2021

dg0yt Jul 21, 2021

xandox Jul 21, 2021

dg0yt Jul 21, 2021

xandox commented Jul 21, 2021

Neumann-A commented Jul 21, 2021

xandox commented Jul 21, 2021

dg0yt commented Jul 21, 2021

xandox commented Jul 21, 2021

dg0yt commented Jul 21, 2021

m-kuhn commented Aug 27, 2021

strega-nil-ms commented Sep 8, 2021

xandox commented Sep 8, 2021

m-kuhn commented Sep 10, 2021

JackBoosY commented Jan 6, 2022

		@@ -1,4 +1,4 @@
		if(NOT VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_TARGET_IS_ANDROID)
		if(NOT VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_CROSSCOMPILING)

[android] vcpkg_configure_make correct set flags for android build #15605

[android] vcpkg_configure_make correct set flags for android build #15605

Conversation

xandox commented Jan 12, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 18, 2021

JackBoosY commented Jan 21, 2021

venabled commented Jan 25, 2021

xandox commented Jan 25, 2021

venabled commented Jan 25, 2021

xandox commented Jan 25, 2021

DanAlbert commented Jan 26, 2021

strega-nil-ms commented Jul 19, 2021

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

JackBoosY commented Jul 21, 2021

xandox commented Jul 21, 2021

dg0yt left a 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

xandox commented Jul 21, 2021

Neumann-A commented Jul 21, 2021

xandox commented Jul 21, 2021

dg0yt commented Jul 21, 2021

xandox commented Jul 21, 2021

dg0yt commented Jul 21, 2021

m-kuhn commented Aug 27, 2021

strega-nil-ms commented Sep 8, 2021

xandox commented Sep 8, 2021

m-kuhn commented Sep 10, 2021

JackBoosY commented Jan 6, 2022