Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#55583 closed defect (fixed)

snappy @1.1.7: restore shared library build (and use the up-to-date cmake PG)

Reported by: RJVB (René Bertin) Owned by: raimue (Rainer Müller)
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc: ryandesign (Ryan Carsten Schmidt), raimue (Rainer Müller)
Port: snappy

Description

The recent upgrade of port:snappy had to migrate to CMake but did so using the obsolete cmake 1.0 PG, doesn't follow the out-of-source build dir guideline, and above all lost the shared library which isn't built by default.

The attached patch updates the cmake PG to 1.1 (which does out-of-source builds by default), and activates the shared library build.

Attachments (2)

snappy.diff (802 bytes) - added by RJVB (René Bertin) 6 years ago.
patch-build-shared+static.diff (2.6 KB) - added by RJVB (René Bertin) 6 years ago.

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by RJVB (René Bertin)

Attachment: snappy.diff added

comment:1 Changed 6 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign added
Keywords: haspatch added
Summary: port:snappy : restore shared library build (and use the up-to-date cmake PG)snappy @1.1.7: restore shared library build (and use the up-to-date cmake PG)

It looks like this changes the port to install the shared library, and to no longer install the static library. Ideally we would like to install both the shared and the static libraries, like the port did before the switch to cmake.

Unfortunately it also looks like the current version and compatibility version of the shared library built by cmake are incorrect; they have regressed from what the autotools build used. This should cause anything that links with libsnappy.dylib to fail to run, saying the library found is too old. (Or, if the user has rev-upgrade enabled, will cause MacPorts to determine that those ports linking with libsnappy.dylib are broken and will rebuild them.) I couldn't find a bug tracker for snappy so I reported it to their mailing list.

comment:2 in reply to:  1 ; Changed 6 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to ryandesign:

will cause MacPorts to determine that those ports linking with libsnappy.dylib are broken

For example:

Incompatible library version: /opt/local/lib/libleveldb.1.19.dylib requires version 5.0.0 or later, but /opt/local/lib/libsnappy.1.dylib provides version 1.0.0
DEBUG: Marking /opt/local/lib/libleveldb.1.19.dylib as broken

comment:3 in reply to:  2 ; Changed 6 years ago by RJVB (René Bertin)

Replying to ryandesign:

I thought I'd seen a static library in the result but you're clearly right, the build system now generates either the one or the other.

The regression in the SOVERSION is unfortunate (but understandable given the from and to values) - it only has the side-effect below on Mac, AFAIK. I don't think it's very reasonable to request upstream to revert to using 5 as the minimum compatibility version, we'll rather have to absorb that on our side. Either in the post-destroot or else with a patch. It may be a few days before I'll get around to doing that so don't hesitate to beat me to it.

For example:

Incompatible library version: /opt/local/lib/libleveldb.1.19.dylib requires version 5.0.0 or later, but /opt/local/lib/libsnappy.1.dylib provides version 1.0.0
DEBUG: Marking /opt/local/lib/libleveldb.1.19.dylib as broken

Yes, but without the shared library you also get an error message, so either way dependents will have to be revbumped due to the upgrade to 1.1.7 .

In practice, leveldb was the only port that really linked to libsnappy on my system and leveldb had been upgraded so I simply rebuilt the new version. Problem solved.

comment:4 in reply to:  3 ; Changed 6 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to RJVB:

The regression in the SOVERSION is unfortunate (but understandable given the from and to values) - it only has the side-effect below on Mac, AFAIK. I don't think it's very reasonable to request upstream to revert to using 5 as the minimum compatibility version, we'll rather have to absorb that on our side. Either in the post-destroot or else with a patch. It may be a few days before I'll get around to doing that so don't hesitate to beat me to it.

Of course it's reasonable to request developers not release broken software.

For example:

Incompatible library version: /opt/local/lib/libleveldb.1.19.dylib requires version 5.0.0 or later, but /opt/local/lib/libsnappy.1.dylib provides version 1.0.0
DEBUG: Marking /opt/local/lib/libleveldb.1.19.dylib as broken

Yes, but without the shared library you also get an error message, so either way dependents will have to be revbumped due to the upgrade to 1.1.7 .

Sure.

In practice, leveldb was the only port that really linked to libsnappy on my system and leveldb had been upgraded so I simply rebuilt the new version. Problem solved.

There are other ports that link with libsnappy.

comment:5 in reply to:  4 Changed 6 years ago by RJVB (René Bertin)

