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
  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-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

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 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, rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 system picks up the dependency for automoc to 
run.  The `lookandfeel` command line tool target does not add the dependency on 
the JSON file and there's no other reason for CMake to know that the JSON file 
must be generated before `lookandfeel`'s AUTOMOC can complete.
  
  ie. CMake perceives this order as legal:
  
  1. AUTOMOC for `lookandfeel`
  2. moc runs on kcm.cpp
  3. JSON generation for AUTOMOC for `kcm_lookandfeel`
  4. kcm_lookandfeel.json is generated
  5. AUTOMOC for `kcm_lookandfeel`
  6. moc runs on kcm.cpp
  7. build continues
  
  2 and 6 are identical here and it seems that CMake is smart enough to only do 
this once in the whole build, but CMake only has the dependency on the JSON for 
the moc at step 6, not step 2, so CMake doesn't know that it also needs to 
delay `lookandfeel`'s AUTOMOC phase until after JSON generation.
  
  That all said, if D10607  works (and that 
diff is precisely how I'd suggest handling the issue), then I think we can then 
abandon this revision.  Let me know if you disagree.

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-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  applied.

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-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

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-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 (`kcm_lookandfeel.json`)
  >  b) the generated moc file (`kcm.moc`) created by moc for the source file 
which references that JSON file in the related QObject subclass declaration and 
also has the include statement (`kcm.cpp`)
  >
  > And this is what
  >
  >   set_property(TARGET ${target} APPEND PROPERTY AUTOGEN_TARGET_DEPENDS 
${json})
  >
  >
  > should ensure at least by what the docs claim IIUC, but somehow does not.
  
  
  OK, I see what you're talking about.
  
  > And adding
  > 
  >   add_dependencies(${target} ${_json_target})
  > 
  > 
  > would also not ensure that straight dependency between the creation of the 
JSON file and the time when moc is run, no?
  
  No, you're right, it would only ensure that automoc and JSON creation both 
happen before the rest of the normal target build occurs.
  
  The custom target has a separate annoyance, using `add_custom_target` instead 
of `add_custom_command` makes it so that the entire JSON generation process 
seems to happen each time make or ninja is run, rather than only when needed 
from the dependencies changing, though I think this is due to the 
`add_dependencies` call rather than any issue specific to the CMake target 
itself.
  
  This isn't just a problem on KDE Neon though, is it?  I thought FreeBSD is 
also affected?

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 explore why it fails there (only 
reported for it so far AFAIK) so it had to be reverted. D10607 
 meanwhile up as a less 
fragile-with-automoc variant.
  
  > I still think a separate fix is needed for kcm_lookandfeel, but the issue 
is that the `kcoreaddons_desktop_to_json` macro generates a JSON file which is 
intended for use in a compiled file, and there's no easy way to connect that 
dependency within *this* macro since the dependent source file is never 
actually passed into the macro.
  
  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 (`kcm_lookandfeel.json`)
  b) the generated moc file (`kcm.moc`) created by moc for the source file 
which references that JSON file in the related QObject subclass declaration and 
also has the include statement (`kcm.cpp`)
  
  And this is what
  
set_property(TARGET ${target} APPEND PROPERTY AUTOGEN_TARGET_DEPENDS 
${json})
  
  should ensure at least by what the docs claim IIUC, but somehow does not.
  
  > Alternately we could try to do what the kcoreaddons_add_plugin macro does 
and let the calling code pass in the sources to be made dependent upon the JSON.
  
  All the other source files should not have any influence here, only the cpp 
file referencing the JSON file.
  
  And adding
  
add_dependencies(${target} ${_json_target})
  
  would also not ensure that straight dependency between the creation of the 
JSON file and the time when moc is run, no?

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 `add_custom_target` dance but CMake complained 
about the JSON file not being present so it looks like it needs to be a real 
CMake target to work.
  
diff --git a/KF5CoreAddonsMacros.cmake b/KF5CoreAddonsMacros.cmake
index f22817d..c15ab98 100644
--- a/KF5CoreAddonsMacros.cmake
+++ b/KF5CoreAddonsMacros.cmake
@@ -58,13 +58,19 @@ function(kcoreaddons_desktop_to_json target desktop)
   endforeach()
 endif()
 
+# Create a virtual target for the generated JSON file and force
+# the passed in target and its auto-generated sources to depend upon it
+string(RANDOM _target_name_suffix)
+set(_json_target "desktop_to_json_${_target_name_suffix}")
+add_custom_target(${_json_target})
 add_custom_command(
-OUTPUT ${json}
+TARGET ${_json_target}
 COMMAND ${command}
 WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
 DEPENDS ${desktop}
 )
-set_property(TARGET ${target} APPEND PROPERTY AUTOGEN_TARGET_DEPENDS 
${json})
+add_dependencies(${target} ${_json_target})
+set_property(TARGET ${target} APPEND PROPERTY AUTOGEN_TARGET_DEPENDS 
${_json_target})
 endfunction()
 
 function(_desktop_to_json_cmake28 desktop json compat)
  
  A resulting Make-based build directory seems to contain the right rules:
  
