ki18n cmake macros: Mark UIC-generated .h files to skip AUTOMOC by default
ClosedPublic

Authored by mpyne on Dec 2 2017, 10:00 PM.

Details

Summary

CMake in 3.10.0 has started warning that .h files which are automatically generated have historically been left out of AUTOMOC, and that future CMake releases will start including them in AUTOMOC.

For now the warning can be squelched by either specifically opting in to future behavior (by setting policy CMP0071) or by manually marking the generated file as needing to skip AUTOMOC. Since the ui_*.h files do not create QObject-based subclasses, it seems to me that matching historical behavior and skipping AUTOMOC is the proper decision.

The question is where in the CMake code to set that property. The warnings I saw were generated for *.h files created by uic. This generation step is ultimately setup by the ki18n_wrap_ui CMake macro from KI18n, so this seems like the right spot to flag the generated .h file as needing to skip AUTOMOC.

Test Plan

KI18n builds and installs.
My test app (JuK) now successfully builds and installs without the CMake warnings previously generated.

Diff Detail

Repository
R249 KI18n
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mpyne created this revision.Dec 2 2017, 10:00 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 2 2017, 10:00 PM
mpyne requested review of this revision.Dec 2 2017, 10:00 PM
aacid added a subscriber: aacid.Dec 3 2017, 10:52 AM

Will this cause trouble with older cmake versions?

mpyne added a comment.Dec 3 2017, 3:53 PM
In D9118#174892, @aacid wrote:

Will this cause trouble with older cmake versions?

Good question, I haven't tried it. I've verified that set_property at least is present in 2.8.12 so it shouldn't introduce a syntax error. Whether the added semantic of turning on SKIP_AUTOMOC introduces an issue is open.

Perhaps it's best to try to wrap with a version check. If no one is able to test on an ancient supported CMake I can try to adapt the patch to add that.

kfunk accepted this revision.Dec 4 2017, 8:59 AM

This shouldn't create any troubles with earlier CMake versions.

Also note that we set the same properties unconditionally in newer Qt5 CMake macros: https://codereview.qt-project.org/#/c/207327

cmake/KF5I18NMacros.cmake
56 ↗(On Diff #23312)

set_source_files_properties(${_header} ...) is probably the more canonical way to do this.

This revision is now accepted and ready to land.Dec 4 2017, 8:59 AM
This revision was automatically updated to reflect the committed changes.