Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide nlohmann/json dependency with multiple headers #1678

Closed
wants to merge 2 commits into from

Conversation

vid512
Copy link

@vid512 vid512 commented Sep 9, 2024

meson.build of nlohmann/json contains additional dependency with multiple headers, instead of single one. This PR allows wrapDB users to use it.

Multiple headers are useful eg. if you want to forward-declare nlohmann::json object (see json_fwd.hpp).

@eli-schwartz
Copy link
Member

IIRC nlohmann_json is never installed this way and the dependency lookup won't ever work for the general case. Not sure this is a good idea...

@vid512
Copy link
Author

vid512 commented Sep 9, 2024

What do you mean "this way"? Via wrapdb? I don't follow.

I just tested it:

  • installed dependency wrap with meson wrap install nlohmann_json,
  • manually edited subprojects/nlohmann_json.wrap as in c1b781c, to simulate this PR
  • included dependency('nlohmann_json_multiple_headers') in meson.build
  • now cpp file with #include <nlohmann/json_fwd.hpp> and forward-declared nlohmann::json compiles just fine

@dcbaker
Copy link
Member

dcbaker commented Sep 9, 2024

Me either. I'm looking, and I would rather see if upstream would take an update to their Meson. I'm opening a PR right now for that purpose. (I use this project downstream, so I have some interest in making this better)

@dcbaker
Copy link
Member

dcbaker commented Sep 9, 2024

nlohmann/json#4452

@vid512
Copy link
Author

vid512 commented Sep 9, 2024

OK I see my mistake now. For whatever reason I thought json_fwd.hpp is only available with multiple headers, I completely missed that json_fwd.hpp is in single_include as well. Yes, this is pointless. At least it prompted update upstream. :)

@vid512 vid512 closed this Sep 9, 2024
@eli-schwartz
Copy link
Member

The other factor here is that the usual name is pkg-config --cflags nlohmann_json or for cmake, find_package(nlohmann_json).

Implementing a new dependency name would mean it only worked as a wrap, but not as a system package, which is a naming wart and probably a leaky internal implementation detail. And implementation details could probably be used by projects that really want them, via subproject().get_variable().

It would generally only matter for projects that don't care about the forward declarations, but rather want to overly specify whether they use amalgamated headers.

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.

3 participants