kpackage_install_bundled_package used install(CODE ..) for the
generation and installation of the rcc file. This had multiple flows,
most of distribution packaging fail as it tried to write to installation
prefix directly.
at time of install instead of the build-time, so it would fail with
permission errors if make install is run as normal user
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| KF5PackageMacros.cmake | ||
|---|---|---|
| 158 ↗ | (On Diff #30486) | This "Generating xyz" might be still good to have. Please consider picking this up with a COMMENT "Generating: ${xyz}"added to add_custom_command, where ${xyz} gets a proper variable name :) and as content the relative path to the current bin dir, by some file(RELATIVE_PATH xyz ${CMAKE_CURRENT_BINARY_DIR} ${GENERATED_RRC_CONTENTS})or possibly using directly without the xyz var ${component}-contents.rcc in the COMMENT message. |
| KF5PackageMacros.cmake | ||
|---|---|---|
| 158 ↗ | (On Diff #30486) | And rather COMMENT "Generating ${xyz}"so without the ":" to follow the usual pattern. |
See older reviews. This won't work because cmake doesn't know when one of the contained files is modified so rcc files are never regenerated properly.
I suggest actually checking DESTDIR.
Seems to work \o/ Good stuff.
Ship this, but lets not rush the workspace changes back in without some more people verifying that.
| KF5PackageMacros.cmake | ||
|---|---|---|
| 164 ↗ | (On Diff #30412) | Does a plain ${KDE_INSTALL_DATADIR} not also work? The non-FULL variable variants are the ones used usually with install(). Not exactly sure why they are, but doing here as well would be consistent at least and less surprising. |
| KF5PackageMacros.cmake | ||
|---|---|---|
| 164 ↗ | (On Diff #30412) | I'll create seperate PR for that. |
Edit.
Installation works, which is better.
However it still doesn't work properly.
make && install
Add a file to applets/digital-clock/contents/ui/Foo.qml with some valid QML
and then use it from main.qml
make && install
I get an error that the new file is not there.
This change cannot handle spaces in the path, which causes all CI builds involving RCC generation which have received this updated code to fail.
https://build.kde.org/job/Plasma%20plasma-pa%20kf5-qt5%20FreeBSDQt5.9/21/consoleText