DPP icon indicating copy to clipboard operation
DPP copied to clipboard

DPP_USE_EXTERNAL_JSON still uses internal JSON when building bots

Open benthetechguy opened this issue 5 months ago • 7 comments

Git commit reference e97ae1903a3a7fc5e0a592b564390f501afd6e56 (10.1.2)

Describe the bug In packaging this for Debian, I removed include/dpp/nlohmann and set -DDPP_USE_EXTERNAL_JSON=ON because Debian packages are supposed to use the packaged versions of libraries when possible. When I build D++, it does in fact use the external nlohmann::json, but when I try to use it in a bot, it tries to include the internal JSON headers and not the external ones, unless I define DPP_USE_EXTERNAL_JSON in my bot's cmake.

To Reproduce Steps to reproduce the behavior:

  1. Build D++ with -DDPP_USE_EXTERNAL_JSON=ON
  2. Build a bot using the D++ you just built
  3. Notice that it is including dpp/nlohmann/json.hpp rather than nlohmann/json.hpp, unless you also define DPP_USE_EXTERNAL_JSON in your bot.

Expected behavior When D++ is built with -DDPP_USE_EXTERNAL_JSON=ON, the files include/dpp/json.h and include/dpp/json_fwd.h should be modified to include the external nlohmann::json rather than the internal one, rather than using a preprocessor directive to determine if that flag is set, because then that flag also has to be set for everything that uses D++, rather than sticking when it is used in the original D++ build.

System Details:

  • OS: Debian sid

benthetechguy avatar Jul 01 '25 14:07 benthetechguy

This is kind of expected, as nlohmann::json is header only. it stores no state to indicate how it is to be used by downstream packages. DPP could store such state but it would have to be compatible with all the package formats and platforms we support. Suggestions welcome.

braindigitalis avatar Jul 01 '25 18:07 braindigitalis

Meh we could easily propagate this at least for manual cmake build & installs

Mishura4 avatar Jul 01 '25 19:07 Mishura4

sure,for cmake it "just works", but no everyone uses cmake... We would likely have to write out a config.h with #defines in it

braindigitalis avatar Jul 01 '25 20:07 braindigitalis

Could you have a include/dpp/json.h.in and tell cmake to generate the header to include either nlohmann/json.hpp or dpp/nlohmann/json.hpp depending on whether DPP_USE_EXTERNAL_JSON is on or off? I'm sure there is a similar solution for all supported build tools. If not, a config.h that does this definition would be a viable solution.

benthetechguy avatar Jul 01 '25 21:07 benthetechguy

i dont like the idea of dynamically changing those files directly in that way. i prefer to keep generated content separate. Similar idea, except i propose we make a separate folder e.g. include/dpp/gen for storing such files that can vary based on the cmake output. It makes it easier to filter them all with gitignore, and less likely some developer might try to manually change them.

braindigitalis avatar Jul 02 '25 08:07 braindigitalis

This issue has had no activity and is being marked as stale. If you still wish to continue with this issue please comment to reopen it.

github-actions[bot] avatar Sep 01 '25 03:09 github-actions[bot]

This issue has had no activity and is being marked as stale. If you still wish to continue with this issue please comment to reopen it.

github-actions[bot] avatar Nov 01 '25 02:11 github-actions[bot]