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

[package] nlohmann_json/3.11.3: Requiring nlohmann_json in a library does not transitively issue required generators. #23970

Closed
Adnn opened this issue May 13, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@Adnn
Copy link
Contributor

Adnn commented May 13, 2024

Description

Let's describe a minimal example reproducing the issue.

Library A depends nlohmann_json/3.11.3 and spdlog/1.13.0. (note: the issue is the same with other nlohmann_json versions).
If a downstream B depends on A (for example, the downstream could be A's test_package), we need it to locate both nlohmann_json and spdlog: this gives the ability to issue find_package() statements for those requirements in B CMake scripts.

With spdlog, this works as expected. But with nlohmann_json, find_package() is failing with error:

Could not find a package configuration file provided by "nlohmann_json".

Investigating further, we can see that B's generators folder contain generators for A, for spdlog and its upstreams, but not for nlohmann_json, which explains why find_package() is failing.

Package and Environment Details

  • Package Name/Version: nlohmann_json/3.11.3
  • Operating System+version: macOS 12.6.5
  • Compiler+version: Apple clang 14.0.0
  • Conan version: conan 2.2.3
  • Python version: Python 3.11.4

Conan profile

--

Steps to reproduce

Please see the minimal example in the description.

Logs

--

@Adnn Adnn added the bug Something isn't working label May 13, 2024
@SpaceIm
Copy link
Contributor

SpaceIm commented May 13, 2024

I believe your issue is not related to conan-center-index. I don't think it's a bug, but a question you should ask to conan client team: https://github.com/conan-io/conan/issues.

It's likely related to the fact that you have written a conanfile of A in which nlohmann_json is defined as a private dependency of A (in conan v2, dependencies of non-header-only libs are private by default, you have to define transitive_headers/transitive_libs traits explicitly in conanfile of A if you want to define public relationship, see https://docs.conan.io/2/reference/conanfile/methods/requirements.html), so indeed conan generators don't generate anything for nlohmann_json when you call conan create/install/build on a recipe B depending on A, since nlohmann_json is header-only. But well it's related to conan client, not to nlohmann_json recipe.

@Adnn
Copy link
Contributor Author

Adnn commented May 14, 2024

Hi @SpaceIm, thank you for your response.

After discussions on the #conan slack, I do not know if it is a bug of the recipe, or a breaking change in Conan V2 client.
The thing is that we defined the nlohmann_json requirement as we always did (we do not make any explicit statement that it would be private), and importantly, we declare it exactly the same as any other dependency (which are not header only, but from our perspective this is a detail of the upstream which should not change so radically how downstreams behave).

The literal requirements declaration in our MRE is:

requires = "nlohmann_json/3.11.3", "spdlog/1.14.1"

And there is nothing else touching requirements.

And from that trivial requirement declaration, spdlog issues generators transitively (as it did with Conan V1, and as we came to expect), but nlohmann_json does not.

As far as we can see this is highly inconsistent, either all recipes should be implicitly "transitively public" or "transitively private" (with the usual ability for users to explicitly override). So either nlohmann_json either spdlog do not behave as it should.

@SpaceIm
Copy link
Contributor

SpaceIm commented May 14, 2024

(we do not make any explicit statement that it would be private)

As I said, in conan v2 dependencies are private by default

and importantly, we declare it exactly the same as any other dependency (which are not header only, but from our perspective this is a detail of the upstream which should not change so radically how downstreams behave).
(...)
And from that trivial requirement declaration, spdlog issues generators transitively (as it did with Conan V1, and as we came to expect), but nlohmann_json does not.

The fact that conan generates CMake config file for private transitive dependencies when dependency is static and its dependencies are static/shared (and not when they are header-only) is related to conan client. There is no issue in recipes themselves.
For example if A is shared, and spdlog static, conan doesn't generate CMake config file for spdlog when you build B (which depends on A). If A is static, conan has to generate config file of spdlog because lib of spdlog must be propagated for the linker.

Now for your recipe B, if CMakeLists of B calls find_package() on nlohmann_json and spdlog, these 2 libraries should be explicitly declared as direct requirements in recipe B (otherwise you have no reason to call find_package() on these 2 libraries, they are internal details of A), you shouldn't rely on transitive dependencies coming from A.

@Adnn
Copy link
Contributor Author

Adnn commented May 14, 2024

I missed your edit @SpaceIm , sorry.

Even though it feels "surprising" that implicit visibility now changes depending on the context, it is a Conan V2 decision.


Just a clarification, which does not change the fact that this is not a bug.

Now for your recipe B, if CMakeLists of B calls find_package() on nlohmann_json and spdlog, these 2 libraries should be explicitly declared as direct requirements in recipe B (otherwise you have no reason to call find_package() on these 2 libraries, they are internal details of A), you shouldn't rely on transitive dependencies coming from A.

Not necessarily, see this statement from CMake documentation:

All required dependencies of a package must also be found in the package configuration file.

This means that a well behaved CMake package config file will call find_package() on all (public) libraries it depends upon, and this call will occur in the context of the leaf project (even though the package whose config actually issue the call can be arbitrarily high in the dependency tree).
Because of that, in any non-trivial project, the CMake configure step will need to find packages that the leaf project has no business knowing about.

@Adnn
Copy link
Contributor Author

Adnn commented May 14, 2024

Not-a-bug, the requirements are not transitive by default in Conan v2.

@Adnn Adnn closed this as completed May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants