Skip to content

Commit

Permalink
Use a version check to provide backwards comatible imported target na…
Browse files Browse the repository at this point in the history 
…mes.
  • Loading branch information
Chuck Atkins committed Sep 18, 2018
1 parent 99939d6 commit 3b1a5ca
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions cmake/config.cmake.in
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
@PACKAGE_INIT@
if(NOT TARGET @PROJECT_NAME@::@NLOHMANN_JSON_TARGET_NAME@)
include("${CMAKE_CURRENT_LIST_DIR}/@NLOHMANN_JSON_TARGETS_EXPORT_NAME@.cmake")
if((NOT TARGET @NLOHMANN_JSON_TARGET_NAME@) AND
(PACKAGE_FIND_VERSION VERSION_LESS 3.2.0))
add_library(@NLOHMANN_JSON_TARGET_NAME@ ALIAS @PROJECT_NAME@::@NLOHMANN_JSON_TARGET_NAME@)
endif()
endif()
check_required_components("@PROJECT_NAME@")

3 comments on commit 3b1a5ca

@ericu65
Copy link

@ericu65 ericu65 commented on 3b1a5ca Sep 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to break "Export package to allow builds without installing":

CMake Error at C:/Users/ericu/source/repos/json/build/nlohmann_jsonConfig.cmake:30 (add_library):
  add_library cannot create ALIAS target "nlohmann_json" because target
  "nlohmann_json::nlohmann_json" is imported but not globally visible.
Call Stack (most recent call first):
  CMakeLists.txt:9 (find_package)

Steps (windows):

  1. Create the required registry entries by Configuring this project;
  2. Add find_package(nlohmann_json REQUIRED 3.2.0) to a downstream CMakeLists
  3. Configure downstream.
  4. Error as above.

Removing this commit allowed builds normally. Since I'm not totally familiar with CMake yet it may be something else.

PS. Colleague on Harman DAI project too :) Great work on the library.

@nlohmann
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericu65 This should be fixed now by merging #1260.

@ericu65
Copy link

@ericu65 ericu65 commented on 3b1a5ca Oct 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fixed. Thanks.

Please sign in to comment.