CMake: Add soversion to library files to generate proper symlinks #394

cebe · 2017-01-05T20:55:09Z

As mentioned in #359 (comment)
the current CMake build does not generate version symlinks for library .so files.
This is because the version is not specified for library targets.

See

I have used VERSION instead of SOVERSION as this seems to be
compatible with windows according to the docs linked above. updated: changed this, see comments below.

I have compared installed files with autotools build vs. cmake:

autotools:

$ find . | sort
.
./bin
./include
./include/tox
./include/tox/toxav.h
./include/tox/toxdns.h
./include/tox/toxencryptsave.h
./include/tox/tox.h
./lib
./lib/libtoxav.a
./lib/libtoxav.la
./lib/libtoxav.so
./lib/libtoxav.so.0
./lib/libtoxav.so.0.0.0
./lib/libtoxcore.a
./lib/libtoxcore.la
./lib/libtoxcore.so
./lib/libtoxcore.so.0
./lib/libtoxcore.so.0.0.0
./lib/libtoxdns.a
./lib/libtoxdns.la
./lib/libtoxdns.so
./lib/libtoxdns.so.0
./lib/libtoxdns.so.0.0.0
./lib/libtoxencryptsave.a
./lib/libtoxencryptsave.la
./lib/libtoxencryptsave.so
./lib/libtoxencryptsave.so.0
./lib/libtoxencryptsave.so.0.0.0
./lib/pkgconfig
./lib/pkgconfig/libtoxav.pc
./lib/pkgconfig/libtoxcore.pc

cmake:

$ find . | sort
.
./include
./include/tox
./include/tox/toxav.h
./include/tox/toxdns.h
./include/tox/toxencryptsave.h
./include/tox/tox.h
./lib
./lib/libtoxav.a
./lib/libtoxav.so
./lib/libtoxav.so.0.1.2
./lib/libtoxcore.a
./lib/libtoxcore.so
./lib/libtoxcore.so.0.1.2
./lib/libtoxcrypto.a
./lib/libtoxcrypto.so
./lib/libtoxcrypto.so.0.1.2
./lib/libtoxdht.a
./lib/libtoxdht.so
./lib/libtoxdht.so.0.1.2
./lib/libtoxdns.a
./lib/libtoxdns.so
./lib/libtoxdns.so.0.1.2
./lib/libtoxencryptsave.a
./lib/libtoxencryptsave.so
./lib/libtoxencryptsave.so.0.1.2
./lib/libtoxfriends.a
./lib/libtoxfriends.so
./lib/libtoxfriends.so.0.1.2
./lib/libtoxgroup.a
./lib/libtoxgroup.so
./lib/libtoxgroup.so.0.1.2
./lib/libtoxmessenger.a
./lib/libtoxmessenger.so
./lib/libtoxmessenger.so.0.1.2
./lib/libtoxnetcrypto.a
./lib/libtoxnetcrypto.so
./lib/libtoxnetcrypto.so.0.1.2
./lib/libtoxnetwork.a
./lib/libtoxnetwork.so
./lib/libtoxnetwork.so.0.1.2
./lib/pkgconfig
./lib/pkgconfig/libtoxav.pc
./lib/pkgconfig/libtoxcore.pc
./lib/pkgconfig/toxav.pc
./lib/pkgconfig/toxcore.pc
./lib/pkgconfig/toxdns.pc
./lib/pkgconfig/toxencryptsave.pc

This change is

iphydf · 2017-01-05T20:58:37Z

0.1.2 is compatible with 0.1.3. Is this reflected by the sonames in this PR?

cebe · 2017-01-05T21:15:08Z

I just read a bit more about sonames and it seems what I have done is not correct. The soname version should only change when incompatible changes are made, so as far as I see, we should only provide the ${PROJECT_VERSION_MAJOR} there as that is the one that indicates incompatibility, right?

jin-eld · 2017-01-05T21:23:10Z

Ideally the cmake variant should be in sync with the .so version in autoconf, have a look at:
https://github.com/TokTok/c-toxcore/blob/master/configure.ac#L15

Maybe we could even come up with a configuration file where this can be set for both build systems.

cebe · 2017-01-05T21:29:35Z

Using the MAJOR number will be in sync with the autotools:

$ objdump -p lib/libtoxcore.so |grep SONAME
  SONAME               libtoxcore.so.0

output above is the same for autotools and cmake now.

after last commit the following files are generated:

$ find .|sort
.
./include
./include/tox
./include/tox/toxav.h
./include/tox/toxdns.h
./include/tox/toxencryptsave.h
./include/tox/tox.h
./lib
./lib/libtoxav.a
./lib/libtoxav.so
./lib/libtoxav.so.0
./lib/libtoxav.so.0.1.2
./lib/libtoxcore.a
./lib/libtoxcore.so
./lib/libtoxcore.so.0
./lib/libtoxcore.so.0.1.2
./lib/libtoxcrypto.a
./lib/libtoxcrypto.so
./lib/libtoxcrypto.so.0
./lib/libtoxcrypto.so.0.1.2
./lib/libtoxdht.a
./lib/libtoxdht.so
./lib/libtoxdht.so.0
./lib/libtoxdht.so.0.1.2
./lib/libtoxdns.a
./lib/libtoxdns.so
./lib/libtoxdns.so.0
./lib/libtoxdns.so.0.1.2
./lib/libtoxencryptsave.a
./lib/libtoxencryptsave.so
./lib/libtoxencryptsave.so.0
./lib/libtoxencryptsave.so.0.1.2
./lib/libtoxfriends.a
./lib/libtoxfriends.so
./lib/libtoxfriends.so.0
./lib/libtoxfriends.so.0.1.2
./lib/libtoxgroup.a
./lib/libtoxgroup.so
./lib/libtoxgroup.so.0
./lib/libtoxgroup.so.0.1.2
./lib/libtoxmessenger.a
./lib/libtoxmessenger.so
./lib/libtoxmessenger.so.0
./lib/libtoxmessenger.so.0.1.2
./lib/libtoxnetcrypto.a
./lib/libtoxnetcrypto.so
./lib/libtoxnetcrypto.so.0
./lib/libtoxnetcrypto.so.0.1.2
./lib/libtoxnetwork.a
./lib/libtoxnetwork.so
./lib/libtoxnetwork.so.0
./lib/libtoxnetwork.so.0.1.2
./lib/pkgconfig
./lib/pkgconfig/libtoxav.pc
./lib/pkgconfig/libtoxcore.pc
./lib/pkgconfig/toxav.pc
./lib/pkgconfig/toxcore.pc
./lib/pkgconfig/toxdns.pc
./lib/pkgconfig/toxencryptsave.pc

To have everything in sync we only need to adjust the version in configure.ac as far as I see.

iphydf · 2017-01-05T21:36:28Z

While we're on 0.x, the x behaves like the major version. 0.2 will be incompatible with 0.1.

cebe · 2017-01-05T21:44:20Z

okay, changed that, also removed the version from static library as these do not support a versioning concept.

iphydf · 2017-01-08T15:20:30Z

CMakeLists.txt

@@ -611,6 +611,10 @@ endif()
 #
 # :: Strict ABI
 #
+# Enabling the STRICT_ABI flag will generate and use an LD version script.
+# It ensures that the dynamic libraries (libtoxcore.so, libtoxav.so) only
+# export the symbols that are defined in their public API (tox.h and toxav.h, respectively).


Break the line at 80 columns.

iphydf · 2017-01-08T15:20:47Z

cmake/ModulePackage.cmake

+    set_target_properties(${lib}_shared PROPERTIES
+      OUTPUT_NAME ${lib}
+      VERSION ${PROJECT_VERSION}
+      # While on 0.x, the x behaves like the major version. 0.2 will be incompatible with 0.1.


iphydf · 2017-01-08T15:21:39Z

Reviewed 1 of 2 files at r1, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

Comments from Reviewable

iphydf · 2017-01-08T15:22:03Z

Rebase and squash the commits, then I'll merge.

This part has been added in TokTok@67ac913 Took the commit message to provide a comment in the file.

As mentioned in TokTok#359 (comment) the current CMake build does not generate version symlinks for library .so files. This is because the version is not specified for library targets. See - https://cmake.org/cmake/help/v3.0/prop_tgt/SOVERSION.html#prop_tgt:SOVERSION - https://cmake.org/cmake/help/v3.0/prop_tgt/VERSION.html#prop_tgt:VERSION Use PROJECT_VERSION_MAJOR and MINOR for SOVERSION because api may break from 0.1 to 0.2 in the 0.x release cycle.

cebe · 2017-01-08T18:18:42Z

Rebase and squash the commits, then I'll merge.

done.

iphydf · 2017-01-08T18:54:16Z

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

nurupo · 2017-01-08T19:30:58Z

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

cmake/ModulePackage.cmake, line 59 at r5 (raw file):

    set_target_properties(${lib}_shared PROPERTIES
      OUTPUT_NAME ${lib}
      VERSION ${PROJECT_VERSION}

it sounds like CMake expects the VERSION to be MAJOR.MINOR too, just like the SOVERSION is, instead of MAJOR.MINOR.PATCH that we have here.

https://cmake.org/cmake/help/v2.8.6/cmake.html#prop_tgt:VERSION

Comments from Reviewable

nurupo · 2017-01-08T19:40:39Z

Review status: all files reviewed at latest revision, 1 unresolved discussion.

cmake/ModulePackage.cmake, line 59 at r5 (raw file):

Previously, nurupo wrote…

it sounds like CMake expects the VERSION to be MAJOR.MINOR too, just like the SOVERSION is, instead of MAJOR.MINOR.PATCH that we have here.

https://cmake.org/cmake/help/v2.8.6/cmake.html#prop_tgt:VERSION

Nevermind, CeBe seems to know better what he is doing.

Comments from Reviewable

nurupo · 2017-01-08T19:44:26Z

Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

cebe · 2017-01-08T19:46:04Z

Review status: all files reviewed at latest revision, 3 unresolved discussions.

CMakeLists.txt, line 616 at r4 (raw file):

Previously, iphydf wrote…

Break the line at 80 columns.

Done.

cmake/ModulePackage.cmake, line 60 at r4 (raw file):

Previously, iphydf wrote…

Same here.

Done.

cmake/ModulePackage.cmake, line 59 at r5 (raw file):

Previously, nurupo wrote…

Nevermind, CeBe seems to know better what he is doing.

Done.

Comments from Reviewable

robinlinden · 2017-01-08T19:46:57Z

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

cebe mentioned this pull request Jan 5, 2017

Unused shared libraries #359

Closed

iphydf added this to the v0.1.4 milestone Jan 5, 2017

iphydf added the CAT:overhead label Jan 5, 2017

iphydf self-assigned this Jan 6, 2017

iphydf reviewed Jan 8, 2017

View reviewed changes

cebe added 2 commits January 8, 2017 19:16

added comment to magic code in CMakeLists.txt

31eaffe

This part has been added in TokTok@67ac913 Took the commit message to provide a comment in the file.

cebe force-pushed the cmake branch from b03d900 to 6ec23c9 Compare January 8, 2017 18:18

iphydf merged commit 6ec23c9 into TokTok:master Jan 8, 2017

cebe deleted the cmake branch January 8, 2017 19:48

iphydf mentioned this pull request Jan 10, 2017

Set library version on future releases #323

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake: Add soversion to library files to generate proper symlinks #394

CMake: Add soversion to library files to generate proper symlinks #394

cebe commented Jan 5, 2017 •

edited

Loading

iphydf commented Jan 5, 2017

cebe commented Jan 5, 2017 •

edited

Loading

jin-eld commented Jan 5, 2017

cebe commented Jan 5, 2017

iphydf commented Jan 5, 2017

cebe commented Jan 5, 2017 •

edited

Loading

iphydf Jan 8, 2017

iphydf Jan 8, 2017

iphydf commented Jan 8, 2017

iphydf commented Jan 8, 2017

cebe commented Jan 8, 2017

iphydf commented Jan 8, 2017

nurupo commented Jan 8, 2017

nurupo commented Jan 8, 2017

nurupo commented Jan 8, 2017

cebe commented Jan 8, 2017

robinlinden commented Jan 8, 2017

CMake: Add soversion to library files to generate proper symlinks #394

CMake: Add soversion to library files to generate proper symlinks #394

Conversation

cebe commented Jan 5, 2017 • edited Loading

iphydf commented Jan 5, 2017

cebe commented Jan 5, 2017 • edited Loading

jin-eld commented Jan 5, 2017

cebe commented Jan 5, 2017

iphydf commented Jan 5, 2017

cebe commented Jan 5, 2017 • edited Loading

iphydf Jan 8, 2017

Choose a reason for hiding this comment

iphydf Jan 8, 2017

Choose a reason for hiding this comment

iphydf commented Jan 8, 2017

iphydf commented Jan 8, 2017

cebe commented Jan 8, 2017

iphydf commented Jan 8, 2017

nurupo commented Jan 8, 2017

nurupo commented Jan 8, 2017

nurupo commented Jan 8, 2017

cebe commented Jan 8, 2017

robinlinden commented Jan 8, 2017

cebe commented Jan 5, 2017 •

edited

Loading

cebe commented Jan 5, 2017 •

edited

Loading

cebe commented Jan 5, 2017 •

edited

Loading