GNU bug report logs - #43204
[PATCH] gnu: taglib: Propagate zlib.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43204 in the body.
You can then email your comments to 43204 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Rohleder <mike <at> rohleder.de>
To: guix-patches <at> gnu.org
Cc: Michael Rohleder <mike <at> rohleder.de>
Subject: [PATCH] gnu: taglib: Propagate zlib.
Date: Fri,  4 Sep 2020 17:38:24 +0200

* 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(-)

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

Message #8 received at 43204 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Michael Rohleder <mike <at> rohleder.de>
Cc: 43204 <at> debbugs.gnu.org, guix-patches <at> gnu.org
Subject: Re: [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
Date: Fri, 04 Sep 2020 17:12:50 +0100

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!

Pierre

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Pierre Langlois <pierre.langlois <at> gmx.com>
Cc: 43204 <at> debbugs.gnu.org, Michael Rohleder <mike <at> rohleder.de>,
 guix-patches <at> gnu.org
Subject: Re: [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
Date: Sat, 05 Sep 2020 12:23:43 +0100

[Message part 1 (text/plain, inline)]

Pierre Langlois writes:

> 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

[0]: https://github.com/taglib/taglib/issues/872

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 43204 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Pierre Langlois <pierre.langlois <at> gmx.com>
Cc: mike <at> rohleder.de, 43204 <at> debbugs.gnu.org
Subject: Re: [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
Date: Mon, 07 Sep 2020 14:15:41 +0200

[Message part 1 (text/plain, inline)]

Hi!

Pierre Langlois <pierre.langlois <at> 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’.

[Message part 2 (text/x-patch, inline)]

commit d8124a707602980556fd33c7dbf9f7483fe1d0df
Author: Ludovic Courtès <ludo <at> 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.

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 <at> enge.fr>
-;;; Copyright © 2014, 2015, 2017 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2014, 2015, 2017, 2020 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2016 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2017 Thomas Danckaert <post <at> 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")

Message #23 received at 43204 <at> debbugs.gnu.org (full text, mbox):

From: Michael Rohleder <mike <at> rohleder.de>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Pierre Langlois <pierre.langlois <at> gmx.com>, 43204 <at> debbugs.gnu.org
Subject: Re: [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
Date: Mon, 07 Sep 2020 15:26:49 +0200

[Message part 1 (text/plain, inline)]

Hey Ludo,

Ludovic Courtès <ludo <at> gnu.org> writes:
> I had the following patch that I intended to push, to avoid propagation.
>
> WDYT?
>
> commit d8124a707602980556fd33c7dbf9f7483fe1d0df
> Author: Ludovic Courtès <ludo <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #26 received at 43204 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: mike <at> rohleder.de, Pierre Langlois <pierre.langlois <at> gmx.com>,
 43204 <at> debbugs.gnu.org
Subject: Re: [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
Date: Mon, 07 Sep 2020 14:41:02 +0100

[Message part 1 (text/plain, inline)]

Ludovic Courtès writes:

> Hi!
>
> Pierre Langlois <pierre.langlois <at> 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 <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #31 received at 43204-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Pierre Langlois <pierre.langlois <at> gmx.com>
Cc: 43204-done <at> debbugs.gnu.org, mike <at> rohleder.de
Subject: Re: [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
Date: Tue, 08 Sep 2020 00:55:25 +0200

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

> Ludovic Courtès writes:

[...]

>> commit d8124a707602980556fd33c7dbf9f7483fe1d0df
>> Author: Ludovic Courtès <ludo <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.