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.

Sign up for GitHub

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

Jump to bottom

Bump so-name and improve CMake #510

Closed
wants to merge 3 commits into from
Closed

Bump so-name and improve CMake #510

wants to merge 3 commits into from

Conversation

besser82
Copy link
Contributor

As stated in the individual commit messages.

@besser82
Makefile: Bump the major version of the .so library generated up to 5.0.
4f4f555 
There has a new interface been added during development.  Also some
changes may break ABI compatibility with the prior library so-name.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
@besser82
CMakeLists: Export the library so-name and the version properly.
f4d847e 
This ensures we have the same behaviour as building with Autotools.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
@besser82
CMakeLists: Set CMP0075 to new, if CMake >= 3.11 is used during build.
bd459dd 
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
@ploxiln
Copy link
Contributor

ploxiln commented Nov 29, 2019

I think that a feature addition typically merits an so minor bump. You said there were also backwards-incompatible ABI changes - it is probably worth mentioning at least one one significant example.

It is probably also worth adding a comment to the commit and to the cmake config about what CMP0075 is.

@hawicz
Copy link
Member

hawicz commented Dec 2, 2019

Also, as of https://github.com/json-c/json-c/blob/11a638048da0344a3bd5f38722ddecdf5ec22041/CMakeLists.txt I've included the VERSION and SOVERSION properties.

@hawicz
Copy link
Member

hawicz commented Dec 2, 2019 •
edited
Loading

If I'm understanding CMP0075 correctly, it would make the target_link_libraries lines in tests/CMakeLists.txt irrelevant, no?
Scratch that, totally misread what CMP0075 is about. It causes check_include_file(...) calls to use whatever we might have added to CMAKE_REQUIRED_LIBRARIES. In our case that's just libm.

hawicz added a commit that referenced this pull request Dec 6, 2019
@hawicz
Set cmake policy CMP0075, to have check_include_file use the value of…
6c55f65 
… CMAKE_REQUIRED_LIBRARIES (currently, adds -lm). See issue #510.
@hawicz
Copy link
Member

hawicz commented Dec 6, 2019

Not merging, since two of the changes have been made independently, and it's not clear that the library version needs to be bumped.

@hawicz hawicz closed this Dec 6, 2019
@besser82 besser82 deleted the besser82/so-name branch April 10, 2020 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@besser82 @ploxiln @hawicz