Make use of upstream CMake infrastructure to detect the compiler toolchain
ClosedPublic

Authored by apol on Mar 29 2018, 2:01 AM.

Details

Summary

Instead of having ad-hoc code for gcc, let CMake do its thing. It has a lot of
logic that we may be interested in, for example it will make the clang switch
much smoother.

Note it raises the minimum cmake to 3.7 for Android, which was released almost
2 years ago.

Test Plan

Built kalgebra on it using kdeorg/android-sdk

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Mar 29 2018, 2:01 AM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptMar 29 2018, 2:01 AM
apol requested review of this revision.Mar 29 2018, 2:01 AM
vkrause accepted this revision.Mar 29 2018, 6:52 AM
vkrause added a subscriber: vkrause.

Nice! Works fine in my setup too.

This revision is now accepted and ready to land.Mar 29 2018, 6:52 AM
This revision was automatically updated to reflect the committed changes.

This change produces invalid JSON in my setup:

"stdcpp-path":" "/opt/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/arm64-v8a/libgnustl_shared.so" -nodefaultlibs -lgcc -lc -lm -ldl",

So it think there's some refinement required to make this more robust. The file stl contains "/opt/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/arm64-v8a/libgnustl_shared.so" -nodefaultlibs -lgcc -lc -lm -ldl in my case. When escaping the quotes correctly so the JSON is no longer invalid the next error I get is:

STL library does not exist at  "/opt/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/arm64-v8a/libgnustl_shared.so" -nodefaultlibs -lgcc -lc -lm -ldl

So I suppose only a single file should be specified here. When I change it to only /opt/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/arm64-v8a/libgnustl_shared.so it finally works.

BTW, I'm using Arch Linux and the following packages/versions:

pacman -Q cmake android-sdk android-ndk android-qt5-arm64-v8a extra-cmake-modules 
cmake 3.12.1-1
android-sdk 26.1.1-1
android-ndk r17.b-1
android-qt5-arm64-v8a 5.11.1-1
extra-cmake-modules 5.49.0-1
apol added a comment.Aug 31 2018, 1:17 PM

@Martchus sounds like your CMAKE_CXX_STANDARD_LIBRARIES gets initialized wrong somehow, no?

@apol I'm not that familiar with Android, but I guess the variable CMAKE_CXX_STANDARD_LIBRARIES is not limited to contain only a single library (note the plural in the variable name). So I think we should handle that.

BTW, I'm just testing with the accelbubble example from Qt. I created a fairly simple CMakeLists.txt for it which definitely doesn't mess with the variables:

cmake_minimum_required(VERSION 3.1.0 FATAL_ERROR)

set(CMAKE_AUTORCC ON)

find_package(Qt5Core)
find_package(Qt5Quick)
find_package(Qt5Sensors)
find_package(Qt5Svg)
find_package(Qt5Xml)

add_executable(accelbubble main.cpp accelbubble.qrc)
target_link_libraries(accelbubble Qt5::Quick Qt5::Sensors Qt5::Svg Qt5::Xml)

I also didn't touch that variable via CLI options when invoking CMake. However, to workaround the issue I tried it. But using -DCMAKE_CXX_STANDARD_LIBRARIES=/opt/android-ndk/sources/cxx-stl/gnu-libstdc++/4.9/libs/arm64-v8a/libgnustl_shared.so only leads to the specified value being pretended to the auto-detected values (so I end up having .../libgnustl_shared.so twice).

It's the same issue I have here, but after our discussion at Akademy I assumed this to be local setup issue. Similar versions here, on Tumbleweed.

Martchus added a comment.EditedAug 31 2018, 2:58 PM

@vkrause Thanks for confirming. So it is not a local issue. Could you find a workaround (beside reverting these changes)?

As I already mentioned, I'm not so familiar with Android. However, for me the way CMake populates CMAKE_CXX_STANDARD_LIBRARIES doesn't look completely unconceivable and in fact compilation/linking seems to work fine that way. To improve we could just iterate over the list of libraries CMake detects and pick the first one which is a full path. That's also not very reliable, but it would at least fix the problem I have right now.

@vkrause Thanks for confirming. So it is not a local issue. Could you find a workaround (beside reverting these changes)?

I edit the file stl in the build dir to strip out the additional -lxxx entries, but that has to be redone after every cmake run. Haven't found the time to properly investigate this yet unfortunately.