CMakeFiles/Makefile2
6522:kcms/lookandfeel/CMakeFiles/kcm_lookandfeel.dir/all: 
kcms/lookandfeel/CMakeFiles/desktop_to_json_ChFVC.dir/all
6597:kcms/lookandfeel/CMakeFiles/kcm_lookandfeel_autogen.dir/all: 
kcms/lookandfeel/CMakeFiles/desktop_to_json_ChFVC.dir/all

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 JSON file which is intended for 
use in a compiled file, and there's no easy way to connect that dependency 
within *this* macro since the dependent source file is never actually passed 
into the macro.
  
  I do think it makes more sense to have the custom command here be a custom 
target but that wouldn't help fix the dependency link-up.
  
  Would it be possible in CMake to alter the macro to somehow make the JSON a 
dependency of *all* the source files of the provided target instead of just the 
auto-generated files?  That would seem to get the right ordering without too 
much fuss.  `add_custom_command` has a second signature that looks useful 
(offers to make the command happen in "PRE BUILD", however it only works for 
MSVC.
  
  Alternately we could try to do what the kcoreaddons_add_plugin macro does and 
let the calling code pass in the sources to be made dependent upon the JSON.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 be specified there.
  
  Additionally, this would just add a different kind of dependency to 
`kcm_lookandfeel`, but not change `lookandfeel`'s dependencies -- the problem 
persists if target B depends on a source file that target A depends on, but 
only target A depends on the generation of a file that this source file depends 
on.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rikmills, rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 generation of the json file. It's pretty 
easy to reproduce this bug by running `make lookandfeeltool_autogen` with a 
fresh build directory.
  
  > For this very patch, I am still lost on how cmake generates the rules for 
automoc stuff, so no clue if switching to an explicit intermediate target 
improves something.
  
  In terms of generated `Makefile`s, without the patch in this review request 
we have 
`$BUILDDIR/kcms/lookandfeel/CMakeFiles/kcm_lookandfeel_autogen.dir/build.make` 
with the right dependencies 
(`kcms/lookandfeel/CMakeFiles/kcm_lookandfeel_autogen` depends on 
`kcms/lookandfeel/kcm_lookandfeel.json`, which calls `desktoptojson`). With 
this patch applied, the difference is that the dependency is moved to 
`$BUILDDIR/CMakeFiles/Makefile2`, where 
`kcms/lookandfeel/CMakeFiles/kcm_lookandfeel_autogen.dir/all` depends on 
`kcms/lookandfeel/CMakeFiles/desktop_to_json_.dir/all`. 
`kcms/lookandfeel/CMakeFiles/lookandfeeltool_autogen.dir/all` still doesn't 
depend on anything though, so the bug persists if I run `make  
lookandfeeltool_autogen` directly.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure, rakuco
Cc: rakuco, kfunk, adridg, kossebau, #frameworks, michaelh


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 FreeBSD (and 
some linuxes)" - leaves the question: why should it exactly fix it? :)
  >
  >
  > From reading the CMake documentation, it seems (but I'll admit that fancy 
dependency stuff is a dark art in CMake) that AUTOGEN_TARGET_DEPENDS shouldn't 
be a **file**, but a **target**. This patch adds a target, created by the given 
command and hooks that into the dependency graph.
  
  
  Okay, by reading the docs I would agree.
  
  >> From discussion of last week on irc, it seemed that the actual problem is 
that the generated make files do not contain the dependency between the JSON 
file that needs to be generated and automoc running over the cpp source file to 
generate the moc file based on the referenced JSON file.
  > 
  > And that's the whole problem, isn't it. If you force the dependency arc to 
be there -- i.e. by using the target property here -- then it works.
  
  Seems I am to blame for reading code with preoccupied mind :) indeed missed 
what the actual intention of the very line
  
set_property(TARGET ${target} APPEND PROPERTY AUTOGEN_TARGET_DEPENDS 
${json})
  
  was.
  
  >> Something which e.g. is tried to be solved by the code in the 
`kcoreaddons_add_plugin` macro, by grepping over all the source files to find 
the cpp file which references the JSON file and then create the dependency by
  >> 
  >>   set_property(SOURCE ${dependent_sources} APPEND PROPERTY OBJECT_DEPENDS 
${json})
  >>
  > 
  > If this patch fixes the dependency arc, then possibly that (expensive?) 
grepping around isn't needed either.
  
  Indeed. I remember vaguely though there were some bugs with 
AUTOGEN_TARGET_DEPENDS, @kfunk didn't you do something related to that?
  
  > I'll try to do some Makefile-dissection.
  
  Tried as well myself to get an idea, though somehow with the patch I still 
could not find the dep rules I expected. Discovered something else though:
  
  the CMakeLists.txt of the lookandfeel kcm actually defines two targets, which 
