Skip to content

Conversation

@ferdymercury
Copy link
Collaborator

@ferdymercury ferdymercury commented Jan 5, 2026

exploring here if this might be a reason why sometimes one gets 'out of date' modules?

rootclingIO.cxx uses RIO and Core classes.

I am not sure if BUILTINS is working well: with add_dependencies explicitly, I got a cyclic dependency error.

see occurrence: https://github.com/root-project/root/actions/runs/20365372252/job/58519132341?pr=20758
and #7704

I also attach here the dependency graph by

 cmake --graphviz=dep.dot /opt/root_src
 dot -Tsvg dep.dot -o dep.svg

dep

using CMakeGraphVizOptions.cmake in the source or binary dir:

# https://embeddeduse.com/2022/03/01/visualising-module-dependencies-with-cmake-and-graphviz/
set(GRAPHVIZ_EXTERNAL_LIBS FALSE)
set(GRAPHVIZ_IGNORE_TARGETS "LLVM*" ".*clang*" ".*resource-headers" "G__*")
set(GRAPHVIZ_EXECUTABLES FALSE)

Btw this SVG graph might be useful to add in the Doxygen docu @couet

I do it as follows in my projects:

Somewhere in the index.md file I write

A graph of CMake target dependencies can be found here:
\dotfile dependencies.dot

and in the CMake:

    configure_file(${CMAKE_CURRENT_SOURCE_DIR}/CMakeGraphVizOptions.cmake ${CMAKE_BINARY_DIR}/CMakeGraphVizOptions.cmake COPYONLY)
    add_custom_target(graphviz
        COMMAND ${CMAKE_COMMAND} "--graphviz=SOME_OUTPUT_DIRECTORY/dependencies.dot" ${CMAKE_SOURCE_DIR}
        WORKING_DIRECTORY "${CMAKE_BINARY_DIR}"
    )

In the Doxyfile:
DOTFILE_DIRS = SOME_OUTPUT_DIR

might be a reason why sometimes one gets 'out of date' modules?
@hageboeck
Copy link
Member

hageboeck commented Jan 5, 2026

Hello @ferdymercury,

happy new year!

I don't think this is the cause for the out-of-date errors. I suspect that the message about pcms being out of date is actually inverted. That is, you rebuild a pcm that's at the centre of the dependency graph, e.g. IO or Core, and a pcm that's more at the leaves doesn't get rebuilt. In that case, it will say that Core is out of date and needs to be rebuilt, but actually it's let's say "RooFit.pcm" that should have been rebuilt. I wanted to have a look a this for while, but didn't get to it. You probably know that you can circumvent these errors by simply deleting all pcms and rebuilding. If I'm right, in the above case, it would be sufficient to delete the RooFit pcm, even if ROOT says that Core is out of date.

Anyway, if the above is correct or partly correct, we don't need to to anything about RootPcmObjs. In particular, it doesn't need to depend on Core or IO. The dependency is actually the other way around, and that's why you got a cyclic dependency error. What it needs instead to build the target is rootcling. This is not specified explicitly, but somewhere in our CMake config there is a rule that rootcling has to be ready before dictionaries can be built.

As a last comment, BUILTINS should only be used for builtins, not for ROOT targets.

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Test Results

    19 files      19 suites   3d 4h 4m 35s ⏱️
 3 792 tests  3 788 ✅ 0 💤 4 ❌
69 070 runs  69 066 ✅ 0 💤 4 ❌

For more details on these failures, see this check.

Results for commit a25d222.

@ferdymercury
Copy link
Collaborator Author

ferdymercury commented Jan 5, 2026

Hi Stephan, happy new year!

Just for my understanding: we have src/rootclingIO.cxx using #include "TClassEdit.h"
We have built now ROOT successfully.

Image now that we modify TClassEdit.h, for example modifying one function signature, but we forget to adapt the signature in src/rootclingIO.css, and try to recompile incrementally. How does Ninja know that it has to rebuild RootPcmObjs ? Wouldn't it say 'all is fine' even if it should say "compilation error", rootclingIO.cxx function XYZ does not exit.

Or we hope that this does not happen very often and otherwise do a clean-build label?

Likewise, we are using TFile and TStreamerInfo from within rootclingIO.cxx, so RootPcmObjs should depend on IO, or these classes should be avoided??

@hageboeck
Copy link
Member

Looking at the top of RootPcmObjs, it is a source file that provides just a few bindings between rootcling and TCling. It therefore doesn't really have to do with pcms, at least on the surface. Given that we define an object library, the only thing that happens is that it gets compiled into a .o file. It doesn't get linked.
To see where it goes (and on what it depends when linked), you have to see where it's used, and that's in libRIO. So what you have now is an object files that's part of RIO, and the latter depends on Core.

Now, let's answer

Image now that we modify TClassEdit.h, for example modifying one function signature, but we forget to adapt the signature in src/rootclingIO.css, and try to recompile incrementally. How does Ninja know that it has to rebuild RootPcmObjs ? Wouldn't it say 'all is fine' even if it should say "compilation error", rootclingIO.cxx function XYZ does not exit.

Unless CMake treats object libraries differently, rootclingIO.cxx should be treated like it's part of libRIO. If TClassEdit.h changes, Core changes, so its dependencies should get recompiled like all the other files in libRIO that depend on a file from a different target. If the headers they include change, the object file is regenerated, and the library is relinked.

@ferdymercury
Copy link
Collaborator Author

Thanks for the clarification!

@ferdymercury ferdymercury deleted the ferdymercury-patch-1 branch January 5, 2026 15:05
@hageboeck
Copy link
Member

I tested what I just said:

$ touch ../core/foundation/inc/TClassEdit.cxx
$ ninja -v -j1 | grep rootclingIO.cxx
...
[8/1124] /usr/bin/c++  [...] -MD -MT io/rootpcm/CMakeFiles/RootPcmObjs.dir/src/rootclingIO.cxx.o -MF io/rootpcm/CMakeFiles/RootPcmObjs.dir/src/rootclingIO.cxx.o.d -o io/rootpcm/CMakeFiles/RootPcmObjs.dir/src/rootclingIO.cxx.o -c root/io/rootpcm/src/rootclingIO.cxx

So this shows that it gets compiled very early.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants