Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build release tarball when nlohmann-json not installed #1074

Merged
merged 9 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions CMakeLists.txt
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ if((NOT WITH_API_ONLY)
OR WITH_ETW)
# nlohmann_json package is required for most SDK build configurations
find_package(nlohmann_json QUIET)
set(nlohmann_json_clone FALSE)
if(NOT nlohmann_json_FOUND)
if(EXISTS ${PROJECT_SOURCE_DIR}/.git)
message("Trying to use local nlohmann::json from submodule")
Expand All @@ -294,15 +295,14 @@ if((NOT WITH_API_ONLY)
include_directories(
${PROJECT_SOURCE_DIR}/third_party/nlohmann-json/single_include)
else()
message(
FATAL_ERROR
"\nnlohmann_json package was not found. Please either provide it manually or clone with submodules. "
"To initialize, fetch and checkout any nested submodules, you can use the following command:\n"
"git submodule update --init --recursive")
set(nlohmann_json_clone TRUE)
include(cmake/nlohmann-json.cmake)
message("\nnlohmann_json package was not found. Cloning from github")
endif()
else()
# Let's fail with default find_package error
find_package(nlohmann_json REQUIRED)
set(nlohmann_json_clone TRUE)
include(cmake/nlohmann-json.cmake)
message("\nnlohmann_json package was not found. Cloning from github")
lalitb marked this conversation as resolved.
Show resolved Hide resolved
endif()
else()
message("Using external nlohmann::json")
Expand Down
34 changes: 34 additions & 0 deletions cmake/nlohmann-json.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
if("${nlohmann-json}" STREQUAL "")
set(nlohmann-json "develop")
endif()
include(ExternalProject)
ExternalProject_Add(nlohmann_json_download
PREFIX third_party
GIT_REPOSITORY https://github.com/nlohmann/json.git
GIT_TAG
"${nlohmann-json}"
UPDATE_COMMAND ""
CMAKE_ARGS
-DJSON_BuildTests=OFF
-DJSON_Install=ON
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
TEST_AFTER_INSTALL
0
DOWNLOAD_NO_PROGRESS
1
LOG_CONFIGURE
1
LOG_BUILD
1
LOG_INSTALL
1
)

ExternalProject_Get_Property(nlohmann_json_download INSTALL_DIR)
SET(NLOHMANN_JSON_INCLUDE_DIR ${INSTALL_DIR}/third_party/src/nlohmann_json_download/single_include)
add_library(nlohmann_json_ INTERFACE)
target_include_directories(nlohmann_json_ INTERFACE
"$<BUILD_INTERFACE:${NLOHMANN_JSON_INCLUDE_DIR}>"
"$<INSTALL_INTERFACE:include>")
add_dependencies(nlohmann_json_ nlohmann_json_download)
add_library(nlohmann_json::nlohmann_json ALIAS nlohmann_json_)
3 changes: 3 additions & 0 deletions exporters/etw/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ set_target_properties(opentelemetry_exporter_etw PROPERTIES EXPORT_NAME

target_link_libraries(opentelemetry_exporter_etw
INTERFACE nlohmann_json::nlohmann_json)
if(nlohmann_json_clone)
add_dependencies(opentelemetry_exporter_etw nlohmann_json::nlohmann_json)
endif()

install(
TARGETS opentelemetry_exporter_etw
Expand Down
5 changes: 5 additions & 0 deletions exporters/otlp/CMakeLists.txt
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ if(WITH_OTLP_HTTP)
opentelemetry_exporter_otlp_http_client
PUBLIC opentelemetry_sdk opentelemetry_proto http_client_curl
nlohmann_json::nlohmann_json)
if(nlohmann_json_clone)
add_dependencies(opentelemetry_exporter_otlp_http_client
nlohmann_json::nlohmann_json)
list(APPEND OPENTELEMETRY_OTLP_TARGETS nlohmann_json_)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this step required?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's needed for install. nlohmann_json::nlohmann_json is an alias for nlohmann_json_ here.

Copy link
Member

Choose a reason for hiding this comment

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

ok, thanks. Should that be an install target for Zipkin and ETW too, or else just have it in cmake/nlohmann-json.cmake ?

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea. done

endif()
target_include_directories(
opentelemetry_exporter_otlp_http_client
PUBLIC "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>"
Expand Down
5 changes: 3 additions & 2 deletions exporters/zipkin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ add_definitions(-DWITH_CURL)
add_library(opentelemetry_exporter_zipkin_trace src/zipkin_exporter.cc
src/recordable.cc)

target_link_libraries(opentelemetry_exporter_zipkin_trace
PUBLIC opentelemetry_trace http_client_curl)
target_link_libraries(
opentelemetry_exporter_zipkin_trace
PUBLIC opentelemetry_trace http_client_curl nlohmann_json::nlohmann_json)

install(
TARGETS opentelemetry_exporter_zipkin_trace
Expand Down
4 changes: 4 additions & 0 deletions ext/test/w3c_tracecontext_test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ else()
PRIVATE ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace http_client_curl
opentelemetry_exporter_ostream_span ${CURL_LIBRARIES}
nlohmann_json::nlohmann_json)
if(lohmann_json_clone)
lalitb marked this conversation as resolved.
Show resolved Hide resolved
add_dependencies(w3c_tracecontext_test nlohmann_json::nlohmann_json)
endif()

endif()