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

Add versioning to shared library #223

Closed
hhoffstaette opened this issue Nov 13, 2018 · 7 comments
Closed

Add versioning to shared library #223

hhoffstaette opened this issue Nov 13, 2018 · 7 comments
Assignees

Comments

@hhoffstaette
Copy link

hhoffstaette commented Nov 13, 2018

This is not urgent until we hit 1.0, but eventually it would be good if the shared library had at least basic x.y.z versioning since right now they look a bit naked, and this is not the most popular thing to do with packagers:

$ls -1 /usr/lib/libprometheus-cpp-*
/usr/lib/libprometheus-cpp-core.so
/usr/lib/libprometheus-cpp-pull.so
/usr/lib/libprometheus-cpp-push.so

I can look into this for cmake and make a PR in the next few days.

@gjasny
Copy link
Collaborator

gjasny commented Nov 13, 2018

But versioning also results in API / ABI promises. Which will lead us to symbol visibility. Which will somehow collide with the templated and non-virtual coding style of Prometheus-C++.

But please get the discussion started with some code.

@jupp0r
Copy link
Owner

jupp0r commented Nov 19, 2018

I think this will ultimately be driven by linux distributions packaging requirements.

@hhoffstaette
Copy link
Author

Sorry for the delay - surprise work commitment required travel. I finally got around to try this (still learning cmake!) and it was quite easy, see 2c17862.
Result looks as expected:

$ll {core,push,pull}/*.so*                        
lrwxrwxrwx 1 holger users   27 Dec  8 15:34 core/libprometheus-cpp-core.so -> libprometheus-cpp-core.so.0*
lrwxrwxrwx 1 holger users   31 Dec  8 15:34 core/libprometheus-cpp-core.so.0 -> libprometheus-cpp-core.so.0.0.0*
-rwxr-xr-x 1 holger users 1.8M Dec  8 15:34 core/libprometheus-cpp-core.so.0.0.0*
lrwxrwxrwx 1 holger users   27 Dec  8 15:34 pull/libprometheus-cpp-pull.so -> libprometheus-cpp-pull.so.0*
lrwxrwxrwx 1 holger users   31 Dec  8 15:34 pull/libprometheus-cpp-pull.so.0 -> libprometheus-cpp-pull.so.0.0.0*
-rwxr-xr-x 1 holger users 624K Dec  8 15:34 pull/libprometheus-cpp-pull.so.0.0.0*
lrwxrwxrwx 1 holger users   27 Dec  8 15:34 push/libprometheus-cpp-push.so -> libprometheus-cpp-push.so.0*
lrwxrwxrwx 1 holger users   31 Dec  8 15:34 push/libprometheus-cpp-push.so.0 -> libprometheus-cpp-push.so.0.0.0*
-rwxr-xr-x 1 holger users 506K Dec  8 15:34 push/libprometheus-cpp-push.so.0.0.0*

gjasny added a commit that referenced this issue Dec 15, 2018
gjasny added a commit that referenced this issue Dec 15, 2018
@gjasny
Copy link
Collaborator

gjasny commented Dec 15, 2018

Hi,

in the symbol-visibility branch I added symbol visibility for shared libraries as well as library versioning. Some observations:

  1. Theoretically it should be possible to add DLL support. But due to heavy use of STL containers in public interfaces the compiler is not too happy with it. Maybe C++ modules will help here.
  2. Right now API visibility is done on a per-class level. As a consequence private members are visible (but nor useable). Exporting only selected members adds a lot of noise.
  3. Library versioning needs semantic versioning. By using ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR} as the soversion I shifted semver one position to the right. So version 0.7.0 will be compatible with 0.7.1 but not with 0.8.0. With C++ code ABI compatibility is often broken by e.g. adding a new virtual function whereby API compatibility remains intact. I'd like to see the version number more as a API instead of ABI compatibility.

gjasny added a commit that referenced this issue Dec 15, 2018
@jupp0r
Copy link
Owner

jupp0r commented Dec 17, 2018

What do you think about getting a package into debian? I imagine at the end of that process, all of these issues will be resolved one way or another.

Regarding 3: technically in semver, everything goes below 1.0, so if we want to provide these guarantees, we should think about 1.0 at some point. I think it's matured enough to have a somewhat stable interface now.

@hhoffstaette
Copy link
Author

I gave the symbol-visibility branch a try and it works well - thanks @gjasny !
My original reason for raising this issue was that I build on Gentoo where non-versioned shared libs are very unpopular (to put it mildly), since the package manager is aware of API/ABI changes and can act accordingly (warnings, automated rebuilds etc.). I plan to eventually contribute this to Gentoo upstream as well.
Also agree with @jupp0r that we can and should break things (for good reason) as long as the version is <1.0. This was more a long-term/packaging concern.

@gjasny
Copy link
Collaborator

gjasny commented Dec 17, 2018 via email

gjasny added a commit that referenced this issue Aug 3, 2019
gjasny added a commit that referenced this issue Aug 31, 2019
gjasny added a commit that referenced this issue Aug 31, 2019
@gjasny gjasny self-assigned this Aug 31, 2019
@gjasny gjasny closed this as completed in 538ed3a Oct 17, 2019
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

No branches or pull requests

3 participants