Skip to content

Reflex Dictionary Fix, master branch (2021.03.11.)

This is to fix the problem with header paths reported in ATEAM-717.

The issue was a bit convoluted... (As one would expect it to be...)

For "old style EDM packages" the atlas_add_dictionary(...) function supports generating smart pointer dictionaries automatically using its optional NAVIGABLES, DATA_LINKS, etc. arguments. This is something that I never liked, as it hardcodes information about our EDM into the core of the build system, but this was the most pragmatic way of doing this during the CMT->CMake transition. However when introducing automatic dictionary generation for the xAOD packages recently, I rather went with coding that logic up in instead.

Since some headers are put under ${CMAKE_CURRENT_BINARY_DIR}${CMAKE_FILES_DIRECTORY} during the auto-generation of the smart pointer dictionaries, the atlas_generate_reflex_dictionary(...) function was taught to take this directory into account in a pretty silly way. It would always add the directory of the "dictionary header file" to the list of include paths given to genreflex.

As long as one used the NAVIGABLES, DATA_LINKS, etc. arguments, this resulted in the desired behaviour. With genreflex receiving exactly the include paths that it needed. But for dictionaries that did not make use of those arguments, it meant that ${CMAKE_CURRENT_SOURCE_DIR} and ${CMAKE_CURRENT_SOURCE_DIR}/<header dir> would both be added to the list of include directories. Which meant that genreflex would record the paths of the header files without <header dir>. Resulting in exactly the behaviour reported in ATEAM-717.

I played around a bit with if( ... MATCHES ... ) to try to keep the current logic for the include paths, but with an additional check, but just couldn't make that work reliably. 😦 So now I just hardcoded that atlas_generate_reflex_dictionary(...) would always add ${CMAKE_CURRENT_BINARY_DIR}${CMAKE_FILES_DIRECTORY} to the include paths passed to genreflex as well. Which I believe should be safe enough.

At least in all the tests I've done, this did fix the paths in the generated .rootmap files...

Merge request reports