Json: CMakeLists.txt in release zips?

[eliasdaler]

Hello, I've tried using FetchContent CMake module and this doesn't work as well as I expected. The problem is that after I download and unpack the zip, there's no CMakeLists.txt file, so I can't just get nlohmann_json target and link with it.

If you take a look at libraries like fmt, they include CMakeLists.txt in their releases, which makes fetching zips and then getting targets at configure step quite easy.

So, can we have CMake related files included in zips for future releases?

[eliasdaler]

On the other hand, having license file and some tests might also be useful. (Just not a lot of MBs ones)

So, a sort of trimmed down sources.tar.gz version of this repo.

But this is just "nice to have" and arguable. CMake files on the other hand are more important to have.

[nlohmann]

What's wrong with using the zip archive of the sources?

[eliasdaler]

Its size - 109 mb. It takes a while to download at slower connections and makes a built folder quite large after it unpacks (unpacking is also done at configuration time and takes some time)

It's the most significant bottleneck for my project right now. Just includes are several kb. Downloading them is really fast, but not having CMake makes them not configurable via CMake easily.

[eliasdaler]

By the way, for those who want to use FetchContent now, this works fine:

include(ExternalProject)

FetchContent_Declare(nlohmann_json
  URL https://github.com/nlohmann/json/releases/download/v3.1.2/include.zip
)

FetchContent_Populate(nlohmann_json)

# temporal, until releases don't include CMakeLists.txt! After, just use:
# add_subdirectory(${nlohmann_json_SOURCE_DIR} ${nlohmann_json_BINARY_DIR})
add_library(nlohmann_json INTERFACE)
target_include_directories(nlohmann_json INTERFACE ${nlohmann_json_SOURCE_DIR})

# and now you can do this:
# target_link_libraries(your_target nlohmann_json)

[nlohmann]

We‘re already supporting a lot of package managers, so I am not really willing to spend more efforts in adding more files to the release.

[OvermindDL1]

@nlohmann That's not a manager of any form, without a CMakeLists.txt in the zip that means add_subdirectory and so forth can't be used, have to link manually instead.

[eliasdaler]

Agreed. The only thing that needs to be added is CMakeLists.txt and (possibly) cmake/config.cmake.in

CMake is not only used for building the library itself, it's also a means to easily use it, as can be seen from my example. And when you have this, you can support a lot of package managers. And this is with several kb zip instead of 100 mb zip.

[nlohmann]

I already do support a lot of package managers, and up to now this is the first time someone wants the CMakeLists.txt in the release.

[theodelrieu]

It could be easier if someone were to publish the library with CPack? I never used it but you could use it with CMake directly too

[OvermindDL1]

I already do support a lot of package managers, and up to now this is the first time someone wants the CMakeLists.txt in the release.

Well I grab it either via hunter or just a straight git clone, so I get a cmake file regardless. ^.^

It could be easier if someone were to publish the library with CPack? I never used it but you could use it with CMake directly too

Oh wait, cpack is not already being used to make the release? What is it a shell script or something instead? That doesn't really generate the config file properly...

[nlohmann]

I shall try cpack.

[eliasdaler]

I already do support a lot of package managers, and up to now this is the first time someone wants the CMakeLists.txt in the release.

They are supported if the user clones the whole repo, right? Up until now, I've manually cloned the repo, removed everything except includes and cmake files and then stored the stripped repo on disk.

If there was a way to clone only some files from a repo, maybe I wouldn't ask for CMake inside the release zips.

It could be easier if someone were to publish the library with CPack? I never used it but you could use it with CMake directly too

While CPack might automate the process, it still depends on what goes in the zip. And if CPack config is in some cmake file and that file won't be present in one of stripped release zips, this would not solve the problem of not having to clone the whole repo.

[nlohmann]

Alright, CPack is not really helpful if applied to the current package. I could try to exclude files from the package, but I think this is not really helping. Question is: does the current CMakeLists.txt would really help if added to the release? Or would we rather need a stripped down version? If so, we may think of writing a script that puts only the sources and that stripped-down CMakeLists.txt in a folder and run make package_sources on in and use that archive for the release. This would be self-contained and we would not need to attach other files to the release. Right?

[eliasdaler]

