1 build: do not `FetchContent nlohmann_json` if already found by `find_package` by sjanel · Pull Request #227 · DataDog/dd-trace-cpp · GitHub
Skip to content

build: do not FetchContent nlohmann_json if already found by find_package#227

Closed
sjanel wants to merge 1 commit intoDataDog:mainfrom
sjanel:do-not-fetch-content-nlohmann-json-if-present
Closed

build: do not FetchContent nlohmann_json if already found by find_package#227
sjanel wants to merge 1 commit intoDataDog:mainfrom
sjanel:do-not-fetch-content-nlohmann-json-if-present

Conversation

@sjanel
Copy link
Copy Markdown
Contributor

@sjanel sjanel commented Aug 5, 2025

Description

This PR makes it possible to use dd-trace-cpp as a dependency when nlohmann_json has already been fetched content by a project.

It also renames json into nlohmann_json as it is more standard in other projects.

Motivation

Additional Notes

Bump version of nlohmann_json to 3.12.0.

Jira ticket: [PROJ-IDENT]

@sjanel sjanel requested a review from a team as a code owner August 5, 2025 09:01
@sjanel sjanel requested review from dmehala and removed request for a team August 5, 2025 09:01
Copy link
Copy Markdown
Contributor

@dmehala dmehala left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @sjanel.
While I'm not opposed to avoiding the download of nlohmann-json if it's already installed on the system, we should ensure that the installed version matches the required one to prevent potential compilation errors.

@dmehala dmehala changed the title Do not FetchContent nlohmann_json if already found by find_package build: Do not FetchContent nlohmann_json if already found by find_package Aug 5, 2025
@dmehala dmehala changed the title build: Do not FetchContent nlohmann_json if already found by find_package build: do not FetchContent nlohmann_json if already found by find_package Aug 5, 2025
@dmehala dmehala changed the title build: do not FetchContent nlohmann_json if already found by find_package build: do not FetchContent nlohmann_json if already found by find_package Aug 5, 2025
@sjanel
Copy link
Copy Markdown
Contributor Author

sjanel commented Aug 5, 2025

Thanks for your contribution @sjanel. While I'm not opposed to avoiding the download of nlohmann-json if it's already installed on the system, we should ensure that the installed version matches the required one to prevent potential compilation errors.

To ensure a minimum version is found, we can do something like:

find_package(nlohmann_json 3.11.2 QUIET)

Comment thread cmake/deps/json.cmake Outdated
Comment thread cmake/deps/json.cmake Outdated
@sjanel sjanel force-pushed the do-not-fetch-content-nlohmann-json-if-present branch from 710ce93 to 0964486 Compare August 14, 2025 14:37
@sjanel sjanel force-pushed the do-not-fetch-content-nlohmann-json-if-present branch from 0964486 to 864c854 Compare August 14, 2025 14:38
Copy link
Copy Markdown
Contributor

@dmehala dmehala left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution 🙇

@dmehala
Copy link
Copy Markdown
Contributor

dmehala commented Aug 14, 2025

Slight details, @sjanel can you sign your commit? please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants