D10450: Generate a custom target in kcoreaddons_desktop_to_json

[Friedrich W. H. Kossebau]

kossebau added a subscriber: kfunk.
kossebau added a comment.


  In D10450#204762 <https://phabricator.kde.org/D10450#204762>, @adridg wrote:
  
  > In D10450#204623 <https://phabricator.kde.org/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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180212/03514c05/attachment.html>