all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
@ 2020-09-04 15:38 Michael Rohleder
  2020-09-04 16:12 ` Pierre Langlois
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Rohleder @ 2020-09-04 15:38 UTC (permalink / raw)
  To: 43204; +Cc: Michael Rohleder

* 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





^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
  2020-09-04 15:38 [bug#43204] [PATCH] gnu: taglib: Propagate zlib Michael Rohleder
@ 2020-09-04 16:12 ` Pierre Langlois
  2020-09-05 11:23   ` Pierre Langlois
  0 siblings, 1 reply; 7+ messages in thread
From: Pierre Langlois @ 2020-09-04 16:12 UTC (permalink / raw)
  To: Michael Rohleder; +Cc: 43204

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




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
  2020-09-04 16:12 ` Pierre Langlois
@ 2020-09-05 11:23   ` Pierre Langlois
  2020-09-07 12:15     ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Pierre Langlois @ 2020-09-05 11:23 UTC (permalink / raw)
  To: Pierre Langlois; +Cc: 43204, mike

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
  2020-09-05 11:23   ` Pierre Langlois
@ 2020-09-07 12:15     ` Ludovic Courtès
  2020-09-07 13:26       ` Michael Rohleder
  2020-09-07 13:41       ` Pierre Langlois
  0 siblings, 2 replies; 7+ messages in thread
From: Ludovic Courtès @ 2020-09-07 12:15 UTC (permalink / raw)
  To: Pierre Langlois; +Cc: mike, 43204

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

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’.


[-- Attachment #2: Type: text/x-patch, Size: 2038 bytes --]

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.

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")

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
  2020-09-07 12:15     ` Ludovic Courtès
@ 2020-09-07 13:26       ` Michael Rohleder
  2020-09-07 13:41       ` Pierre Langlois
  1 sibling, 0 replies; 7+ messages in thread
From: Michael Rohleder @ 2020-09-07 13:26 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Pierre Langlois, 43204

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

Hey Ludo,

Ludovic Courtès <ludo@gnu.org> writes:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [bug#43204] [PATCH] gnu: taglib: Propagate zlib.
  2020-09-07 12:15     ` Ludovic Courtès
  2020-09-07 13:26       ` Michael Rohleder
@ 2020-09-07 13:41       ` Pierre Langlois
  2020-09-07 22:55         ` bug#43204: " Ludovic Courtès
  1 sibling, 1 reply; 7+ messages in thread
From: Pierre Langlois @ 2020-09-07 13:41 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: mike, Pierre Langlois, 43204

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]


Ludovic Courtès writes:

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#43204: [PATCH] gnu: taglib: Propagate zlib.
  2020-09-07 13:41       ` Pierre Langlois
@ 2020-09-07 22:55         ` Ludovic Courtès
  0 siblings, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2020-09-07 22:55 UTC (permalink / raw)
  To: Pierre Langlois; +Cc: 43204-done, mike

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

> Ludovic Courtès writes:

[...]

>> 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’.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-09-07 22:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-04 15:38 [bug#43204] [PATCH] gnu: taglib: Propagate zlib Michael Rohleder
2020-09-04 16:12 ` Pierre Langlois
2020-09-05 11:23   ` Pierre Langlois
2020-09-07 12:15     ` Ludovic Courtès
2020-09-07 13:26       ` Michael Rohleder
2020-09-07 13:41       ` Pierre Langlois
2020-09-07 22:55         ` bug#43204: " Ludovic Courtès

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.