Created attachment 165386 [details] Use bsd.lib.mk to build and install both shared and static libraries The current version of the port provides a static library (libsquish.a) only with PIC-compiled objects. The attached patch makes it provide both -- a static library with non-PIC objects plus a shared library. The change relies on bsd.lib.mk and removes the need for gmake. This will allow multiple processes to share the code in memory and depending ports to LIB_DEPEND on libsquish.
This only builds on 11.x, on other versions: Error: Missing: lib/libsquish.a Error: Missing: lib/libsquish.so Error: Missing: lib/libsquish.so.1 Actually I'd prefer to stay as far from bsd.*.mk in ports as possible. When it's used it always requires a ton of hacks to make work properly (like UID/GID handling) and it's still fragile. PS. Btw, you may use USES=uidfix to get GID (instead of calling two subprocesses) and LIB{OWN,GRP} defined for you. And I'd prefer MAKE_ARGS= foo \ bar to MAKE_ARGS= foo MAKE_ARGS+= bar As the former is much more readable.
(In reply to Dmitry Marakasov from comment #1) > This only builds on 11.x, on other versions: > Error: Missing: lib/libsquish.a > Error: Missing: lib/libsquish.so > Error: Missing: lib/libsquish.so.1 Weird -- my own primary platform is 10.2 here, and it worked. Can you check your environment? I used to create files/BSDmakefile in such cases, but today's make can be given just the bsd.foo.mk and will find it itself under /usr/share/mk > Actually I'd prefer to stay as far from bsd.*.mk in ports as possible. > When it's used it always requires a ton of hacks to make work properly > (like UID/GID handling) and it's still fragile. Funny, my experience is the opposite -- I find the bsd.*.mk collection to be much more useful and stable to the usual alternative of libtool-based builds (talk about hacks!)... For example, the graphics/libfpx/files/Makefile.bsd did not need changes in years. But squish in particular offers no bundled way to build shared library at all... > MAKE_ARGS= foo \ > bar > to > MAKE_ARGS= foo > MAKE_ARGS+= bar > As the former is much more readable. I don't have a strong preference of my own, but style(9) used to hint at the latter in the examples containing long lists of declared variables. Not sure, how much a kernel style guide for C is applicable to ports Makefiles, but Uses/uidfix.mk, for example, uses += notation and that was the last portmgr-maintained makefile I visited :-) (see Bug 206126). But, like I said, I don't really care.
(In reply to Mikhail Teterin from comment #2) > Weird -- my own primary platform is 10.2 here, and it worked. Can you check > your environment? I used to create files/BSDmakefile in such cases, but > today's make can be given just the bsd.foo.mk and will find it itself under > /usr/share/mk 10.2 works, but 10.1 doesn't. > Funny, my experience is the opposite -- I find the bsd.*.mk collection to be > much more useful and stable to the usual alternative of libtool-based builds > (talk about hacks!)... > > For example, the graphics/libfpx/files/Makefile.bsd did not need changes in > years. But squish in particular offers no bundled way to build shared > library at all... You must be kidding. The whole makefile is a huge hack in which, despite that you're using so-called framework, you do everything by hand, and where you don't do stuff by hand, you fix the framework to work correctly with ports. And even though you have a completely custom Makefile, you still need 2 hacks in the port itself, uidfix and she stuff after bsd.port.mk, while the latter is forbidden by the PHB. Why'd you even need all that when straightforward GNU_CONFIGURE works out of box?
(In reply to Dmitry Marakasov from comment #3) > 10.2 works, but 10.1 doesn't. Weird... Can you attach the log? Ok, may be on still has to use a little files/BSDmakefile instead of going for bsd.lib.mk directly. > The whole makefile is a huge hack in which [...] :-) Is it? I'll admit, that it is a little hairy, but that's because the upstream sources are a mixture of several different libraries. But even that one did not require much maintenance once done -- so it is not "fragile". Ok, maybe, it was not the best example -- how about x11/gpctool? Or security/pam_pwdfile? net/libtnl? > Why'd you even need all that when straightforward GNU_CONFIGURE works > out of box? In case of libfpx this was not true at first. And any patch would've been bigger -- and hairier -- than a straight Makefile of our own. I don't think, we'll convince one another here as to approach. If the maintainer is Ok with my proposal, I'm tempted to commit it -- unless it does not actually work on some of the OS-releases. In this case, I'll resumbit the patch with a separate files/BSDmakefile instead.
(In reply to Mikhail Teterin from comment #4) > Weird... Can you attach the log? I can, but now I wonder why haven't you tested it yourself, and how you were going to commit it without proper testing. https://people.freebsd.org/~amdmi3/squish.log > > The whole makefile is a huge hack in which [...] > > :-) Is it? I'll admit, that it is a little hairy, but that's because the > upstream sources are a mixture of several different libraries. > > But even that one did not require much maintenance once done -- so it is not > "fragile". Ok, maybe, it was not the best example -- how about x11/gpctool? > Or security/pam_pwdfile? net/libtnl? I'll check these out later, for libfpx I already have a patch I'll submit as soon as dependent ports build-testing finishes. > I don't think, we'll convince one another here as to approach. If the > maintainer is Ok with my proposal, I'm tempted to commit it -- unless it > does not actually work on some of the OS-releases. In this case, I'll > resumbit the patch with a separate files/BSDmakefile instead. Well, I don't approve it. Separate Makefile could be ok as soon as it doesn't contain that many hacks, however, I'd also consider patching upstream Makefile to produce shared library, or, which seems the best for me, switching to newer upstream code which uses CMake and supports sse, altivec and shared library out of box. For the note, https://github.com/sarbian/libsquish contains code identical to upstream SVN head, example patch follows.
Created attachment 165407 [details] patch to switch to newer upstream source
Comment on attachment 165407 [details] patch to switch to newer upstream source According to the author -- see http://code.google.com/p/libsquish/ -- the 1.11 ``is exactly the same as squish 1.10 but has contributed project files for vs8 and vs9'' Have you checked, if what's available at git actually adds meaningful improvements to FreeBSD users?
> Well, I don't approve it. Separate Makefile could be ok as soon > as it doesn't contain that many hacks There are not "hacks" -- these same legitimate knobs specified on command-line will move to the separate little Makefile, that's all. I'll ask for the maintainer's approval. I don't see, why we need to argue about this bikeshed -- or seek each other's approvals. > I'd also consider patching upstream Makefile to produce shared library The patch will be bigger -- and harder to read -- than our own Makefile based on bsd.lib.mk > switching to newer upstream code which uses CMake and supports sse, > altivec and shared library out of box Has it been _released_ yet by the author? If not, I think, we should wait.
Created attachment 165408 [details] Use bsd.lib.mk to build and install both shared and static libraries This version creates a separate files/BSDmakefile instead of invoking bsd.lib.mk directly -- which seems troublesome on 10.1.
libsquish 1.10 and 1.11 only differ in the included Visual Studio solution. It seems that the github Dmitry linked is a third-party fork of the original subversion repository, and it contains a few additional changes on top of 1.11. I'm not opposed in principle to using those additional changes if the CMake files make building it easier, but the changes do seem to change the ABI. We would have to at least bump the soversion, I reckon. I'm not familiar enough with bsd.lib.mk to comment on it. I'll give the new patch a try.
(In reply to rddeblois from comment #10) There is a multitude of libsquish-forks on GitHub: https://github.com/search?utf8=%E2%9C%93&q=libsquish&type=Repositories&ref=searchresults I'd advise against picking one without a compelling reason -- such as considerable new functionality. I would not qualify improvements in build glue as a good reason on its own -- we can do that ourselves. (In reply to Dmitry Marakasov from comment #5) > for libfpx I already have a patch I'll submit as soon as dependent ports > build-testing finishes. Please, pardon my annoyance, but why would you bother building/testing other people's ports that build and install fine, while your own -- graphics/devil -- has had a problem of medium severity since 2012? I really wish, you'd applied your bandwidth to build-testing the patch attached to Bug 196161 first. Your last comment on that bug promised to take care of the reported problem a year ago...
Perhaps I'm doing something wrong, but your latest patch doesn't actually build for me on FreeBSD 10.1. It doesn't seem to actually ever build the libraries. ===> Building package for squish-1.10_2 pkg-static: Unable to access file /usr/home/rdb/ports/squish/work/stage/usr/local/lib/libsquish.a: No such file or directory pkg-static: Unable to access file /usr/home/rdb/ports/squish/work/stage/usr/local/lib/libsquish.so.1: No such file or directory pkg-static: Unable to access file /usr/home/rdb/ports/squish/work/stage/usr/local/lib/libsquish.so: No such file or directory
(In reply to rddeblois from comment #12) Ok, I get it... The LIB_CXX knob, that I used, was only added by bdrewery last April and is not known to 10.1. -LIB_CXX=squish +LIB=squish I can also upload yet another patch, but it probably is not worth it for a one-liner. The purpose of LIB_CXX was to have the standard C++ library automatically linked into the shared library produced. However, because squish does not export any C-functions, any consumer of it must be linked with C++ anyway, so that's not important.
> It seems that the github Dmitry linked is a third-party fork of the original > subversion repository, and it contains a few additional changes on top of > 1.11. Yes, but it's completely equal to original SVN. > I'm not opposed in principle to using those additional changes if the CMake > files make building it easier, but the changes do seem to change the ABI. > We would have to at least bump the soversion, I reckon. If we switch to it right away, we won't need to bump anything as there was no previous version of the shared library. However we'll need to check that new changes don't break API.
(In reply to Mikhail Teterin from comment #8) > > switching to newer upstream code which uses CMake and supports sse, > > altivec and shared library out of box > > Has it been _released_ yet by the author? If not, I think, we should wait. It only had a single commit last year so I really doubt it'll have any releases anytime soon. Still, WIP repo may contain useful changes, and it may be better to make use of these as long as compatibility is preserved.
The new mi@ patch more or less looks good to me, for the note. Still, I'd prefer newer code and CMake build.
Comment on attachment 165408 [details] Use bsd.lib.mk to build and install both shared and static libraries The patch builds fine for me with the change s/LIB_CXX/LIB/ . The only thing I'm not 100% sure about is whether the library shouldn't be called libsquish.so.1.10 since I don't think the ABI is guaranteed to remain consistent between minor versions - then again, this is such an infrequently updated library that it probably doesn't matter. I see no compelling reason at the moment to stray from the official release and use the latest version from a thirdparty git repository. It's something we could consider in the future should the need arise. I'm not qualified to comment on whether we should use bsd.lib.mk. I'll leave the rest up to Dmitry. Thanks!
(In reply to rddeblois from comment #17) > The patch builds fine for me with the change s/LIB_CXX/LIB/ . The only > thing I'm not 100% sure about is whether the library shouldn't be called > libsquish.so.1.10 since I don't think the ABI is guaranteed to remain > consistent between minor versions - then again, this is such an infrequently > updated library that it probably doesn't matter. Just for the note, CMake build installs with SONAME libsquish.so.0.0. On the one hand, we may stick to that, on the other - library from 1.10 may be not binary compatible with HEAD version, in which case if we switch to HEAD or later version at some point, and it still ships with libsquish.so.0.0, we'll have to change upstream version.
The CMake version building as .0.0 is undoubtedly not what we want. Not sure if it's a mistake or deliberate. Either way not too surprising as it's not a public release.
(In reply to rddeblois from comment #17) > The patch builds fine for me with the change s/LIB_CXX/LIB/ . The one I first submitted would've worked too after the above substitution. Dmitry objects to it on the grounds of aesthetics, and I agree, that putting these declarations into a separate file is neater. However, the neater method necessitates a separate file AND a directory (files/) and our usual approach is to stuff things into Makefile -- whether they are PLIST_FILES (instead of a separate pkg-plist) or post- and pre- scripts (instead of eponymous standalone files under scripts/). So, I'm fine either way -- what's your preference, Reinier?
(In reply to rddeblois from comment #19) > The CMake version building as .0.0 is undoubtedly not what we want. Why not? Though two digits is uncommon, it's completely legal SONAME.
(In reply to Mikhail Teterin from comment #20) Your last patch (plus s/LIB_CXX/LIB/ of course) looks cleanest to me, although I have no particularly strong preference.
A commit references this bug: Author: mi Date: Wed Jan 13 15:42:40 UTC 2016 New revision: 406051 URL: https://svnweb.freebsd.org/changeset/ports/406051 Log: Up until now the port installed only a static version of libsquish -- because the upstream's makefile only built that. Add our own little makefile, that properly builds both static and shared variants using bsd.lib.mk PR: 206125 Approved by: Reinier de Blois (maintainer) Changes: head/graphics/squish/Makefile head/graphics/squish/files/ head/graphics/squish/files/BSDmakefile
Committed. I think, graphics/devil can now reenable support for squish without problems downstream too, but that would need to be verified...