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

json-schema-validator: make sure to mark transitive requirement headers #14353

Merged

Conversation

prince-chrismc
Copy link
Contributor

Specify library name and version: lib/1.0

This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!


@conan-center-bot

This comment has been minimized.

@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit 5088e8c
json-schema-validator/2.0.0
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libjson-schema-validator.so' links to system library 'm' but it is not in cpp_info.system_libs.
json-schema-validator/2.1.0
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libnlohmann_json_schema_validator.so' links to system library 'm' but it is not in cpp_info.system_libs.

@garethsb
Copy link
Contributor

After the previous update to nlohmann_json/3.11.2, I'm seeing linking issues with json-schema-validator related to the introduction of the ABI inline namespace in 3.11.0 - nlohmann/json#3590

Does this help with that? Or do we need a requires patch_mode as well?

@prince-chrismc
Copy link
Contributor Author

https://www.youtube.com/watch?v=kKGglzm5ous

This will help in 2.0 with the "full embedded mode" to my knowledge -- it should require a recompile when the version is different :) It wont be specific tied to the version like in 1.x but rather the actually package_id

@garethsb
Copy link
Contributor

garethsb commented Nov 23, 2022

OK... so doesn't solve the problem I have which is that:

nmos-cpp/cci.20220620 requires json-schema-validator/2.1.0, which used to require nlohmann_json/3.10.5 but since #13959 depends on 3.11.2.

Since nmos-cpp/cci.20220620 was built before that dependency bump, it depends on the older nlohmann_json, so linking fails.

nlohmann_json explicitly makes no claim of ABI compatibility between versions, even patch versions (and the inline namespace introduced with 3.11.0 includes the patch number). Does that mean the json-schema-validator recipe ought to have:

    def package_id(self):
        self.info.requires["nlohmann_json"].patch_mode()

But I'm not even sure that's enough to force nmos-cpp to rebuild... maybe it needs its own direct requires on nlohmann_json even though it doesn't care which version... hmmm...

@prince-chrismc
Copy link
Contributor Author

It does help future you since we should all stop needing to change these in downstream recipes 😸

(insert meme of me rewinding my brain to Conan 1.x)

Yes, I think it would make sense to add that -- the binary should be different for any version of nlohmann_json...
I would maybe even say full_version_mode would be good.

Given we know this is a problematic case it makes sense to exceptionally add it here

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline

All green in build 2 (8e9e8e47b3872325b6e6e880f2fddaf2dcc0c02f):

  • json-schema-validator/2.1.0@:
    All packages built successfully! (All logs)

  • json-schema-validator/2.0.0@:
    All packages built successfully! (All logs)

@ghost
Copy link

ghost commented Nov 27, 2022

I detected other pull requests that are modifying json-schema-validator/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prsso don't hesitate to report issues/improvements there.

@conan-center-bot conan-center-bot merged commit 3d7bf18 into conan-io:master Nov 28, 2022
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.

5 participants