Thalhammer / jwt-cpp

A header only library for creating and validating json web tokens in c++
https://thalhammer.github.io/jwt-cpp/
MIT License
875 stars 239 forks source link

Find nlohmann_json test dependency only when required #342

Closed justend29 closed 5 months ago

justend29 commented 5 months ago

nlohmann::json is used in the examples and tests of jwt-cpp. However, it was always found, making it a transitive dependency for all users of jwt-cpp - even those not building examples or tests. These changes simply check if nlohman_json is required as a host dependency before searching for it.

justend29 commented 5 months ago

@prince-chrismc @Thalhammer

Yes - I believe falling back to downloading and building the package when it's not found is not a convenience, it's a mistake. It's far too easy to mistype paths and then get the surprise of downloading a package you already have. Explicit selection of which to use is worthwhile.

A dependent option, maybe JWT_EXTERNAL_TEST_NLOHMANN_JSON, would offer this control. Additionally, CMake has built-in integration between find_package and fetch_content offered in CMake 3.24+ allowing standard CMake variables control how the dependencies are acquired, and dependency providers can be used to override it. Nevertheless, this project only mandates CMake 3.14+, and CMake 3.24's integrations would only complement JWT's variable for users who use compatible versions.

Adding now. Let me know if you'd like further changes.

Also, really great library. Thank you for the contributions.

prince-chrismc commented 5 months ago

LGTM, thanks for the contribution.

I think there are missing tests for and/or feature for pickup the different JSON libraries from out find config but thats also not how it's supported? it just adds all of them and lets the user figure the rest out 🤷