Replying to ryandesign:

I don't think it's very reasonable to request upstream to revert to using 5 as the minimum compatibility version, we'll rather have to absorb that on our side. Either in the post-destroot or else with a patch. It may be a few days before I'll get around to doing that so don't hesitate to beat me to it.

Of course it's reasonable to request developers not release broken software.

There's nothing broken in their software in this aspect. They decided to change a versioning number from one that didn't make much sense to one that corresponds to the library major version. That's their right and yes, that breaks something on our end but not in a way that cannot be repaired by relinking.

In practice, leveldb was the only port that really linked to libsnappy on my system and leveldb had been upgraded so I simply rebuilt the new version. Problem solved.

There are other ports that link with libsnappy.

There are ports that claim they depend on libsnappy, like QtWebEngine but in practice that port apparently uses the copy it includes in its source tree.

Changed 6 years ago by RJVB (René Bertin)

comment:6 Changed 6 years ago by RJVB (René Bertin)

I've attached a patch that adds a no-frills solution to build both shared and static libraries. It could probably be done a bit more elegantly but given how small the project is there's hardly a penalty to building everything twice.

Changing the compatibility version back to 5.0.0 is a different matter. I've not found a good way to do this, because cmake imposes the SOVERSION as the compatibility version. So either we get libsnappy.dylib -> libsnappy.1.dylib -> libsnappy.1.1.7.dylib with compat. version 1.0.0, or we get libsnappy.dylib -> libsnappy.5.dylib -> libsnappy.1.1.7.dylib . The only way to restore the 5.0.0 compat. version would thus be to set the SOVERSION to 5 in the CMake file, and then to rename libsnappy.5.dylib (and update the link library symlink).

FWIW, the "5" number probably comes from the strange way libtool encodes library versions (see https://cmake.org/Bug/view.php?id=4383#c6870 and beyond; I'm guessing the previous snappy build system used libtools `-version-info 5:x:y) IOW, this is indeed an unhappy side-effect that bites on OS X only, not on Linux where only the filename counts (which hasn't changed).

comment:7 Changed 6 years ago by raimue (Rainer Müller)

Cc: raimue added

The switch to a static library only broke linking with libsnappy for C applications. See report for qemu in #56763.

comment:8 Changed 6 years ago by raimue (Rainer Müller)

I see no use case for a static library. We do not want any port to link with static libraries, as that would not allow updating without increasing the revision of all recursive dependents. We should only provide the shared library.

Pull request: https://github.com/macports/macports-ports/pull/2125

comment:9 Changed 6 years ago by RJVB (René Bertin)

The switch to a static library only broke linking with libsnappy for C applications

I didn't (intend to) claim that there was a linking problem with the static lib. The problem is that the shared library is gone, which means rebuilding all dependents (including the huge QtWebEngine).

Restoring the shared library without addressing the SOVERSION issue described above does cause a linking and/or runtime issue if you don't rebuild the dependents ... or don't install a copy of the runtime dylib (libsnappy.X.dylib) with the proper SOVERSION set.

We do not want any port to link with static libraries, as that would not allow updating without increasing the revision of all recursive dependents.

I think that would only be true if a port C depends on (uses) a library from a port B but also uses API from a static library from port A that is linked into and exposed by port B.

If port C calls code from port A it should declare a dependency on that port. If the dependency is indirect there is no need for that, but then there is also no need for recursive rev-bumping (only port B needs the revbump).

Example: poppler-qt4/5 and poppler. The Qt bindings are complete and evolve very little, if ever, so I don't think Qt ports using poppler through those wrappers have ever had to revbump (yet poppler breaks API quite frequently). IOW, poppler-qt *could* link libpoppler statically without any ill effect.

comment:10 Changed 6 years ago by raimue (Rainer Müller)

Owner: set to raimue
Resolution: fixed
Status: newclosed

In 816cd6b08d5c79c4bdfd7a9e269330fcad239538/macports-ports (master):

snappy: Build shared, disable static library

Closes: #55583

comment:11 Changed 6 years ago by raimue (Rainer Müller)

In 547e3c593e113338f7b524a79f679cc93e8a2af1/macports-ports (master):

Rev-bump ports linking with snappy

The snappy port accidentally only provided a static libsnappy.a instead
of libsnappy.dylib as a shared libary. All ports linking with libsnappy
need to be rebuilt due to the change in snappy @1.1.7_1.

See: #55583

Note: See TracTickets for help on using tickets.