nlohmann_json: add configuration options - #30784
Conversation
|
Hi @vic-elgato Unfortunately I dont think these options are something we would accept
That would be the expected behaviour because it matches what the nlohmann documentation describes: https://json.nlohmann.me/features/macros/ - it says the following:
So to me it sounds like this isn't something the recipe should be doing automatically for consumers, but rather, something that consumer needs to do at inclusion time, simply as is documented. We've had issues in the past with recipes that do exactly what you are proposing, impeding users who actually want different translation units on the consumer side to be built with different macros. We would go from a single package that can satisfy those uses cases, to a package that sets options it for the entire graph. |
|
Hi @jcar87 Thanks for the explanation, and I get the concern about not blocking people who configure things per translation unit. Where I'm coming from is that these macros don't really behave like purely local choices. The remaining configuration macros aren't encoded in the ABI namespace, but that doesn't necessarily make differing configurations between TUs safe. They can result in different definitions or available APIs for header-defined entities while still using the same It also isn't only my own code once other packages pull the nlohmann headers into my TUs. For example, json-schema-validator exposes nlohmann types through For the per-TU case itself, I don't think the added options prevent it. They're still plain macros, so a TU that wants a different value can still redefine it before including the header. The Conan options are a baseline for the package graph, not a lock, and their defaults match upstream, so nothing changes for consumers who don't set them. For reference, #30752 (stduuid) went in yesterday with the same shape: header-only, with options mapped to If you'd still rather not have these in the recipe I understand, I mainly wanted to lay out the reasoning behind it. |
Summary
Changes to recipe: nlohmann_json/all versions
Motivation
nlohmann/json is configured almost entirely through preprocessor macros (e.g. implicit conversion, enable exceptions, extended diagnostics etc). The recipe didn't expose any of them, so the only way to change that behavior was to define the macros by hand in your own package.
That works, but it has to be repeated in every consumer and it's easy to get wrong: the macros have to be defined consistently everywhere the headers are included, and some of them (JSON_DIAGNOSTICS in particular) change the layout of basic_json. Being able to set this once as a Conan option, or in a profile, is a lot harder to get wrong and helps especially with shared dependencies across packages.
Details
Adds an option per documented configuration macro, all defaulting to the same behavior as before, so nothing changes unless you set one. Since the macros were introduced in different releases, config_options() drops the ones a given version doesn't know about, and setting them there fails with "option doesn't exist" instead of quietly doing nothing.
Worth noting for review: the value macros are defined also when left at their default rather than only when they differ from the header default. That keeps the behavior pinned to the option, and makes a disagreement with a package that defines the same macro itself show up as a macro redefinition warning instead of one value silently winning on link order.
json-schema-validator does this today with JSON_DIAGNOSTICS, and should switch to the new option once this is in.
New options (see: https://json.nlohmann.me/features/macros/):
Add a 👍 reaction to pull requests you find important to help the team prioritize, thanks!