PODIO_JSON_OUTPUT not set downstream, but json support still compiled into libraries

[wdconinc]

Note: this is a lazy copy from mattermost

This seems to be caused as follows:

• during the build of EDM4hep, PODIO_JSON_OUTPUT is defined and libedm4hepDict.so includes the to_json function (Component.h.jinja2 is inline, but others aren't e.g. Collection.cc.jinja2).
• during compiling with ROOT cling, the #define isn't set so it gets confused.

Here is a reproducer in our production environment which also works on my local spack with dd4hep, podio, edm4hep installed:

ddsim --compactFile $DD4hepINSTALL/DDDetectors/compact/SiD.xml -G -N10 --outputFile SiD.edm4hep.root
root -l -q 'podio_issue.cxx+g("SiD.edm4hep.root")'

where podio_issue.cxx is

#include "ROOT/RDataFrame.hxx"
#include "TCanvas.h"

#include "edm4hep/MCParticleCollection.h"

int podio_issue(const char* fname)
{
  ROOT::RDataFrame df("events", fname);

  auto df0 = df.Define("n", "MCParticles.size()");
  auto h_n = df0.Histo1D({"h_n", "; N ", 10, 0, 10}, "n");

  auto c = new TCanvas();
  h_n->DrawCopy();
  return 0;
}

That returns

Processing podio_issue.cxx+g("SiD.edm4hep.root")...
libedm4hepDict dictionary payload:267:7: error: no matching constructor for initialization of 'nlohmann::json' (aka 'basic_json<>')
  j = nlohmann::json{{"location", value.location},
      ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/local/include/nlohmann/json.hpp:892:5: note: candidate constructor not viable: requires at most 3 arguments, but 9 were provided
    basic_json(initializer_list_t init,
    ^
/opt/local/include/nlohmann/json.hpp:1007:5: note: candidate constructor not viable: requires 2 arguments, but 9 were provided
(etc)

[tmadlener]

I can also reproduce this in my development environment. Quickest workaround for now seems to be to add a #define PODIO_JSON_OUTPUT to each macro that wants to use this. Obviously that is not a longterm solution.

(Adding a few thoughts that we already partially discussed in private):
I gave putting all to_json function implementations into the respective headers a quick go, and that doesn't really work because then we would also have to start replacing forward declarations with #includes of the corresponding headers, which most likely is not viable because relations can lead to cyclic includes here.

I am also not sure whether we can make, e.g. a #define EDM4HEP_PODIO_OUTPUT that we implicitly generate based on the value of PODIO_JSON_OUTPUT work nicely. Maybe the cleanest solution would be to make this explicitly opt-in by creating a new option in the YAML file that enables / disables the json conversion.

[wdconinc]

Another thought. Maybe we can make the current approach work? I don't actually understand what goes wrong in cling. Clearly it finds the dictionary and the (correct, for my system) nlohmann-json headers. So I don't really understand why it thinks the constructor signature doesn't match? Is this maybe due to an implicit c++ option that affects initializer lists?

[tmadlener]

Looking at the error message again, I think the problem is actually in an inline definition of a to_json function, as it is in TrackState.h which is a component. Not sure yet what that teaches us about the problem.

[tmadlener]

Maybe it would be easiest to remove the #ifdef PODIO_JSON_OUTPUT with a

#if __has_include("nlohmann/json.hpp")
  #include "nlohmann/json.hpp"
  // potentially a version check
  #define PODIO_JSON_OUTPUT // or something else for easier internal use
#endif

in that way we would at least not depend on some external -DPODIO_JSON_OUTPUT, but would have some (maybe) consistent internal check.

[wdconinc]

I think it seems like the Components just aren't guarded. Nevermind, that didn't make sense.