Bug 216823
| Summary: | graphics/mupdf: Build shared libraries | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Tobias Kortkamp <tobik> | ||||||||||||
| Component: | Individual Port(s) | Assignee: | Tijl Coosemans <tijl> | ||||||||||||
| Status: | Closed FIXED | ||||||||||||||
| Severity: | Affects Only Me | CC: | tijl, uzsolt | ||||||||||||
| Priority: | --- | Keywords: | patch | ||||||||||||
| Version: | Latest | Flags: | uzsolt:
maintainer-feedback+
|
||||||||||||
| Hardware: | Any | ||||||||||||||
| OS: | Any | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Tobias Kortkamp
2017-02-05 13:40:22 UTC
I think the mupdf's PORTEPOCH increase a small typo - you want increase PORTREVISION (I think) :) It was a fix year 2014 about fPIC and the main patch was created by tijl: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192397 I'm not expert in .so-files but why do you need SO_MAJOR? It seems the mupdf and zathura-pdf-mupdf builds&works well. Created attachment 179671 [details] libmupdf.diff (In reply to Zsolt Udvari from comment #2) Thanks for testing! (In reply to Zsolt Udvari from comment #1) > I think the mupdf's PORTEPOCH increase a small typo - you want increase PORTREVISION (I think) :) I changed PORTVERSION -> DISTVERSION and 1.10a > 1.10.a according to `pkg version -t 1.10a 1.10.a`, so I think a PORTEPOCH bump is needed. But maybe the DISTVERSION change isn't even needed in this case. I'm mostly going by https://www.freebsd.org/doc/en/books/porters-handbook/makefile-distfiles.html and what I had to do with games/iortcw (bug #214959). Anyway I reverted the change and am bumping PORTREVISION instead :) > I'm not expert in .so-files but why do you need SO_MAJOR? I'm not either. AFAIUI, you need to have some kind of indicator for the ABI version of the library. In theory you could have two incompatible libmupdf's on the system e.g. libmupdf.so.1.0 and libmupdf.so.2.0 (e.g. from Mupdf 1.9a or 1.10a) with different binaries linking to either one of them and the dynamic linker needs to be able to distinguish the two. Since Mupdf doesn't include a way to build a shared library in its build system we have to invent your own versioning scheme and set the libs soname appropriately. Take a look at https://www.freebsd.org/doc/en/books/developers-handbook/policies-shlib.html. According to the article I probably should've started SO_MAJOR at 1, so I changed that too. I don't see the benefit of SO_MAJOR. Why isn't enough a simple libmupdf.so? Created attachment 179674 [details] libmupdf-unversioned.diff I don't think I can offer a better explanation to why I thought it was needed than what I tried to give in comment #3. Anyway there is not much point in arguing about it, so here is a version of the patch without SO_MAJOR and just libmupdf.so :) (In reply to Tobias Kortkamp from comment #5) I want to understand the SO_MAJOR. I think if OpenBSD's developers use this idea it has benefit/meaning. For example: $ readelf -a libcurl.so | grep -i soname 0x000000000000000e (SONAME) Library soname: [libcurl.so.4] The "SO_MAJOR" can read from libcurl.so and it matches with the filename. Can we will read it from libmupdf.so? If the "SONAME" and "SO_MAJOR" doesn't match what will happen? You cannot use DISTVERSION here because upstream released 1.10a after 1.10 and 1.10.a comes before 1.10.
There's an extra patch for graphics/llpp. What is that about?
SO_MAJOR is just an arbitrary name for a variable. What matters is that the library is now linked with -soname=$@ where $@ means the make target which is $(MUPDF_LIB) or $(THIRD_LIB).
If upstream does not guarantee ABI compatibility between versions (e.g. they may change function prototypes or change the size of types), then you must use a versioned soname. Programs that use the library will then fail to start with a new version because they are still looking for the old library name. This is a safer failure mode than the unpredictable failures that may happen when programs blindly use an unversioned and possibly incompatible library.
You can use any string as soname. I think it would be better to replace ${SO_MAJOR}.0 with $(SOVERSION) in the OpenBSD patch and set SOVERSION=${PORTVERSION} in the port Makefile.
(In reply to Tijl Coosemans from comment #7) @Tijl Thanks for explanation. Will the changed soname causes re-build on servers? And: does the linker think that the libmupdf.so.10a is newer than libmupdf.so.10? If yes I think too the "SONAME" can be PORTVERSION. (In reply to Zsolt Udvari from comment #8) Currently the package build servers always rebuild all ports that depend on an updated port, so they will pick up the changed library name, but you should not rely on this and always bump PORTREVISION on these ports. The linker does not compare versions. It only looks for exact matches. All that matters is that incompatible versions of a library have unique names. (In reply to Tijl Coosemans from comment #9) "The linker does not compare versions. It only looks for exact matches." Oh, indeed, I'm stupid. The 'ldd' always prints the exact filenames. One more question: is it enough to create links and "USE_LDCONFIG=yes"? @Tobias: if you use SOVERSION=${PORTVERSION}, I think, it's acceptable (both mupdf and zathura-pdf-mupdf) with the following modification: "@cd ${STAGEDIR}..." should be "cd ${STAGEDIR}..." (without @, to be verbose) @Tobias: thanks for your idea and work :) Created attachment 179681 [details] libmupdf.diff Here is the patch with the requested changes :) (In reply to Tijl Coosemans from comment #7) > There's an extra patch for graphics/llpp. What is that about? It's a (very low priority) fix for the brightness decrease key in llpp, which I haven't submitted yet and forgot to remove from the patch. I'm hoping it can go in with this change, but if that's inappropriate just say the word and I'll remove it. (In reply to Tobias Kortkamp from comment #12) Oh, I think I wasn't (enough) clear. In this case IMHO shouldn't introduce new variable (SOVERSION) in Makefile. Maybe it's enough: MAKE_ARGS=.... SOVERSION=${PORTVERSION} PLIST_SUB= SOVERSION=${PORTVERSION} I think we will not use SOVERSION which differs from PORTVERSION. Do you agree? Created attachment 179682 [details]
libmupdf.diff
(In reply to Zsolt Udvari from comment #10) > is it enough to create links and "USE_LDCONFIG=yes"? ldconfig won't cache this library I think. It's an old program that only looks for files named libNAME.so.N where N is a single number. There are many libraries nowadays that don't match that pattern. The port should still set USE_LDCONFIG=yes just in case ldconfig is ever fixed. The unversioned links created in post-install aren't related to ldconfig. They are used at compile-time when -lfoo becomes libfoo.so. A commit references this bug: Author: tijl Date: Tue Feb 7 13:40:00 UTC 2017 New revision: 433550 URL: https://svnweb.freebsd.org/changeset/ports/433550 Log: Patch graphics/mupdf to build shared libraries instead of static ones. Because the libraries are faily big this significantly reduces the size of programs linking to it. Use PORTVERSION as the library version because upstream does not guarantee compatibility between any two versions. Add an upstream patch to graphics/llpp to fix brightness increase key. PR: 216823 Submitted by: Tobias Kortkamp <t@tobik.me> (maintainer of llpp) Approved by: Zsolt Udvari <udvzsolt@gmail.com> (maintainer) Obtained from: OpenBSD Changes: head/graphics/llpp/Makefile head/graphics/llpp/files/patch-build.sh head/graphics/llpp/files/patch-main.ml head/graphics/mupdf/Makefile head/graphics/mupdf/files/patch-Makefile head/graphics/mupdf/pkg-plist head/graphics/zathura-pdf-mupdf/Makefile |