pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
470 stars 135 forks source link

CMake install regression #181

Closed trevor-creator closed 1 year ago

trevor-creator commented 2 years ago

We are hoping to use nlohmann::json and json-schema-validator as submodules in our project. We are building for single-binary deployment, so our project is not meant to be installed using CMake. However, when I tried using master yesterday, I found that submodule use without installing no longer works. It appears that PR #165 has undone PR #81, re-causing error #79.

Example, using the demonstration from #79 and starting with its original json and validator versions:

user@ubuntu:~/test/json_schema_validator_issue$ cmake -S . -B build
-- Using the single-header code from /home/user/test/json_schema_validator_issue/nlohmann_json/single_include/
-- Found nlohmann_json::nlohmann_json-target - linking with it
-- Configuring done
CMake Error: install(EXPORT "nlohmann_json_schema_validatorTargets" ...) includes target "nlohmann_json_schema_validator" which requires target "nlohmann_json" that is not in any export set.
-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.
user@ubuntu:~/test/json_schema_validator_issue$ cd nlohmann_json/
user@ubuntu:~/test/json_schema_validator_issue/nlohmann_json$ git checkout v3.10.4
Previous HEAD position was 7acf6218b :memo: fix typo #1903
HEAD is now at fec56a1a1 Merge branch 'release/3.10.4'
user@ubuntu:~/test/json_schema_validator_issue/nlohmann_json$ cd ../json_schema_validator/
user@ubuntu:~/test/json_schema_validator_issue/json_schema_validator$ git checkout  cc05de93274ee432ec75be5fbbd01e8d3b6d0ccd
Previous HEAD position was 8a7d1d3 Adapt CMake project name to be coherent with nlohmann::json's naming
HEAD is now at cc05de9 Create basic github-actions workflow (#175)
user@ubuntu:~/test/json_schema_validator_issue/json_schema_validator$ cd ..
user@ubuntu:~/test/json_schema_validator_issue$ cmake -S . -B build
-- The CXX compiler identification is GNU 9.3.0
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Using the single-header code from /home/user/test/json_schema_validator_issue/nlohmann_json/single_include/
-- Found nlohmann_json::nlohmann_json-target - linking with it
-- Configuring done
-- Generating done
-- Build files have been written to: /home/user/test/json_schema_validator_issue/build
user@ubuntu:~/test/json_schema_validator_issue$ cd json_schema_validator/
user@ubuntu:~/test/json_schema_validator_issue/json_schema_validator$ git checkout master
Previous HEAD position was cc05de9 Create basic github-actions workflow (#175)
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
user@ubuntu:~/test/json_schema_validator_issue/json_schema_validator$ cd ..
user@ubuntu:~/test/json_schema_validator_issue$ cmake -S . -B build
-- Using the single-header code from /home/user/test/json_schema_validator_issue/nlohmann_json/single_include/
-- Configuring done
CMake Error: install(EXPORT "nlohmann_json_schema_validatorTargets" ...) includes target "nlohmann_json_schema_validator" which requires target "nlohmann_json" that is not in any export set.
-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.

The change responsible appears to be here:

https://github.com/pboettch/json-schema-validator/pull/165/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL144-R103

The new CMakeLists always attempts to install the library, even if JSON is not installed (because JSON only installs/exports if it is the top-level project or the install is explicitly enabled).

While I'm not that familiar with CMake installs, my first thought would be to emulate JSON's install conditions and only install if json-schema-validator is the top-level project with JSON installed, or if a variable is explicitly set to enable the install.

trevor-creator commented 2 years ago

Looking into nlohmann::json's history, it stopped installing by default when a submodule as of v3.10.0.

A workaround is to not set set(JSON_Install OFF CACHE INTERNAL "") before including nlohmann::json if using v3.9.1 or earlier, or to set set(JSON_Install ON CACHE INTERNAL "") if using a more recent version. If mandatory install will be kept, this should be documented.

vrince commented 2 years ago

What about we set install target only when JSON_VALIDATOR_IS_TOP_LEVEL ? We already skip examples and tests if used as a submodule ...

trevor-creator commented 2 years ago

What about we set install target only when JSON_VALIDATOR_IS_TOP_LEVEL ? We already skip examples and tests if used as a submodule ...

Commented on the PR, but realized I maybe should reply here too: the PR that caused this regression came from requests to install when used as a submodule (#162, #164). It needs to be an option.

pboettch commented 1 year ago

I think some (or all of) your issues are fixed now? Please re-open if still needed.