Skip to content

Commit

Permalink
Required same version of nlohmann_json in dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
JohanMabille committed Mar 20, 2024
1 parent 631b7c2 commit f8fd348
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
7 changes: 6 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,12 @@ message(STATUS "XEUS_BUILD_TESTS: ${XEUS_BUILD_TESTS}")
# Dependencies
# ============

set(nlohmann_json_REQUIRED_VERSION 3.2.0)
# nlohmann_json requires libraries that exchange json objects to be linked
# with the same version of nlohmann_json. Therefore this version should be
# the same in all xeus components; to do so, downstream projects should not
# search # directly for nlohmann_json, but rely on find_dependency called by
# find_package(xeus) instead.
set(nlohmann_json_REQUIRED_VERSION 3.11)
set(xtl_REQUIRED_VERSION 0.7)

if (NOT TARGET nlohmann_json)
Expand Down
4 changes: 3 additions & 1 deletion xeusConfig.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR};${CMAKE_MODULE_PATH}")

include(CMakeFindDependencyMacro)
find_dependency(xtl @xtl_REQUIRED_VERSION@)
find_dependency(nlohmann_json @nlohmann_json_REQUIRED_VERSION@)
# nlohmann_json requires libraries that exchange json objects to be linked
# with the same version of nlohmann_json.
find_dependency(nlohmann_json @nlohmann_json_REQUIRED_VERSION@ EXACT)

This comment has been minimized.

Copy link
@kaelingre

kaelingre Mar 24, 2024

I believe that this line is dangerous. In this way, one always demands the exact version 3.11 of nlohmann_json (the variable nlohmann_json_REQUIRED_VERSION is hardcoded to "3.11"), and not the one that is actually being used (which is AT LEAST 3.11, but not necessarily exactly 3.11). See for example the problem reported here: jupyter-xeus/xeus-zmq#39

edit: I just saw that this has already been fixed in the following commit: 2047292

This comment has been minimized.

Copy link
@JohanMabille

JohanMabille Mar 26, 2024

Author Member

Yes, I realized the mistake right after realeasing xeus 4.0.0, it has been fixed in 4.0.1


# This is required when linking with the static target
if(NOT EMSCRIPTEN)
Expand Down

0 comments on commit f8fd348

Please sign in to comment.