Bug fix: find c++ stl using regex
ClosedPublic

Authored by sh-zam on Apr 14 2019, 8:55 PM.

Details

Summary

Find C++ shared lib path using regex. This change makes finding path
independent of the order in which it was added by cmake.

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.
sh-zam created this revision.Apr 14 2019, 8:55 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptApr 14 2019, 8:55 PM
Restricted Application added a subscriber: kde-buildsystem. · View Herald Transcript
sh-zam requested review of this revision.Apr 14 2019, 8:55 PM

Can we add a cmake_minimum_required to 3.7? Regex fails to compile on older versions, when run outside of Android.cmake.

apol added a comment.Apr 15 2019, 12:08 AM

Can we add a cmake_minimum_required to 3.7? Regex fails to compile on older versions, when run outside of Android.cmake.

Sure, add it.
Why are you using it without Android.cmake?

sh-zam updated this revision to Diff 56257.Apr 15 2019, 12:28 AM
  • disallow whitespaces in path

Why are you using it without Android.cmake?

I am building Krita and to add create-apk target I am using include (ECMAndroidDeployQt.cmake), I can't use Android.cmake because of some variables as it expects to run as a toolchain.

Later today, I will submit the patch for Krita which you might have to review :-)

sh-zam updated this revision to Diff 56311.Apr 15 2019, 4:53 PM
  • disallow whitespaces in path
apol added a comment.Apr 17 2019, 2:36 PM

Why are you using it without Android.cmake?

I am building Krita and to add create-apk target I am using include (ECMAndroidDeployQt.cmake), I can't use Android.cmake because of some variables as it expects to run as a toolchain.

What does it mean that "it expects to run as a toolchain"?

toolchain/ECMAndroidDeployQt.cmake
43

This looks quite awful. Why do you prefer using regex to string(FIND)? It could be something like
string(FIND "${VALUE}" "c++.so\"" OUT) if you want to be more specific.

sh-zam added inline comments.Apr 17 2019, 2:51 PM
toolchain/ECMAndroidDeployQt.cmake
43

Because string (FIND ..) doesn't work if libc++.so isn't added first to CMAKE_CXX_STANDARD_LIBRARIES

In my case this is the original value of CMAKE_CXX_STANDARD_LIBRARIES:

-latomic -lm "/home/sh_zam/Android/Sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libunwind.a" "/home/sh_zam/Android/Sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libc++_shared.so"

and the value written to stl file is the same thing as above, which down the line causes androidqtdeploy to fail.

apol accepted this revision.Apr 17 2019, 3:16 PM
This revision is now accepted and ready to land.Apr 17 2019, 3:16 PM

What does it mean that "it expects to run as a toolchain"?

Sorry, I didn't notice this.

On this line

get_filename_component(_CMAKE_ANDROID_DIR "${CMAKE_TOOLCHAIN_FILE}" PATH)

we get the path of ECM/toolchain, which is used to include ECMAndroidDeployQt.cmake, which in turn can only be extracted if it is being run as a toolchain.

In my case if I do this: include (Android.cmake), then it won't be able to find the ECMAndroidDeployQt.cmake because I am using android-ndk's toolchain.

This revision was automatically updated to reflect the committed changes.
apol added a comment.Apr 17 2019, 4:08 PM

In my case if I do this: include (Android.cmake), then it won't be able to find the ECMAndroidDeployQt.cmake because I am using android-ndk's toolchain.

And why do you have to do this? Why can't krita work like every other KDE project? In the end we'll want Krita to work in binary-factory.kde.org like the rest...

well.. I couldn't get it running as a toolchain and kept running into errors.

I can give it another hit, though.