[Differential] [Updated] D3646: Ignore host libs/includes/cmakeconfig files in Android toolchain

kossebau (Friedrich W. H. Kossebau) noreply at phabricator.kde.org
Mon Dec 19 00:37:30 UTC 2016


kossebau added inline comments.

INLINE COMMENTS

> apol wrote in Android.cmake:163
> Maybe call it `ECM_ALTERNATIVE_ROOT_PATH`, in case we ever need to use it on other platforms as well?

Hm, indeed a point in that ECM_ANDROID_ROOT_PATH sounds like this is the only one and might replace the root set by the ndk. So would agree a better term should be found.

`ALTERNATIVE` sounds a little bit strange to me initially, also not part of any existing terms in cmake/ecm vars for what I found. But given it is used in the description of CMAKE_FIND_ROOT_PATH (see https://cmake.org/cmake/help/v3.1/variable/CMAKE_FIND_ROOT_PATH.html?highlight=alternative) for the values passed here, and in the end is pretty self explaining.
Or something like `ECM_ADDITIONAL_FIND_ROOT_PATH` to pick up the cmake-naming and still point out this are just additional ones? Perhaps with a better term than `ADDITIONAL`?

> apol wrote in Android.cmake:188
> This is unrelated. Also maybe while at it you can document it right away.
> I just added it so that we can detect problems at config time rather than while linking.

So gnustl_shared is required by Qt for Android? Any link for backing this up which I could use to please curious people? I could not find anything when I quickly searched, so this here seemed just some magic checking :)

> apol wrote in Android.cmake:209
> That's not requried, as the NDK already passes the `__ANDROID__`.

This was just moved around (see l.167 before), as part of grouping things together instead of interleaved setup (as also mentioed at end of summary text). Am to be blamed for trying too many things in one review, even if related, will see to keep things as simple as possible in the future.

This was added in https://phabricator.kde.org/R240:faedc8d01697ab1d573d5740f24e7279f4dba14f. 
So it was now found this would not be needed? Which Qt versions would `__ANDROID__` work with? @cordlandwehr , your commit to be defended here, please tell :)

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  avoidHostLibsIncludesinAndroidToolchain

REVISION DETAIL
  https://phabricator.kde.org/D3646

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau, #frameworks, #gcompris, #minuet, apol, mutlaqja, sandsmark, nienhueser, cordlandwehr
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161219/202eebbf/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list