Skip to content

Change the way we ship nlohmann::json

Paul Gessinger-Befurt requested to merge json-build into master

Previously we just bundled json.hpp and included it locally. This MR does the following:

  • Introduce a flag ACTS_USE_EXTERNAL_NLOHMANN_JSON which defaults to OFF.
    • If it is OFF, cmake will download a version of nlohmann::json into the build folder at configure time, and use add_subdirectory to import it. Targets using it can just do target_link_libraries(ActsJsonPlugin PUBLIC nlohmann_json::nlohmann_json and then include it like #include <nlohmann/json.hpp>. At install time, the single header version of nlohmann::json will be installed alongside Acts (we want this because it's part of the public API of the JSON plugin.
    • If it is ON, cmake will just call find_package(nlohmann_json 3.2.0 REQUIRED). This will make the same target nlohmann_json::nlohmann_json available, so no other changes should be required.

After discussion: revised strategy

  • Introduce a flag ACTS_USE_BUNDLED_NLOHMANN_JSON which defaults to ON.

    • If it is ON, cmake will add_subdirectory the bundled version in thirdparty/nlohmann_json. Targets using it can just do target_link_libraries(ActsJsonPlugin PUBLIC nlohmann_json::nlohmann_json and then include it like #include <nlohmann/json.hpp>. At install time, the single header version of nlohmann::json will be installed alongside Acts (we want this because it's part of the public API of the JSON plugin.
    • If it is OFF, cmake will just call find_package(nlohmann_json 3.2.0 REQUIRED). This will make the same target nlohmann_json::nlohmann_json available, so no other changes should be required.

  • Also I'm doing a bit of cleanup in the JSON plugin, the test wasn't actually built, and the header files had global namespace aliases like using json = nlohmann::json which is not ideal.

  • Change the minimum cmake version required to 3.11. That's available in LCG95, and we currently don't support versions before that.

The idea is that in the ATLAS externals build, where another package already "builds" nlohmann::json we can pick that up instead of rolling our own. This was brought up by @akraszna in atlas/atlasexternals!605 (closed).

@akraszna thoughts? (Or anyone else?)

Edited by Paul Gessinger-Befurt

Merge request reports