Here's some analysis. When I download repo as zip using GitHub, I get 109 MB zip. it's 240 MB when unpacked.

After that I've removed "jeopardy" benchmark and data folders for two tests: "json_nlohmann_tests" and "regression".

After that, I've got 15.3 MB left. It's 7.34 MB when zipped.

So, I think we can have a stripped release version with these tests left out. The user should either clone the whole repo to run them or there should be some script which will download the data if the user specifies that these tests should be built (via some BUILD_EXTRA_TESTS option or something)

[eliasdaler]

It'll probably be nice to use ExternalData module for downloading test data during the build time if the user wants to build these tests.

The question is: where do we store the data files? We can possibly store them in one of the release zips so that they can later be downloaded from GitHub separately.

[OvermindDL1]

Store them off in a branch or another repo.

[nlohmann]

I still would like to keep the repository as is. I was thinking about stripping down the CMakeLists.txt to a minimum required to integrate the library.

[eliasdaler]

I think the repo should stay as it is now. My proposal is just to change contents of CMakeLists a bit to allow external download if test data is not present in the source folder (and it won't be present if we make a release zip with everything except the heavy data).

I previously thought that intergration-only CMakeLists is a good idea, but it introduces two problems:

1) You can't run simple tests with it
2) You have to maintain it separately and write a script to strip out unneeded parts.

I may experiment in a fork with ExternalData and release zips if you agree with my approach. I think it'll work pretty fine. To summarize:

1) If user does git clone, everything works as it is now
2) If user downloads a release zip, it contains everything except for heavy data files. If the user then wants to run benchmark one of the tests which use a lot of data, CMake downloads the needed data files from a separate zip which will be published as a part of "release"

Any problems with that approach?

[nlohmann]

That sounds reasonable. (I wasn't afraid of generating the CMakeLists.txt, as the current one is pretty static right now. But you are right, it's better to have tests as well).

It would be great to see an example for how ExternalData would work.

[eliasdaler]

Okay, I'll try to come up with something this week and I'll notify you once it's somewhat ready for testing. :)

[theodelrieu]

I did play with ExternalData before, I'm confident that this is a good solution to the long-lasting problem of this repo's size.

[graph]

FetchContent is fairly new in cmake. Can CMake developers modify it to allow scans for source files and simply automatically add them all? I think that route would be better overall. Maybe they'll be willing with that request.

[eliasdaler]

We don't need FetchContent for getting test data. We don't need it for configuration time

ExternalProject/ExternalData are in CMake since 3.4, I think.

[nlohmann]

Does anyone has an idea how to proceed here? Volunteers with PRs welcome! :-)

[graph]

For work we have a similar system. What we do is in the CMakeLists.txt we check for some files that are only present in the main repo, and in the distribution are not present. If such a file is detected do the normal thing. If not it's a distribution is assumed and it's setup as a basic library with not much happening. CMake has return() which is helpful for early exit. I think this might be a good direction for this library. FetchContent is fairly new, I haven't used FetchContent.

For some header only libraries we have CMakeLists.txt generate a dummy cpp file that contains a function like

int libraryname_dummy() {
    return 42;
}

then add_library() can have a source which helps workaround some issues and corner cases, I don't remember what the issues are. Maybe put the below just after the project() declaration.

if(NOT EXISTS nlohmann_json.natvis) 
    set(sourceDir "${CMAKE_CURRENT_LIST_DIR}")
    get_filename_component(sourceDir ${sourceDir} ABSOLUTE)

    # Add a library target with the sources of this library
    if(NOT EXISTS ${CMAKE_BINARY_DIR}/nlohmann_json.cpp)
        file(WRITE ${CMAKE_BINARY_DIR}/nlohmann_json.cpp "void nlohmann_json_dummy(){}")
    endif()
    add_library(nlohmann_json STATIC ${CMAKE_BINARY_DIR}/nlohmann_json.cpp json.hpp)

    # include this into the library target
    target_include_directories(nlohmann_json
        PUBLIC
            ${sourceDir}
    )
    return()
endif()

FetchContent is fairly new. CMake developers are nice maybe they will be willing to upgrade FetchContent to support header only zip files and automatically search for headers & source files. Someone should post something in their bug tracker. If it's not in scope for them then something like above would be good. I have no experience with FetchContent someone with experience should comment too regarding this approach.

