set autorcc and autouic by default
AbandonedPublic

Authored by sitter on Jul 29 2019, 11:45 AM.

Details

Summary

I couldn't find any pertinent discussion on the topic but some reviews I
stumbled over did set it on some of our application repos and also wonder
why we don't enable it by default.

autorcc allows more idiomatic use of qrc as they may be used like any
"ordinary" source file and cmake will know what to do with them (namely
compile into relevant cpp for inclusion in target) without the developer
having to worry about anything.

autouic does the same albeit for .ui files.

Test Plan

.qrc files can be added to src list variables and will get automatically generated into cpp files in the binary dir and built into the target

Diff Detail

Repository
R240 Extra CMake Modules
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15295
Build 15313: arc lint + arc unit
sitter created this revision.Jul 29 2019, 11:45 AM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptJul 29 2019, 11:45 AM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
sitter requested review of this revision.Jul 29 2019, 11:45 AM
apol added a subscriber: apol.Jul 29 2019, 11:47 AM

+1 I've been using it in several project, it works good. I don't see how this could be misused.

sitter edited the summary of this revision. (Show Details)Jul 29 2019, 11:48 AM

Just a small principal comment with "supporting-old-versions" hat on.
Cannot give feedback on autorcc itself, never used, so no idea how useful/reliable it is.

kde-modules/KDECMakeSettings.cmake
245

Not sure if anyone is using cmake < 3.0 these days, but if, this will be a surprise box to them, as with some biulds things work (where cmake >= 3.0) and with some builds not.

This needs explicit mentioning in the docs, so developers know what they have to prepare for, also a mention since which ECM version one can rely on this behaviour.

sitter added inline comments.Jul 29 2019, 12:13 PM
kde-modules/KDECMakeSettings.cmake
245

This is cmakes's own behavior. <3.0 has no autorcc to begin with.

That being said the only reason I put the if there is because ECM itself is compatible with 2.8.12, so the if seemed appropriate.

kossebau added inline comments.Jul 29 2019, 12:28 PM
kde-modules/KDECMakeSettings.cmake
245

Ah, I see now <nip-more-coffee/>

Still leaves the desire to know as user of KDECMakeSettings since which vesion I could rely on CMAKE_AUTORCC being set, so with which min ECM version I can remove the line from own code -> some since 6.62 in the docs, please.

+1, same for AUTOUIC probably?

Indeed, both good points.

sitter updated this revision to Diff 62838.Jul 31 2019, 10:30 AM

document behavior and when it was introduced, also enable autouic

sitter retitled this revision from set autorcc by default to set autorcc and autouic by default.Jul 31 2019, 10:31 AM
sitter edited the summary of this revision. (Show Details)
krop added a subscriber: krop.Aug 14 2019, 1:57 PM

@kossebau @vkrause @apol something else to change?

kde-modules/KDECMakeSettings.cmake
82

5.62 now

I'm happy with this, +2 from my side.

Patch as is fine with me.
But no expert on both flags itself, no idea if there could be sideeffects of having those two now injected as ON to all projects. Possibly best to do global rebuilds of all projects on CI once this lands.

sitter updated this revision to Diff 64032.Aug 19 2019, 12:23 PM

bump version again

apol accepted this revision.Aug 19 2019, 12:35 PM

LGTM

This revision is now accepted and ready to land.Aug 19 2019, 12:35 PM
This revision was automatically updated to reflect the committed changes.
nicolasfella reopened this revision.Aug 19 2019, 1:26 PM
nicolasfella added a subscriber: nicolasfella.

This seems to cause the build of sonnet to fail: https://invent.kde.org/snippets/394

Another user has reported that ktexteditor and knewstuff fail too

This revision is now accepted and ready to land.Aug 19 2019, 1:26 PM
nicolasfella requested changes to this revision.Aug 19 2019, 1:26 PM
This revision now requires changes to proceed.Aug 19 2019, 1:26 PM

Yep, KTextEditor is broken for me, too.

AutoUic error

"/local/ssd/cullmann/kde/src/ktexteditor/src/completion/katecompletionconfig.cpp"

Could not find "completionconfigwidget.ui" in

"/local/ssd/cullmann/kde/src/ktexteditor/src/completion/completionconfigwidget.ui"

[spam comment removed by sysadmin]