What is the intended way to consume this repository if supplying your own nlohmann_json?

[eike-fokken]

Hi!

I see that you made big changes to the CMakeLists.txt files. I very much like that you go to the hassle, instead of "never touching a running system" and collecting technical dept.

Unfortunately/Luckily I am one of those people, who now have to follow suit.

In my project https://github.com/eike-fokken/grazer/ I can now not merge your latest commit, because I include both the schema validator and nlohmann_json itself as a submodule.

I can just remove nlohmann_json and rely on the validator to fetch it for me but I prefer to have the dependency fetching done via

git submodule update --init --recursive

instead of in the cmake configure step. This is so, because I sometimes don't have internet (German trains...) and sometimes the easiest way to remove cmake hiccups is to remove the build directory and reconfigure.

Therefore I ask: Can I force the schema validator to use my own cmake target nlohmann_json, which is provided by the line

add_subdirectory(nlohmann_json)

in my CmakeLists.txt before I include this repository.

If I do so right now, I get this error:

CMake Error at gcc_debug/_deps/nlohmann_json-src/CMakeLists.txt:103 (add_library):
  add_library cannot create target "nlohmann_json" because another target
  with the same name already exists.  The existing target is an interface
  library created in source directory "/home/eike/git/grazer/nlohmann_json".
  See documentation for policy CMP0002 for more details.

Note: My cmake is 3.25.1.

[pboettch]

It would be ideal if we can fix it that this special behavior to get nlohmann::json will work. @LecrisUT Could you take a look at it? (I'm asking because it was you who changed, and I'm a little bit out of time currently)

[LecrisUT]

Sure, no problem. @eike-fokken The modern way of doing this with submodules , i.e. to set FETCHCONTENT_SOURCE_DIR_NLOHMANN_JSON to the path of the submodule. This is usually done only by the top-level cmake script in order to allow the user and downstream to set their appropriate override as well. It is not recommended to use only add_subdirectory without providing an option to override.

I actually am a bit reluctant towards #279 because the user should either use system installed version via find_package or set FETCHCONTENT_BASE_DIR if size download is an issue. The project structure in that look-aside repo is vulnerable to changes in upstream cmake file and restructurings.

(Btw, @eike-fokken, don't use detached heads. Do ping to remind to push an update, but there are a few tasks leftover in the CI that I have overlooked)

[pboettch]

@LecrisUT Oh, sorry to not have consulted you before, I thought you would intervene automatically. If needed, I'll revert the commit if there is a better solution.

I think the most important step is to update the README to reflect the changes or add missing working functionalities.

[LecrisUT]

Sorry, I don't have this repo watched atm, and I am a bit jumping back-and-forth between projects. Do ping me and I can take a look at stuff.

Yeah, the readme can use some revamping. Can you ping me in the weekend and I can setup a RTD doc for this.

[eike-fokken]

Sure, no problem. @eike-fokken The modern way of doing this with submodules , i.e. to set FETCHCONTENT_SOURCE_DIR_NLOHMANN_JSON to the path of the submodule. This is usually done only by the top-level cmake script in order to allow the user and downstream to set their appropriate override as well. It is not recommended to use only add_subdirectory without providing an option to override.

Thanks, that's what I wanted to know.

(Btw, @eike-fokken, don't use detached heads. Do ping to remind to push an update, but there are a few tasks leftover in the CI that I have overlooked)

I don't know, what you mean here. Where did I enter a detached head?

[pboettch]

Could you try using FETCHCONTENT_SOURCE_DIR_NLOHMANN_JSON in your case, where you do the sub-module download yourself, with the repository of your choice.

Then we can allow FetchContent to download the full repo by default and special cases as yours would not download anything.

[LecrisUT]

I don't know, what you mean here. Where did I enter a detached head?

I am referring to the git submodule references in your repo.

[eike-fokken]

Oh, it's about my own repository. Thanks for the hint. I just still don't know, what the actual mistake is, you found there. Is it, that I configure many of the submodules as "shallow"? I did have some problems with that. I chose to do that to again reduce download sizes, because basically I never want to actually have the history of my submodules, just the most recent release. But I would be happy to get another hint on how to do that properly.

[LecrisUT]

Oh, it's about my own repository. Thanks for the hint. I just still don't know, what the actual mistake is, you found there.

I can't find a way to show this in GH ui, but your repo is pointing to cae6fad80001510077a7f40e68477a31ec443add. It's best if it's pointing to a tagged release in most cases

[eike-fokken]

Oh, yes, that's because I still havn't fixed my issue and therefore havn't updated the dependency, yet.

[LecrisUT]

Btw, you might not have caught my edit, but the practice is to point to tags, not potentially detached commits. Unfortunately git submodules cannot point to a branch specifically, but that's where FetchContent compensates. I.e. you point the git submodule to the last (preferably tagged) repo, while the FetchContent and CI can point to git branch.

[eike-fokken]

I learned a lot here, my question is answered. Thanks a lot!

[pboettch]

I learned a lot here, my question is answered. Thanks a lot!

Same for me ;-)

[craigscott-crascit]

FWIW, there's a couple of statements made in the discussion here which I think are incorrect (I'm the author of the FetchContent module and one of the CMake maintainers).

i.e. to set FETCHCONTENT_SOURCE_DIR_NLOHMANN_JSON to the path of the submodule. This is usually done only by the top-level cmake script in order to allow the user and downstream to set their appropriate override as well.

The FETCHCONTENT_SOURCE_DIR_<PkgName> variables should not be set by the project. They are meant to be a developer control so the developer can change them locally to override where a particular dependency comes from to point at any location they want.

I can't find a way to show this in GH ui, but your repo is pointing to https://github.com/pboettch/json-schema-validator/commit/cae6fad80001510077a7f40e68477a31ec443add. It's best if it's pointing to a tagged release in most cases

It's perfectly fine to point at a git hash, and in some cases it absolutely should be done. The only definitive and reliable way to refer to a specific commit is by its commit hash. Tags can move, so pointing at a tag instead of a commit hash is technically not 100% reliable. Usually, tags shouldn't move, but it can (and does) occur. If you're willing to accept that caveat, then using a tag can be more friendly to humans, but you can achieve the same thing by putting a comment with the tag name after the commit hash. To be clear, using a tag is fine if you accept that caveat, but using a commit hash is also fine.

Btw, you might not have caught my edit, but the practice is to point to tags, not potentially detached commits. Unfortunately git submodules cannot point to a branch specifically, but that's where FetchContent compensates. I.e. you point the git submodule to the last (preferably tagged) repo, while the FetchContent and CI can point to git branch.

In general, don't point FetchContent at branches. That makes your builds non-repeatable, and it also forces each CMake configure run to contact the remote. If you use a commit hash, there is no network communication if you already have the commit checked out from a previous run. You also avoid network communication if you use a tag, but this might only be for more recent CMake versions (earlier ones may still contact the remote due to some implementation bugs).

[eike-fokken]

Thanks for the input directly from the source! May I ask how you happened upon this closed issue?

[craigscott-crascit]

I was bringing json-schema-validator (latest release 2.2.0) into a consulting client's project using FetchContent, and they were already bringing nlohmann_json into their project that way. For that tagged release, the start of the top level CMakeLists.txt contains cmake_minimum_required(VERSION 3.2), and that generates a deprecation warning with CMake 3.27 or later. I checked that file on the latest main branch and saw that it had been updated by #262. After following a few links, I ended up here.