kdevplatform_add_plugin: remove effectless OBJECT_DEPENDS on json file
ClosedPublic

Authored by kossebau on Feb 19 2018, 6:27 PM.

Details

Summary

The JSON file argument passed to K_PLUGIN_FACTORY_WITH_JSON ends up
being used with the macro Q_PLUGIN_METADATA, which is a no-code macro
at the C++ level and only used to note information used by moc for the
generated moc file.

So when the content of the JSON file has changed, this will not change
anything in the preprocessed source file itself. It will only change the
content of the moc file generated based on it, which already is noted
as dependency to the object file due to being a file included by the
source.

This code possibly was added hoping to solve the issue of cmake's automoc
not properly dealing with Q_OBJECT macros hidden away in other C++
preprocessor macros. But it misses to catch the issue that it is the
generated moc file which needs to be regenerated on a change of the JSON
file, not just the object file.
Regenerating the moc file is something only automoc decides about when
being run and which seems not influenced by CMakeLists.txt rules.

Cmp. https://gitlab.kitware.com/cmake/cmake/issues/17750

Test Plan

Still builds as before.
Editing JSON files still results in same (broken) effective rebuild
behaviour: the moc file is not regenerated, so the new JSON content
not picked up. Other than before the unneeded-because-all-sources-the-same
rebuild of the object file is skipped though.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Feb 19 2018, 6:27 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptFeb 19 2018, 6:27 PM
kossebau requested review of this revision.Feb 19 2018, 6:27 PM
nalvarez accepted this revision.Feb 19 2018, 6:43 PM
nalvarez added a subscriber: nalvarez.

+1

I think what should actually ensure the moc file is regenerated is the AUTOGEN_TARGET_DEPENDS property, but we're setting that already (L141, now L124) and it doesn't seem to work.

This revision is now accepted and ready to land.Feb 19 2018, 6:43 PM

+1

I think what should actually ensure the moc file is regenerated is the AUTOGEN_TARGET_DEPENDS property, but we're setting that already (L141, now L124) and it doesn't seem to work.

That one seems to only work when Q_OBJECT is not embedded into another preprocessor macro. See https://gitlab.kitware.com/cmake/cmake/issues/17750 I just filed which has a test case to demo the issue.

apol added a subscriber: apol.Feb 19 2018, 11:35 PM

Port to the kcoreaddons macro?

In D10666#209890, @apol wrote:

Port to the kcoreaddons macro?

It's just a few lines duplicated after this patch, so not that much gained. Then kcoreaddons macro does not support KDEV_ADD_PLUGIN_SKIP_INSTALL, so not straight possible without more preparation.

apol accepted this revision.Feb 20 2018, 11:54 AM
This revision was automatically updated to reflect the committed changes.