D26749: WIP: Support NDK r20 and Qt 5.14

[Volker Krause]

vkrause added a comment.


  In D26749#620142 , @apol wrote:
  
  > Seems ready to land to me.
  
  
  It does seem to break poppler in the old setup here, not sure yet why though. 
If we land the docker change as well that is probably acceptable though.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: vkrause, apol
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
GB_2, bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> vkrause wrote in Android.cmake:173
> I actually didn't test 64bit ARM here, only armv7 and x86, both work. What 
> does break for you there?

I'm not sure what was going on, seems to work now.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: vkrause, apol
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
GB_2, bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Aleix Pol Gonzalez]

apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  Seems ready to land to me.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: vkrause, apol
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
GB_2, bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Volker Krause]

vkrause added inline comments.

INLINE COMMENTS

> apol wrote in Android.cmake:173
> I did some testing and for me it works for arm64 but not for arm32.

I actually didn't test 64bit ARM here, only armv7 and x86, both work. What does 
break for you there?

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
GB_2, bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Aleix Pol Gonzalez]

apol added a comment.


  Patch makes sense to me overall, we could consider landing it.

INLINE COMMENTS

> vkrause wrote in Android.cmake:173
> It's "better" in the way that it actually works with NDK r20, while CMake 
> 3.16 failed to even pass the basic compiler checks. For a toolchain file 
> shipped with a toolchain making adjustments for toolchain changes is of 
> course possible immediately, while with CMake we have to wait for the next 
> release if such changes are necessary.
> 
> It's unfortunately different in various variable names as well in some 
> details of what it sets or doesn't set. That's where most of the changes in 
> this file come from.
> 
> The main reason we didn't use this right from the start, and probably also 
> the reason the toolchain file in CMake exists is that the NDK didn't have 
> CMake support in older versions.

I did some testing and for me it works for arm64 but not for arm32.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, 
GB_2, bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Volker Krause]

vkrause added a comment.


  Excellent news! Could you post your Kirigami patch somewhere maybe? Makes 
this easier to test here :)
  
  For icons we probably need a similar adjustment, I bet they got affected by 
the same Qt change. Same for translations I think, in ki18n.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Francis Herne]

flherne added a comment.


  I got my Kirigami app to load by patching kirigami.{cpp,h}:
  
  Altered `resolveFilePath()` to `return 
QStringLiteral("qrc:/android_rcc_bundle/qml/org/kde/kirigami.2/") + path;` and 
similarly for `resolveFileUrl()`.
  
  Added 
`QResource::registerResource(QStringLiteral("assets:/android_rcc_bundle.rcc"));`
 in `registerTypes()` [Qt is supposed to do this, but perhaps not early enough?]
  
  https://codereview.qt-project.org/c/qt/qtbase/+/270573/24 is the relevant Qt 
change.
  
  Icons are still broken.
  There are probably better ways to solve this.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Francis Herne]

flherne added a comment.


  Thanks for this.
  
  ADB log of the Kirigami problem, which results in a blank screen ['gredit' is 
the name of the app]:
  
libgredit_arm64-v8a.so: QQmlApplicationEngine failed to load component
libgredit_arm64-v8a.so: qrc:/main.qml:5:1: Type Kg.ApplicationWindow 
unavailable
libgredit_arm64-v8a.so: 
file:///data/app/org.qtproject.example.gredit-n9QHCzgETccPFueeH24YPQ==/lib/arm64/ApplicationWindow.qml:
 No such file or directory
  
  The `--Added-by-androiddeployqt--` dir in the APK, which previously contained 
these QML files, is no longer created.
  
  The file contents now seem to appear in an RCC file at 
`assets/android_rcc_bundle.rcc`, but clearly Kirigami isn't understanding this.
  
  Icons packaged by `kirigami_package_breeze_icons` no longer appear under 
`assets`.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: flherne, apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Volker Krause]

vkrause added inline comments.

INLINE COMMENTS

> apol wrote in Android.cmake:173
> Why's this better? Or how is it different?

It's "better" in the way that it actually works with NDK r20, while CMake 3.16 
failed to even pass the basic compiler checks. For a toolchain file shipped 
with a toolchain making adjustments for toolchain changes is of course possible 
immediately, while with CMake we have to wait for the next release if such 
changes are necessary.

It's unfortunately different in various variable names as well in some details 
of what it sets or doesn't set. That's where most of the changes in this file 
come from.

The main reason we didn't use this right from the start, and probably also the 
reason the toolchain file in CMake exists is that the NDK didn't have CMake 
support in older versions.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> Android.cmake:173
> +set(ANDROID_STL ${CMAKE_ANDROID_STL_TYPE})
> +include(${CMAKE_ANDROID_NDK}/build/cmake/android.toolchain.cmake REQUIRED)
> +

Why's this better? Or how is it different?

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Volker Krause]

vkrause added a task: T12520: Qt 5.14.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D26749: WIP: Support NDK r20 and Qt 5.14

[Volker Krause]

vkrause retitled this revision from "Support NDK r20 and Qt 5.14" to "WIP: 
Support NDK r20 and Qt 5.14".
vkrause edited the test plan for this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: vkrause
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns