ROOT dictionaries and continuous integration

ROOT
1

Dear All,

I wasn’t entirely sure where/how to bring up this topic. But hopefully experts will be noticing this post.

We switched the ATLAS build system to CMake in the recent past. And in the very recent past we moved all our code into CERN’s GitLab. Where we run CI jobs on every merge request going into the repository. As such, our build system needs to be able to handle all sorts of code changes in a smart way. Recognising all the different cases that require some code to be re-built for some reason.

Luckily CMake gives very good support for incremental builds for the operations that it itself supports/provides. But for operations that we implement ourselves, we’re much more on our own.

With this lengthy introduction, we ran into an issue with dictionary generation in the CI system. In the very first iteration of our CMake code the dictionary generation was set up using a custom command like:

add_custom_command( OUTPUT ${dictsource} ${pcm_name} ${rootmap_name}
      COMMAND ${CMAKE_COMMAND} -E make_directory
      ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}
      COMMAND sh ${CMAKE_CURRENT_BINARY_DIR}/make${dictname}ReflexDict.sh
      DEPENDS ${ARG_HEADER} ${ARG_SELECTION} )

The important part here is that the command was only set up to explicitly depend on changes in the “dictionary header” (a single header file that includes all headers that are needed for the dictionary generation) and the selection.xml file.

I recently realised however that this doesn’t cover the case when one of the actual headers is modified. Or a header very much down the inclusion chain is modified. So I updated the code like this:

add_custom_command( OUTPUT ${dictsource} ${pcm_name} ${rootmap_name}
      COMMAND ${CMAKE_COMMAND} -E make_directory
      ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}
      COMMAND sh ${CMAKE_CURRENT_BINARY_DIR}/make${dictname}ReflexDict.sh
      DEPENDS ${ARG_HEADER} ${ARG_SELECTION} ${pkgName}Pkg
      IMPLICIT_DEPENDS CXX ${ARG_HEADER} )

Making use of add_custom_command's IMPLICIT_DEPENDS feature.

https://cmake.org/cmake/help/v3.0/command/add_custom_command.html

However, apparently CMake doesn’t handle dependencies correctly with this. We started encountering a lot of issues in our CI system where a merge request tried to introduce a new class, but was rejected. And the next CI job that tried to run on the same node was stuck still looking for the new class’s header that was no longer in the source tree.

Now, I’ll be opening a ticket about that for CMake itself. But, this all made me wonder: How do people handle dictionary generation with CMake these days? Such that it would even be CI friendly. Because the GENERATE_ROOT_DICTIONARY function in https://github.com/root-project/root/blob/master/cmake/modules/RootNewMacros.cmake is not CI friendly. It can’t detect it if one of the headers included by a header that’s passed directly to the function, is changed. Did people really not run into issues with this so far?

Cheers,
Attila

2

May be @mato can help you.

3

That was quick, thanks! Yes, I was just about to address Pere, after figuring out his user name in this system.

4

For information, here is the CMake bug report:

5

For me the IMPLICIT_DEPENDS feature works fine after having fixed a little bug in the macro ROOT_GENERATE_DICTIONARY(…). The problem is that the keyword CXX has to precede each header and linkdef file. The current implementation in RootNewMacros.cmake

  add_custom_command(OUTPUT ${dictionary}.cxx ${pcm_name} ${rootmap_name}
                     COMMAND ${command} -f  ${dictionary}.cxx ${newargs} ${excludepathsargs} ${rootmapargs}
                                        ${ARG_OPTIONS} ${definitions} ${includedirs} ${headerfiles} ${_linkdef}
                     IMPLICIT_DEPENDS CXX ${_linkdef} ${_list_of_header_dependencies}
                     DEPENDS ${_list_of_header_dependencies} ${_linkdef} ${ROOTCINTDEP})

is wrong and has to be converted to

  add_custom_command(OUTPUT ${dictionary}.cxx ${pcm_name} ${rootmap_name}
                     COMMAND ${command} -f  ${dictionary}.cxx ${newargs} ${excludepathsargs} ${rootmapargs}
                                        ${ARG_OPTIONS} ${definitions} ${includedirs} ${headerfiles} ${_linkdef}
                     IMPLICIT_DEPENDS CXX ${_linkdef} CXX <header1> CXX <header2> ...
                     DEPENDS ${_list_of_header_dependencies} ${_linkdef} ${ROOTCINTDEP})

In the origin I was probably testing it only for linkdef files inclusions.

6

Created issue https://sft.its.cern.ch/jira/browse/ROOT-8764

7

I’m really not looking carefully enough… For some reason I missed the IMPLICIT_DEPENDS line completely in your code.

This is of course nothing complicated, but I implemented this in the following way in the ATLAS code:

# Set up proper C++ dependencies for the local headers:
set( implicitHeaders )
foreach( header ${localHeaders} )
   list( APPEND implicitHeaders CXX ${CMAKE_CURRENT_SOURCE_DIR}/${header} )
endforeach()
...
   IMPLICIT_DEPENDS ${implicitHeaders} )

Keep in mind though that “fixing” this issue in the ROOT code may actually cause you more grief than joy at this point. I’m just about to remove the usage of IMPLICIT_DEPENDS from the ATLAS code. As the issue described in the CMake ticket is causing us a lot of trouble in the last few weeks.

8

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.