[PATCH] gnu: taglib: Propagate zlib.

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Michael Rohleder
  • Pierre Langlois
Owner
unassigned
Submitted by
Michael Rohleder
Severity
normal

Debbugs page

Michael Rohleder wrote 5 years ago
(address . guix-patches@gnu.org)(name . Michael Rohleder)(address . mike@rohleder.de)
20200904153824.27164-1-mike@rohleder.de
* gnu/packages/mp3.scm (taglib)[inputs]: Move zlib to [propagated-inputs].
---
It seems, consumer of taglib (commit 89e1e44813) needs to be linked w/ libz
according to the installed pkg-config.

I noticed that emacs-emms-print-metadata fails to link with a missing -lz lib,
(I guess, all revdeps of taglib that don't have zlib as an input have that problem)

gnu/packages/mp3.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/packages/mp3.scm b/gnu/packages/mp3.scm
index 7ee009df74..8ea282be97 100644
--- a/gnu/packages/mp3.scm
+++ b/gnu/packages/mp3.scm
@@ -175,7 +175,7 @@ a highly stable and efficient implementation.")
(arguments
'(#:tests? #f ; Tests are not ran with BUILD_SHARED_LIBS on.
#:configure-flags (list "-DBUILD_SHARED_LIBS=ON")))
- (inputs `(("zlib" ,zlib)))
+ (propagated-inputs `(("zlib" ,zlib)))
(home-page "https://taglib.org")
(synopsis "Library to access audio file meta-data")
(description
--
2.28.0
Pierre Langlois wrote 5 years ago
(name . Michael Rohleder)(address . mike@rohleder.de)
87363xmsfx.fsf@gmx.com
Hi Michael,

Michael Rohleder writes:

Toggle quote (8 lines)
> * gnu/packages/mp3.scm (taglib)[inputs]: Move zlib to [propagated-inputs].
> ---
> It seems, consumer of taglib (commit 89e1e44813) needs to be linked w/ libz
> according to the installed pkg-config.
>
> I noticed that emacs-emms-print-metadata fails to link with a missing -lz lib,
> (I guess, all revdeps of taglib that don't have zlib as an input have that problem)

Oh, indeed emacs-emms doesn't build, sorry for the breakage! :-/ I see

Propagating zlib seems like the right thing to do (although I'm not a
maintainer), thanks for the patch!

Pierre
Pierre Langlois wrote 5 years ago
(name . Pierre Langlois)(address . pierre.langlois@gmx.com)
87wo18piv4.fsf@gmx.com
Pierre Langlois writes:

Toggle quote (18 lines)
> Hi Michael,
>
> Michael Rohleder writes:
>
>> * gnu/packages/mp3.scm (taglib)[inputs]: Move zlib to [propagated-inputs].
>> ---
>> It seems, consumer of taglib (commit 89e1e44813) needs to be linked w/ libz
>> according to the installed pkg-config.
>>
>> I noticed that emacs-emms-print-metadata fails to link with a missing -lz lib,
>> (I guess, all revdeps of taglib that don't have zlib as an input have that problem)
>
> Oh, indeed emacs-emms doesn't build, sorry for the breakage! :-/ I see
> the pkg-config file was changed here https://github.com/taglib/taglib/commit/ef1312d62239f399c40233d76ef3328b8dadf984
>
> Propagating zlib seems like the right thing to do (although I'm not a
> maintainer), thanks for the patch!

Actually, thinking about this a little more, I'm not sure I understand
upstream decision to propagate -lz. The commit fixes [0] which indicates
it's so that taglib can be linked statically, but then that means if
we're dynamically linking, the application will also dynamically link
with zlib when it doesn't need to (at least not directly). And in guix
we only build shared libs for taglib so we're never statically linking
it AFAIK.

So, here I'm a bit torn here, should we just follow what upstream is
indicating? Even it doesn't look right to me, but I might be wrong! Or,
should we revert the change that propagates -lz?

Thanks,
Pierre

-----BEGIN PGP SIGNATURE-----

iQFMBAEBCgA2FiEEctU9gYy29KFyWDdMqPyeRH9PfVQFAl9TdT8YHHBpZXJyZS5s
YW5nbG9pc0BnbXguY29tAAoJEKj8nkR/T31UKoIH/1Symc4WsyqqhGTafMjEbql8
ai5420ZcSv7DRWd9IHrffN+WeF44Nt+eVakMQaqqJHtTmAgER6UzjRK0zGxRv3Rp
Dr9py+zdhIwLOkjB2IhmOhztdjmOahj0a3xyPkdnaTdEEpqGNqe9rDruwwqekC15
IWCjWcZQ9w/5djV7MbHLXPDOnulJVVR9AG2Pk9O06Q8qSIk/p/nAdfvz1iDY08uX
Hg0oALdbRoxBrOdUdOywGSa545nNqGbLL/N7Ipc98jLfCS0dru93OY90ocBxP9no
bP65AvWiJiM8vFM6qIjTSMFc4ZBhF/AoBtHKGjrJvnzTjMS15C+adaowKuY4dM4=
=4MeF
-----END PGP SIGNATURE-----

Ludovic Courtès wrote 5 years ago
(name . Pierre Langlois)(address . pierre.langlois@gmx.com)
875z8pkck2.fsf@gnu.org
Hi!

Pierre Langlois <pierre.langlois@gmx.com> skribis:

Toggle quote (12 lines)
> Actually, thinking about this a little more, I'm not sure I understand
> upstream decision to propagate -lz. The commit fixes [0] which indicates
> it's so that taglib can be linked statically, but then that means if
> we're dynamically linking, the application will also dynamically link
> with zlib when it doesn't need to (at least not directly). And in guix
> we only build shared libs for taglib so we're never statically linking
> it AFAIK.
>
> So, here I'm a bit torn here, should we just follow what upstream is
> indicating? Even it doesn't look right to me, but I might be wrong! Or,
> should we revert the change that propagates -lz?

I had the following patch that I intended to push, to avoid propagation.

WDYT?

Ludo’.
commit d8124a707602980556fd33c7dbf9f7483fe1d0df
Author: Ludovic Courtès <ludo@gnu.org>
Date: Mon Sep 7 09:56:08 2020 +0200

gnu: taglib: 'taglib-config --libs' shows -L flag for zlib.
Fixes compilation of emacs-emms-print-metadata.
* gnu/packages/mp3.scm (taglib)[arguments]: Add #:phases.

Toggle diff (32 lines)
diff --git a/gnu/packages/mp3.scm b/gnu/packages/mp3.scm
index 7ee009df74..a7574f0cf9 100644
--- a/gnu/packages/mp3.scm
+++ b/gnu/packages/mp3.scm
@@ -1,6 +1,6 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2013 Andreas Enge <andreas@enge.fr>
-;;; Copyright © 2014, 2015, 2017 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014, 2015, 2017, 2020 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2017 Thomas Danckaert <post@thomasdanckaert.be>
@@ -174,7 +174,18 @@ a highly stable and efficient implementation.")
(build-system cmake-build-system)
(arguments
'(#:tests? #f ; Tests are not ran with BUILD_SHARED_LIBS on.
- #:configure-flags (list "-DBUILD_SHARED_LIBS=ON")))
+ #:configure-flags (list "-DBUILD_SHARED_LIBS=ON")
+ #:phases (modify-phases %standard-phases
+ (add-before 'configure 'adjust-zlib-ldflags
+ (lambda* (#:key inputs #:allow-other-keys)
+ ;; Make sure users of 'taglib-config --libs' get the -L
+ ;; flag for zlib.
+ (substitute* "CMakeLists.txt"
+ (("set\\(ZLIB_LIBRARIES_FLAGS -lz\\)")
+ (string-append "set(ZLIB_LIBRARIES_FLAGS -L"
+ (assoc-ref inputs "zlib")
+ " -lz)")))
+ #t)))))
(inputs `(("zlib" ,zlib)))
(home-page "https://taglib.org")
(synopsis "Library to access audio file meta-data")
Michael Rohleder wrote 5 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
87sgbtsoo6.fsf@rohleder.de
Hey Ludo,

Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (15 lines)
> I had the following patch that I intended to push, to avoid propagation.
>
> WDYT?
>
> commit d8124a707602980556fd33c7dbf9f7483fe1d0df
> Author: Ludovic Courtès <ludo@gnu.org>
> Date: Mon Sep 7 09:56:08 2020 +0200
>
> gnu: taglib: 'taglib-config --libs' shows -L flag for zlib.
>
> Fixes compilation of emacs-emms-print-metadata.
>
> * gnu/packages/mp3.scm (taglib)[arguments]: Add #:phases.
>

Nice!
I think we (or upstream) should do something like this anyway.

--
"If you want to travel around the world and be invited to speak at a lot
of different places, just write a Unix operating system."
Linus Torvalds
-----BEGIN PGP SIGNATURE-----

iQFFBAEBCAAvFiEEdV4t5dDVhcUueCgwfHr/vv7yyyUFAl9WNRkRHG1pa2VAcm9o
bGVkZXIuZGUACgkQfHr/vv7yyyWKlAgAic0tvH7pQVOOABlE1NDYUv7OjCdsvF0b
shXaoTe241XBi3P/P0xKbxHC5Z6XTey9JzXVOYMsCa4nLpPzBko3nzaFKuT5QYL5
BpY+7hWRSXKPOKB/OsNmLMRek/ahlgX1hiFYp6IFt1eWeabaRzsUgUuONlD90WDb
Z0811XVGgx/hUzgT5PahY72lWoVrOQhis9Zxy78nZdL1Vn7bPJ4wxbEGklNjs6gk
wSED2cbnHUmJXhC+gdkiRwqJTr5owbHdFkmEzaMr2NDEXl2IuLsr7loJXTxwc5Lu
hN78sq/cSSElZU0ES5YPDgqMb+CL9aK/odISbksWC6VTbEhEP0Yu9g==
=jeW4
-----END PGP SIGNATURE-----

Pierre Langlois wrote 5 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
87eendpuvl.fsf@gmx.com
Ludovic Courtès writes:

Toggle quote (32 lines)
> Hi!
>
> Pierre Langlois <pierre.langlois@gmx.com> skribis:
>
>> Actually, thinking about this a little more, I'm not sure I understand
>> upstream decision to propagate -lz. The commit fixes [0] which indicates
>> it's so that taglib can be linked statically, but then that means if
>> we're dynamically linking, the application will also dynamically link
>> with zlib when it doesn't need to (at least not directly). And in guix
>> we only build shared libs for taglib so we're never statically linking
>> it AFAIK.
>>
>> So, here I'm a bit torn here, should we just follow what upstream is
>> indicating? Even it doesn't look right to me, but I might be wrong! Or,
>> should we revert the change that propagates -lz?
>
> I had the following patch that I intended to push, to avoid propagation.
>
> WDYT?
>
> Ludo’.
>
> commit d8124a707602980556fd33c7dbf9f7483fe1d0df
> Author: Ludovic Courtès <ludo@gnu.org>
> Date: Mon Sep 7 09:56:08 2020 +0200
>
> gnu: taglib: 'taglib-config --libs' shows -L flag for zlib.
>
> Fixes compilation of emacs-emms-print-metadata.
>
> * gnu/packages/mp3.scm (taglib)[arguments]: Add #:phases.

LGTM!

I was originally thinking we could just drop the `-lz`, since it
/should/ only be needed for people who statically link with taglib, and
we only ship shared libs. But actually, it's probably safer to follow
what upstream is doing.

Thanks,
Pierre
-----BEGIN PGP SIGNATURE-----

iQFMBAEBCAA2FiEEctU9gYy29KFyWDdMqPyeRH9PfVQFAl9WOG4YHHBpZXJyZS5s
YW5nbG9pc0BnbXguY29tAAoJEKj8nkR/T31UxnUIAK5lzRBIWA6e2gltYGSBIgw6
Sjy36KME6/TBu8D2uT5009wJGLaHNYHrGEg7kO8hDvoEt4AiomV0fbIdt+LEIY+O
EBbtrSNyuc2tS1PLF2clf/sPjIOomzM+MiST0Fq0SizQBjHL8Ohw/MDl6YG/1l5Y
KP2vJGh21g0KY5IVtY3FRA/KVjzTVev/TNkFJqNjvrniEEwb4DLoZ/ngYZ5sw9Pu
302KHsMXgkm8TrrZty+573Qdz11mm/gqhZnuj4/NSVdrQbq5NUvxqTZg7p6fuemZ
cZ7auXP9WSIREg3bv7ZgLH3NNVs8tUFP6C9HemA/4rCdy0hpoUBjGauN5oQQHoc=
=TnBV
-----END PGP SIGNATURE-----

Ludovic Courtès wrote 5 years ago
(name . Pierre Langlois)(address . pierre.langlois@gmx.com)
874ko9fb8i.fsf@gnu.org
Pierre Langlois <pierre.langlois@gmx.com> skribis:

Toggle quote (2 lines)
> Ludovic Courtès writes:

[...]

Toggle quote (17 lines)
>> commit d8124a707602980556fd33c7dbf9f7483fe1d0df
>> Author: Ludovic Courtès <ludo@gnu.org>
>> Date: Mon Sep 7 09:56:08 2020 +0200
>>
>> gnu: taglib: 'taglib-config --libs' shows -L flag for zlib.
>>
>> Fixes compilation of emacs-emms-print-metadata.
>>
>> * gnu/packages/mp3.scm (taglib)[arguments]: Add #:phases.
>
> LGTM!
>
> I was originally thinking we could just drop the `-lz`, since it
> /should/ only be needed for people who statically link with taglib, and
> we only ship shared libs. But actually, it's probably safer to follow
> what upstream is doing.

Yeah. Pushed as d2a7114e0b46fccad1e02e301c58a5124f361c5c, thanks!

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 43204@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 43204
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help