[OvermindDL1]

then add_library() can have a source which helps workaround some issues and corner cases, I don't remember what the issues are.

I've never seen that be needed thus far? Do you have a reproducible example?

[graph]

it was some corner cases I don't remember. If it's not needed then forget about it, and keep it simple.

[eliasdaler]

We don't need FetchContent for test data, because we don't need it during configuration time. We don't even need it during build time - we need it during test's runtime only, so we can fetch them after the build is done. This can be done with ExternalData package, but from what I've seen, it's a pain to work with.

ExternalProject looks better and it can deal with zips, but I'm still a bit sad about not having a separate repo for test data, because right now repo is unusable as a git submodule due to it's size. :(

[theodelrieu]

ExternalData is not that hard to use, we have to decide where to store the data itself, after that it's just about hashing each file one by one.

I'm using it for one of my projects, I'll try to find some time to put the test data on one of my repos and make a PR. Once it works, @nlohmann could just fork the test data repository.

[nlohmann]

I am sorry if I repeat myself: I would like to keep the repository as is and not split it into a code and a test repository (see
https://github.com/nlohmann/json/issues/1184#issuecomment-410845749). I would be happy to see a proposal like
https://github.com/nlohmann/json/issues/1184#issuecomment-410847649.

[theodelrieu]

I was talking about a data repository, a bit like this one, but the test data can be put anywhere, it does not matter if it's stored in a repository or not, as CMake will fetch individual files anyway.

[theodelrieu]

I opened PR #1251 to solve the repo size problem.

About the original issue (CMakeLists.txt in release zips), wouldn't it be better to include the CMake config files so that find_package works as expected?

[OvermindDL1]

CMake config files

Yes absolutely. However those should be generated on the local system or it may not match and thus may not work.

[theodelrieu]

However those should be generated on the local system

Well, they must be relocatable so I think it is not a problem if we package them.

[eliasdaler]

@theodelrieu yes, having CMakeLists.txt can solve part of the problem. But we need to make sure that it works with only a part of repository being included into release zip.

[OvermindDL1]

Well, they must be relocatable so I think it is not a problem if we package them.

As long as it is installed in the proper locations, but the proper locations on each system can be different. CPack can help there though as it can generate the necessary files when installed.

[theodelrieu]

@eliasdaler I'm not talking about the CMakeLists.txt but rather the nlohmann_jsonConfig.cmake files (i.e. files that get install by running cmake --install .).

@OvermindDL1 Wouldn't it be solved by setting explicitly CMAKE_PREFIX_PATH?

[eliasdaler]

@theodelrieu oh, I see. I don't think this is what I need. I need exported targets and rules to build the library. Imagine me having layout like this:

src/
   my_lib/
   json/

json contains CMakeLists.txt. When I compile everything, I call:

add_subdirectory(json)

src/json contains CMakeLists.txt. It defines build rules, targets, etc., so I can later say target_link_libraries(my_lib PRIVATE nlohmann::json) at configuration stage, before the library is even built or installed.

[theodelrieu]

I need exported targets and rules to build the library.

I don't understand why you want to build/install the library, I must miss something obvious :)

Imagine the following:

src/
    my_lib/
    nlohmann_json/
        include/
        lib/
            cmake/
                 nlohmann_json/
                     nlohmann_jsonConfig.cmake
                     nlohmann_jsonTargets.cmake

With the above layout, you just have to change your CMakeLists.txt to:

set(CMAKE_PREFIX_PATH "${PROJECT_SOURCE_DIR}/src/nlohmann_json;${CMAKE_PREFIX_PATH}")

find_package(nlohmann_json REQUIRED)
target_link_libraries(my_lib PRIVATE nlohmann_json::nlohmann_json)

I use Conan to manage dependencies, but the few times I had to deal with pure CMake was like that, your use-case might be different though, sorry if I misunderstood you

[OvermindDL1]

I still assert most should try https://hunter.sh, it handles nlohmann_json perfectly and is pure cmake, so no weird external dependency manager things to use or install like conan or vcpkg or so (and it actually caches builds properly).

[eliasdaler]

@theodelrieu I see. This should work, yes. :)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.