D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-19 Thread Friedrich W . H . Kossebau
kossebau added a comment. And given what I think I learned here; also created D10665 to remove the seemingly effectless rules for depending the source's object file on the JSON file. REPOSITORY R244 KCoreAddons REVISION DETAIL

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-19 Thread Friedrich W . H . Kossebau
kossebau added a comment. BTW, for the related issue of moc not being re-run on the regeneration of the JSON file, I managed to get some test case to narrow this to automoc possibly and just reported it as https://gitlab.kitware.com/cmake/cmake/issues/17750 REPOSITORY R244 KCoreAddons

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-18 Thread Tobias C . Berner
tcberner abandoned this revision. tcberner added a comment. Works for me REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco Cc: bcooksley, rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-18 Thread Raphael Kubo da Costa
rakuco added a comment. I agree this can be abandoned -- whatever solution we agree upon should probably be done in plasma-desktop. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco Cc: bcooksley,

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Michael Pyne
mpyne added a comment. OK then, I think @kossebau is right in that this is a dependency issue in the `lookandfeel` part of plasma-desktop. The `kcm_lookandfeel` target declares the JSON dependency (with the CMake macro) in time for CMake to care about it and ensure the generated build

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Raphael Kubo da Costa
rakuco added a comment. > This isn't just a problem on KDE Neon though, is it? I thought FreeBSD is also affected? To be clear, FreeBSD is affected by not having any fix in the tree (i.e. the json file not being present when moc is invoked), whereas Neon fails with D10485

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Ben Cooksley
bcooksley added a comment. For the record, the FreeBSD builds on the CI system hit this fairly regularly. As shown by https://build.kde.org/view/Plasma/job/Plasma%20plasma-desktop%20kf5-qt5%20FreeBSDQt5.9/ REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Michael Pyne
mpyne added a comment. In D10450#208453 , @kossebau wrote: > So just to make sure we are all on the same page: for what I have understood meanwhile is what is missing but needed here is a dependency rule between > a) the generated JSON file

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Friedrich W . H . Kossebau
kossebau added a comment. In D10450#208413 , @mpyne wrote: > Yes, I think I agree with @rakuco. Especially since the fix for D10485 ended up being reverted. Would be happy if anyone on KDE neon could

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Michael Pyne
mpyne added a comment. What about this? I can't change the diff since I didn't create the RR, but this seems to cause the required dependency rules to be added and works for me to build plasma-desktop. The only real addition is the `add_dependencies` call. I tried this without the

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Michael Pyne
mpyne added a comment. Yes, I think I agree with @rakuco. Especially since the fix for D10485 ended up being reverted. I still think a separate fix is needed for kcm_lookandfeel, but the issue is that the `kcoreaddons_desktop_to_json` macro generates a

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Raphael Kubo da Costa
rakuco added a comment. Per my previous comment, I still don't see how changing this to a target would solve anything. For one, the CMake implementation allows both files and targets to

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-17 Thread Tobias C . Berner
tcberner added a comment. I think as @adridg points out that it should be a target, this should go in -- and the @kossebau already committed the proper workaround in D10485 , right? REPOSITORY R244 KCoreAddons REVISION DETAIL

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-16 Thread Michael Pyne
mpyne added a comment. So what's the conclusion here? Is this only a bug in kcm_lookandfeel or do we think that some follow-on changes are still required in `kcoreaddons_desktop_to_json`? REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner,

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-13 Thread Friedrich W . H . Kossebau
kossebau added a comment. To add some code to my words, a proposed fix for the detected unneeded json file and separate cmdl tool dependency now up here: https://phabricator.kde.org/D10485 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner,

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-12 Thread Raphael Kubo da Costa
rakuco requested changes to this revision. rakuco added a comment. This revision now requires changes to proceed. @kossebau's right: the problem lies in `lookandfeeltool` depending on `kcm.cpp`, while the `kcoreaddons_desktop_to_json()` call makes the `kcm_lookandfeel` target depend on the

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-12 Thread Friedrich W . H . Kossebau
kossebau added a subscriber: kfunk. kossebau added a comment. In D10450#204762 , @adridg wrote: > In D10450#204623 , @kossebau wrote: > > > "This (hopefully) fixes the build failure noticed in the

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-12 Thread Bhushan Shah
bshah accepted this revision. bshah added a comment. This revision is now accepted and ready to land. +1 REPOSITORY R244 KCoreAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner, #freebsd, mpyne, bshah, dfaure Cc: adridg, kossebau, #frameworks,

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-12 Thread Adriaan de Groot
adridg added a comment. In D10450#204623 , @kossebau wrote: > "This (hopefully) fixes the build failure noticed in the FreeBSD (and some linuxes)" - leaves the question: why should it exactly fix it? :) From reading the CMake

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-12 Thread Adriaan de Groot
adridg added a comment. Associated bug https://bugs.kde.org/show_bug.cgi?id=389982 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner, #freebsd, mpyne, bshah, dfaure Cc: adridg, kossebau, #frameworks, michaelh

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-11 Thread Friedrich W . H . Kossebau
kossebau added a comment. "This (hopefully) fixes the build failure noticed in the FreeBSD (and some linuxes)" - leaves the question: why should it exactly fix it? :) From discussion of last week on irc, it seemed that the actual problem is that the generated make files do not contain

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-11 Thread Tobias C . Berner
tcberner added reviewers: bshah, dfaure. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D10450 To: tcberner, #freebsd, mpyne, bshah, dfaure Cc: #frameworks, michaelh

D10450: Generate a custom target in kcoreaddons_desktop_to_json

2018-02-11 Thread Tobias C . Berner
tcberner created this revision. tcberner added reviewers: FreeBSD, mpyne. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. tcberner requested review of this revision. REVISION SUMMARY This creates a custom target 'desktop_to_json_X'