both have as source the 'kcm.cpp' file, 'kcm_lookandfeel' and 'lookandfeeltool`:
  
set(kcm_lookandfeel_SRCS
  kcm.cpp
  ../krdb/krdb.cpp
  ../cursortheme/xcursor/cursortheme.cpp
  ../cursortheme/xcursor/xcursortheme.cpp
)

add_library(kcm_lookandfeel MODULE ${kcm_lookandfeel_SRCS})
kcoreaddons_desktop_to_json(kcm_lookandfeel "kcm_lookandfeel.desktop" 
SERVICE_TYPES kcmodule.desktop)

set(lookandfeeltool_SRCS
lnftool.cpp
kcm.cpp
../krdb/krdb.cpp
../cursortheme/xcursor/cursortheme.cpp
../cursortheme/xcursor/xcursortheme.cpp
)

add_executable(lookandfeeltool ${lookandfeeltool_SRCS})
  
  Also note that automoc will be run on the sources for both targets. While the 
kcoreaddons_desktop_to_json macro as used is just adding rules related to the 
JSON file for the kcm_lookandfeel target.
  
  Now if we have a very close look at the build log files, we can see that it 
is actually the automoc being run for the sources of the lookandfeeltool 
target, which fails due to the missing JSON file. Which makes sense, given 
there are no dependency rules set by us. So even with this patch we have a race 
condition on the parallel build for the kcm_lookandfeel. This might also 
explain why we only see this kind of failure for this kcm, but nowhere else.
  
  So a separate fix for the build issue would be to make sure the 
K_PLUGIN_FACTORY_WITH_JSON call is in a separate source file only used for the 
kcm_lookandfeel target.
  
  For this very patch, I am still lost on how cmake generates the rules for 
automoc stuff, so no clue if switching to an explicit intermediate target 
improves something.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure
Cc: kfunk, adridg, kossebau, #frameworks, michaelh


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, michaelh


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 documentation, it seems (but I'll admit that fancy 
dependency stuff is a dark art in CMake) that AUTOGEN_TARGET_DEPENDS shouldn't 
be a **file**, but a **target**. This patch adds a target, created by the given 
command and hooks that into the dependency graph.
  
  > From discussion of last week on irc, it seemed that the actual problem is 
that the generated make files do not contain the dependency between the JSON 
file that needs to be generated and automoc running over the cpp source file to 
generate the moc file based on the referenced JSON file.
  
  And that's the whole problem, isn't it. If you force the dependency arc to be 
there -- i.e. by using the target property here -- then it works. Note, though, 
that I haven't dissected a Makefile / ninja file to see that the dependency arc 
is really there. We just tested it by re-running the build a couple of times on 
32- and 48-core machines.
  
  > Something which e.g. is tried to be solved by the code in the 
`kcoreaddons_add_plugin` macro, by grepping over all the source files to find 
the cpp file which references the JSON file and then create the dependency by
  > 
  >   set_property(SOURCE ${dependent_sources} APPEND PROPERTY OBJECT_DEPENDS 
${json})
  >
  
  If this patch fixes the dependency arc, then possibly that (expensive?) 
grepping around isn't needed either.
  
  I'll try to do some Makefile-dissection.

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-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 the dependency between the JSON 
file that needs to be generated and automoc running over the cpp source file to 
generate the moc file based on the referenced JSON file.
  So in a highly parallel build the automoc is run before the JSON file is 
generated.
  
  Something which e.g. is tried to be solved by the code in the 
`kcoreaddons_add_plugin` macro, by grepping over all the source files to find 
the cpp file which references the JSON file and then create the dependency by
  
set_property(SOURCE ${dependent_sources} APPEND PROPERTY OBJECT_DEPENDS 
${json})
  
  Thing is that Plasma only uses the `kcoreaddons_desktop_to_json` macro, so 
this is why it runs the chance to fail here, while all other projects which 
create the JSON still on the fly, also call `kcoreaddons_add_plugin`, cmp. 
https://lxr.kde.org/search?_filestring=&_string=kcoreaddons_add_plugin&_casesensitive=1
  
  But not further explored, so still curious if that really is the case or if 
you found the real reason.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D10450

To: tcberner, #freebsd, mpyne, bshah, dfaure
Cc: kossebau, #frameworks, michaelh


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' and then depends on
  it instead of the output file.
  
  This (hopefully) fixes the build failure noticed in the FreeBSD (and some 
linuxes)
  
  - 
https://build.kde.org/view/OS%20-%20FreeBSD/job/Plasma%20plasma-desktop%20kf5-qt5%20FreeBSDQt5.9/138/console
  - https://mail.kde.org/pipermail/kde-freebsd/2018-February/027372.html
  - https://www.mail-archive.com/kde-bugs-dist@kde.org/msg207825.html

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10450

AFFECTED FILES
  KF5CoreAddonsMacros.cmake

To: tcberner, #freebsd, mpyne
Cc: #frameworks, michaelh