Change the way we ship nlohmann::json
Previously we just bundled json.hpp and included it locally. This MR does the following:
-
Introduce a flagACTS_USE_EXTERNAL_NLOHMANN_JSONwhich defaults to OFF.If it is OFF, cmake will download a version ofnlohmann::jsoninto the build folder at configure time, and useadd_subdirectoryto import it. Targets using it can just dotarget_link_libraries(ActsJsonPlugin PUBLIC nlohmann_json::nlohmann_jsonand then include it like#include <nlohmann/json.hpp>. At install time, the single header version ofnlohmann::jsonwill 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 callfind_package(nlohmann_json 3.2.0 REQUIRED). This will make the same targetnlohmann_json::nlohmann_jsonavailable, so no other changes should be required.
After discussion: revised strategy
-
Introduce a flag
ACTS_USE_BUNDLED_NLOHMANN_JSONwhich defaults to ON.- If it is ON, cmake will
add_subdirectorythe bundled version inthirdparty/nlohmann_json. Targets using it can just dotarget_link_libraries(ActsJsonPlugin PUBLIC nlohmann_json::nlohmann_jsonand then include it like#include <nlohmann/json.hpp>. At install time, the single header version ofnlohmann::jsonwill 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 targetnlohmann_json::nlohmann_jsonavailable, so no other changes should be required.
- If it is ON, cmake will
-
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::jsonwhich 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