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 macOS linker versioning information #3358

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

tschoonj
Copy link
Contributor

@tschoonj tschoonj commented Apr 3, 2018

This patch exploits the information residing in ltversion to set the
-compatibility_version and -current_version flags that are passed to the
linker on macOS.

This PR contains part of the patch found in #2577

@tschoonj
Copy link
Contributor Author

tschoonj commented Apr 3, 2018

@jpakkane As requested...

@@ -902,14 +902,15 @@ def thread_flags(self, env):
ICC_OSX = 1
ICC_WIN = 2

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Flake8]

[E227] missing whitespace around bitwise or shift operator

@@ -902,14 +902,15 @@ def thread_flags(self, env):
ICC_OSX = 1
ICC_WIN = 2

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Flake8]

[E999] SyntaxError: invalid syntax

@@ -902,14 +902,15 @@ def thread_flags(self, env):
ICC_OSX = 1
ICC_WIN = 2

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Flake8]

[E225] missing whitespace around operator

This patch exploits the information residing in ltversion to set the
-compatibility_version and -current_version flags that are passed to the
linker on macOS.
@tschoonj tschoonj force-pushed the macos-versioning-fix-2 branch from 23ef07c to 25c302d Compare April 3, 2018 11:49
major = int(splitted[0])
minor = int(splitted[1])
revision = int(splitted[2])
args += ['-compatibility_version', '%d' % (major + minor + 1)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit baffling. It would mean that versions 1.2.3 and 2.1.3 would get the same compatibility version. Is that really what should happen? Is there a link to Apple's documentation that says what this should do? I tried looking some time ago but the docs I found were most unhelpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They would indeed have the same compatibility_version, but a different soversion, which is why this works.
Have a look at SharedLibrary::process_kwargs:

            elif self.ltversion:
                # library version is defined, get the soversion from that
                # We replicate what Autotools does here and take the first
                # number of the version by default.
                self.soversion = self.ltversion.split('.')[0]

@jpakkane
Copy link
Member

jpakkane commented Apr 8, 2018

From my limited understanding of library versioning on OSX this seems reasonable. Anyone with more experience want to comment? @nirbheek?

Copy link
Member

@nirbheek nirbheek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for working on this @tschoonj!

Do we want to also add some tests to verify that compatibility version and current version are working as intended?

@jpakkane jpakkane merged commit fa6ca16 into mesonbuild:master Apr 16, 2018
@tschoonj
Copy link
Contributor Author

Thanks @jpakkane and @nirbheek !

@nirbheek
Copy link
Member

nirbheek commented May 9, 2018

After discussing with @ePirat it looks like we merged this PR incorrectly. -current_version is not used for any decisions by the linker (so we can set it to be the project version), and -compatibility_version is meant to be set by the developer to signal backwards-compatibility, so that older apps can load newer libraries if the compatibility version matches.

This means that at best, we can set it to soversion. The versioning in this PR is not recommended by Apple, but is what is used by libtool, and we do not use libtool versioning anywhere else in Meson.

It is going to be very confusing if we version libraries on Linux in one way and on macOS using another way.

We should revert this PR.

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.

4 participants