[PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[宋文武]

* gnu/packages/gtk.scm (gdk-pixbuf+svg): New variable.
---
 gnu/packages/gtk.scm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index 1a10e9b..79983d2 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -433,6 +433,33 @@ in the GNOME project.")
    (license license:lgpl2.0+)
    (home-page "https://developer.gnome.org/gdk-pixbuf/")))
 
+(define-public gdk-pixbuf+svg
+  (package (inherit gdk-pixbuf)
+    (name "gdk-pixbuf+svg")
+    (inputs
+     `(("librsvg" ,librsvg)
+       ,@(package-inputs gdk-pixbuf)))
+    (arguments
+     '(#:configure-flags '("--with-x11")
+       #:tests? #f ; tested by the gdk-pixbuf package already
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'install 'register-svg-loader
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let* ((out     (assoc-ref outputs "out"))
+                    (librsvg (assoc-ref inputs "librsvg"))
+                    (loaders
+                     (append
+                      (find-files out "^libpixbufloader-.*\\.so$")
+                      (find-files librsvg "^libpixbufloader-.*\\.so$")))
+                    (gdk-pixbuf-query-loaders
+                     (string-append out "/bin/gdk-pixbuf-query-loaders")))
+               (zero? (apply system* `(,gdk-pixbuf-query-loaders
+                                       "--update-cache" ,@loaders)))))))))
+    (description (string-append
+                  (package-description gdk-pixbuf)
+                  "  This version provides SVG support by default."))))
+
 (define-public at-spi2-core
   (package
    (name "at-spi2-core")
-- 
2.6.3

[PATCH 2/2] gnu: gtk+: Enable SVG support.

[宋文武]

* gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
instead of 'gtk-pixbuf'.
(gtk+)[arguments]: Remove 'wrap-gtk-encode-symbolic-svg' phase.
---
 gnu/packages/gtk.scm | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index 79983d2..0798062 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -557,7 +557,7 @@ is part of the GNOME accessibility project.")
    (outputs '("out" "doc"))
    (propagated-inputs
     `(("atk" ,atk)
-      ("gdk-pixbuf" ,gdk-pixbuf)
+      ("gdk-pixbuf" ,gdk-pixbuf+svg)
       ("pango" ,pango)))
    (inputs
     `(("cups" ,cups)
@@ -617,7 +617,7 @@ application suites.")
    (propagated-inputs
     `(("at-spi2-atk" ,at-spi2-atk)
       ("atk" ,atk)
-      ("gdk-pixbuf" ,gdk-pixbuf)
+      ("gdk-pixbuf" ,gdk-pixbuf+svg)
       ("libepoxy" ,libepoxy)
       ("libxcursor" ,libxcursor)
       ("libxi" ,libxi)
@@ -661,18 +661,7 @@ application suites.")
            (("SUBDIRS = gdk gtk a11y css reftests")
             "SUBDIRS = gdk"))
          #t)
-       (alist-cons-after
-        'install 'wrap-gtk-encode-symbolic-svg
-        ;; By using GdkPixbuf, gtk-encode-symbolic-svg needs to know
-        ;; librsvg's loaders.cache to handle SVG files.
-        (lambda* (#:key inputs outputs #:allow-other-keys)
-          (let* ((out (assoc-ref outputs "out"))
-                 (prog (string-append out "/bin/gtk-encode-symbolic-svg"))
-                 (librsvg (assoc-ref inputs "librsvg"))
-                 (loaders.cache (find-files librsvg "^loaders\\.cache$")))
-            (wrap-program prog
-              `("GDK_PIXBUF_MODULE_FILE" = ,loaders.cache))))
-        %standard-phases))))
+       %standard-phases)))
    (native-search-paths
     (list (search-path-specification
            (variable "GUIX_GTK3_PATH")
-- 
2.6.3

Re: [PATCH 2/2] gnu: gtk+: Enable SVG support.

[宋文武]

宋文武 <iyzsong@gmail.com> writes:

> * gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
> instead of 'gtk-pixbuf'.
Typo, it's 'gdk-pixbuf'.

This patch shouldn't apply directly to master right?  I think this
should be pushed with the themes patch and maybe some gtk+ releated
packages update to a seperated branch.

Re: [PATCH 2/2] gnu: gtk+: Enable SVG support.

[Jan Nieuwenhuizen]

> * gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
> -      ("gdk-pixbuf" ,gdk-pixbuf)
> +      ("gdk-pixbuf" ,gdk-pixbuf+svg)

Ah, I've been `waiting' for this.  Looked into it briefly, but
could not find it.  Thanks!

Greetings,
Jan

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar®  http://AvatarAcademy.nl  

Re: [PATCH 2/2] gnu: gtk+: Enable SVG support.

[Jookia]

On Mon, Mar 14, 2016 at 08:56:26PM +0800, 宋文武 wrote:
> 宋文武 <iyzsong@gmail.com> writes:
> 
> > * gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
> > instead of 'gtk-pixbuf'.
> Typo, it's 'gdk-pixbuf'.
> 
> This patch shouldn't apply directly to master right?  I think this
> should be pushed with the themes patch and maybe some gtk+ releated
> packages update to a seperated branch.

I'll add it to my set of patches, package some themes up that use all these
changes and get some people to test it. Perhaps that could be the branch.

Jookia.

Re: [PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[Andreas Enge]

Hello,

On Mon, Mar 14, 2016 at 08:38:12PM +0800, 宋文武 wrote:
> * gnu/packages/gtk.scm (gdk-pixbuf+svg): New variable.

does it need to be a variable, or would it not make more sense to simply
modify gdk-pixbuf?

Andreas

libtool archives introduce unwanted inputs to build

[宋文武]

Report by Jookia in #guix that with my gdk-pixbuf+svg patches,
thunar still can't find SVG icons, it's linked with the old gdk-pixbuf.

While the build inputs of ‘thunar’ only have ‘gdk-pixbuf+svg’, one
of its input ‘libnotify’ was build with ‘gdk-pixbuf’.  It didn’t
propagated it but it’s in ‘libnotify.la’.

Remove ‘gdk-pixbuf’ from libnotify’s inputs or the ‘libnotify.la’ file
will fix thunar.

So libtool archives are the hidden propagated-inputs, maybe we should
remove them all?

Re: [PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[宋文武]

Andreas Enge <andreas@enge.fr> writes:

> Hello,
>
> On Mon, Mar 14, 2016 at 08:38:12PM +0800, 宋文武 wrote:
>> * gnu/packages/gtk.scm (gdk-pixbuf+svg): New variable.
>
> does it need to be a variable, or would it not make more sense to simply
> modify gdk-pixbuf?
gdk-pixbuf+svg depends on librsvg, while librsvg depends on gdk-pixbuf,
so I think there must be two version.  and the closure size of
gdk-pixbuf+svg is 210MB while gdk-pixbuf has 116MB, so I think we should
using the large one only when necessary.

Re: [PATCH 2/2] gnu: gtk+: Enable SVG support.

[Ludovic Courtès]

iyzsong@member.fsf.org (宋文武) skribis:

> 宋文武 <iyzsong@gmail.com> writes:
>
>> * gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
>> instead of 'gtk-pixbuf'.
> Typo, it's 'gdk-pixbuf'.
>
> This patch shouldn't apply directly to master right?  I think this
> should be pushed with the themes patch and maybe some gtk+ releated
> packages update to a seperated branch.

Right, it should be in a separate branch, say ‘gtk-update’.

Ludo’.

Re: [PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[Ludovic Courtès]

宋文武 <iyzsong@gmail.com> skribis:

> * gnu/packages/gtk.scm (gdk-pixbuf+svg): New variable.

[...]

> +    (description (string-append
> +                  (package-description gdk-pixbuf)
> +                  "  This version provides SVG support by default."))))

‘description’ and ‘synopsis’ must be literal strings so they are
internationalized.

So instead, you could write something like:

  (synopsis "GNOME image loading and manipulation library, with SVG support")

OK for master with this change.

Thanks,
Ludo’.

Re: libtool archives introduce unwanted inputs to build

[Ludovic Courtès]

iyzsong@member.fsf.org (宋文武) skribis:

> Report by Jookia in #guix that with my gdk-pixbuf+svg patches,
> thunar still can't find SVG icons, it's linked with the old gdk-pixbuf.
>
> While the build inputs of ‘thunar’ only have ‘gdk-pixbuf+svg’, one
> of its input ‘libnotify’ was build with ‘gdk-pixbuf’.  It didn’t
> propagated it but it’s in ‘libnotify.la’.
>
> Remove ‘gdk-pixbuf’ from libnotify’s inputs or the ‘libnotify.la’ file
> will fix thunar.
>
> So libtool archives are the hidden propagated-inputs, maybe we should
> remove them all?

I don’t think so.  :-)

From what I understand the problem is that we end up with the two
variants of gdk-pixbuf in the dependency graph, and it turns out that
it’s not the one we were hoping for that gets picked up.

The solution to this is to “debug” the dependency graph and arrange
things so that the dependency graph of GTK+ includes only one variant of
gdk-pixbuf.  (It might be that it’s easier said than done, but we have
to try!)

WDYT?

Ludo’.

Re: [PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[Andreas Enge]

On Tue, Mar 15, 2016 at 05:31:32PM +0800, 宋文武 wrote:
> gdk-pixbuf+svg depends on librsvg, while librsvg depends on gdk-pixbuf,
> so I think there must be two version.  and the closure size of
> gdk-pixbuf+svg is 210MB while gdk-pixbuf has 116MB, so I think we should
> using the large one only when necessary.

Okay, thanks for the explanation; maybe you could add it also as a comment
to the patch, so that later we do not wonder why there are two packages.

Andreas

Re: [PATCH 2/2] gnu: gtk+: Enable SVG support.

[Jookia]

On Tue, Mar 15, 2016 at 05:09:34PM +0100, Ludovic Courtès wrote:
> iyzsong@member.fsf.org (宋文武) skribis:
> 
> > 宋文武 <iyzsong@gmail.com> writes:
> >
> >> * gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
> >> instead of 'gtk-pixbuf'.
> > Typo, it's 'gdk-pixbuf'.
> >
> > This patch shouldn't apply directly to master right?  I think this
> > should be pushed with the themes patch and maybe some gtk+ releated
> > packages update to a seperated branch.
> 
> Right, it should be in a separate branch, say ‘gtk-update’.

If you're looking for GTK patches to shove in to a branch, I have a few ready,
though most have been put in the mailing list. I have some grafted ones too.

> Ludo’.

Jookia.

Re: libtool archives introduce unwanted inputs to build

[Jookia]

On Tue, Mar 15, 2016 at 05:14:53PM +0100, Ludovic Courtès wrote:
> iyzsong@member.fsf.org (宋文武) skribis:
> I don’t think so.  :-)
> 
> From what I understand the problem is that we end up with the two
> variants of gdk-pixbuf in the dependency graph, and it turns out that
> it’s not the one we were hoping for that gets picked up.
> 
> The solution to this is to “debug” the dependency graph and arrange
> things so that the dependency graph of GTK+ includes only one variant of
> gdk-pixbuf.  (It might be that it’s easier said than done, but we have
> to try!)
> 
> WDYT?

I don't know about this, if we're giving the option for applications to use two
variants at once then they should be able to do it. If we can only handle one
gdk-pixbuf, I think we should have a gdk-pixbuf that has SVG support by default.

That said, I don't know where we'd start looking at the dependency graph for all
packages on Guix that indirectly require GTK and make sure they don't use
gdk-pixbuf directly.

> Ludo’.

Jookia.

Re: [PATCH 1/2] gnu: Add gdk-pixbuf+svg.

[宋文武]

ludo@gnu.org (Ludovic Courtès) writes:

> 宋文武 <iyzsong@gmail.com> skribis:
>
>> * gnu/packages/gtk.scm (gdk-pixbuf+svg): New variable.
>
> [...]
>
>> +    (description (string-append
>> +                  (package-description gdk-pixbuf)
>> +                  "  This version provides SVG support by default."))))
>
> ‘description’ and ‘synopsis’ must be literal strings so they are
> internationalized.
>
> So instead, you could write something like:
>
>   (synopsis "GNOME image loading and manipulation library, with SVG support")
>
Done, Thanks!

Re: [PATCH 2/2] gnu: gtk+: Enable SVG support.

[宋文武]

ludo@gnu.org (Ludovic Courtès) writes:

> iyzsong@member.fsf.org (宋文武) skribis:
>
>> 宋文武 <iyzsong@gmail.com> writes:
>>
>>> * gnu/packages/gtk.scm (gtk+, gtk+-2)[propagated-inputs]: Use 'gtk-pixbuf+svg'
>>> instead of 'gtk-pixbuf'.
>> Typo, it's 'gdk-pixbuf'.
>>
>> This patch shouldn't apply directly to master right?  I think this
>> should be pushed with the themes patch and maybe some gtk+ releated
>> packages update to a seperated branch.
>
> Right, it should be in a separate branch, say ‘gtk-update’.
Sure, I push it and some packages updates to ‘gtk-update’, will pick
them later after the next guix release :-)

Re: libtool archives introduce unwanted inputs to build

[Ludovic Courtès]

Jookia <166291@gmail.com> skribis:

> On Tue, Mar 15, 2016 at 05:14:53PM +0100, Ludovic Courtès wrote:
>> iyzsong@member.fsf.org (宋文武) skribis:
>> I don’t think so.  :-)
>> 
>> From what I understand the problem is that we end up with the two
>> variants of gdk-pixbuf in the dependency graph, and it turns out that
>> it’s not the one we were hoping for that gets picked up.
>> 
>> The solution to this is to “debug” the dependency graph and arrange
>> things so that the dependency graph of GTK+ includes only one variant of
>> gdk-pixbuf.  (It might be that it’s easier said than done, but we have
>> to try!)

[...]

> That said, I don't know where we'd start looking at the dependency graph for all
> packages on Guix that indirectly require GTK and make sure they don't use
> gdk-pixbuf directly.

I would suggest looking at the dependency graph of Thunar since that’s
the one 宋文武 mentioned.  For instance:

  guix graph -t references thunar | grep -e '-> .*gdk-pixbuf'

Ludo’.