[PATCH -v2] Fix gtk-im-modules for Gtk+3

[PATCH -v2] Fix gtk-im-modules for Gtk+3

[huang_ying_caritas]

From: Huang Ying <huang.ying.caritas@gmail.com>

Gtk+3 now have multiple outputs, so the gtk-query-immodules-3.0 should be find
in bin output.

The fix works, but appears hacky, because I haven't read much guix source
code, so I don't know the best solution.

Signed-off-by: "Huang, Ying" <huang.ying.caritas@gmail.com>
---
 guix/profiles.scm | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 495a9e2e7..9ae6dbf09 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -476,7 +476,7 @@ replace it."
                 (cons (gexp-input thing output) deps)))
               (manifest-entries manifest)))
 
-(define* (manifest-lookup-package manifest name #:optional version)
+(define* (manifest-lookup-package manifest name #:optional version output)
   "Return as a monadic value the first package or store path referenced by
 MANIFEST that is named NAME and optionally has the given VERSION prefix, or #f
 if not found."
@@ -499,6 +499,9 @@ if not found."
                 (and (string=? name name*)
                      (if version
                          (string-prefix? version version*)
+                         #t)
+                     (if output
+                         (string-suffix? output version*)
                          #t))))
             items))
 
@@ -737,9 +740,10 @@ creates the GTK+ 'icon-theme.cache' file for each theme."
 for both major versions of GTK+."
 
   (mlet %store-monad ((gtk+   (manifest-lookup-package manifest "gtk+" "3"))
+                      (gtk+-bin (manifest-lookup-package manifest "gtk+" "3" "bin"))
                       (gtk+-2 (manifest-lookup-package manifest "gtk+" "2")))
 
-    (define (build gtk gtk-version)
+    (define (build gtk gtk-version gtk-bin out)
       (let ((major (string-take gtk-version 1)))
         (with-imported-modules '((guix build utils)
                                  (guix build union)
@@ -756,7 +760,8 @@ for both major versions of GTK+."
 
               (let* ((prefix  (string-append "/lib/gtk-" #$major ".0/"
                                              #$gtk-version))
-                     (query   (string-append #$gtk "/bin/gtk-query-immodules-"
+                     (query   (string-append (ungexp gtk-bin out)
+                                             "/bin/gtk-query-immodules-"
                                              #$major ".0"))
                      (destdir (string-append #$output prefix))
                      (moddirs (cons (string-append #$gtk prefix "/immodules")
@@ -784,8 +789,8 @@ for both major versions of GTK+."
 
     ;; Don't run the hook when there's nothing to do.
     (let ((gexp #~(begin
-                    #$(if gtk+   (build gtk+   "3.0.0")  #t)
-                    #$(if gtk+-2 (build gtk+-2 "2.10.0") #t))))
+                    #$(if gtk+   (build gtk+   "3.0.0"  gtk+-bin "bin")  #t)
+                    #$(if gtk+-2 (build gtk+-2 "2.10.0" gtk+-2 "out") #t))))
       (if (or gtk+ gtk+-2)
           (gexp->derivation "gtk-im-modules" gexp
                             #:local-build? #t
-- 
2.11.0

Re: [PATCH -v2] Fix gtk-im-modules for Gtk+3

[Huang, Ying]

Hi, All,

huang_ying_caritas@163.com writes:

> From: Huang Ying <huang.ying.caritas@gmail.com>
>
> Gtk+3 now have multiple outputs, so the gtk-query-immodules-3.0 should be find
> in bin output.
>
> The fix works, but appears hacky, because I haven't read much guix source
> code, so I don't know the best solution.

I know this fix isn't good, but the problem exists.  Could anyone help
me to fix it properly?  Without the fix, I could not use fcitx input
method.  Because XIM immodule will hang some applications like gedit.

Best Regards,
Huang, Ying

> Signed-off-by: "Huang, Ying" <huang.ying.caritas@gmail.com>
> ---
>  guix/profiles.scm | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index 495a9e2e7..9ae6dbf09 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
> @@ -476,7 +476,7 @@ replace it."
>                  (cons (gexp-input thing output) deps)))
>                (manifest-entries manifest)))
>  
> -(define* (manifest-lookup-package manifest name #:optional version)
> +(define* (manifest-lookup-package manifest name #:optional version output)
>    "Return as a monadic value the first package or store path referenced by
>  MANIFEST that is named NAME and optionally has the given VERSION prefix, or #f
>  if not found."
> @@ -499,6 +499,9 @@ if not found."
>                  (and (string=? name name*)
>                       (if version
>                           (string-prefix? version version*)
> +                         #t)
> +                     (if output
> +                         (string-suffix? output version*)
>                           #t))))
>              items))
>  
> @@ -737,9 +740,10 @@ creates the GTK+ 'icon-theme.cache' file for each theme."
>  for both major versions of GTK+."
>  
>    (mlet %store-monad ((gtk+   (manifest-lookup-package manifest "gtk+" "3"))
> +                      (gtk+-bin (manifest-lookup-package manifest "gtk+" "3" "bin"))
>                        (gtk+-2 (manifest-lookup-package manifest "gtk+" "2")))
>  
> -    (define (build gtk gtk-version)
> +    (define (build gtk gtk-version gtk-bin out)
>        (let ((major (string-take gtk-version 1)))
>          (with-imported-modules '((guix build utils)
>                                   (guix build union)
> @@ -756,7 +760,8 @@ for both major versions of GTK+."
>  
>                (let* ((prefix  (string-append "/lib/gtk-" #$major ".0/"
>                                               #$gtk-version))
> -                     (query   (string-append #$gtk "/bin/gtk-query-immodules-"
> +                     (query   (string-append (ungexp gtk-bin out)
> +                                             "/bin/gtk-query-immodules-"
>                                               #$major ".0"))
>                       (destdir (string-append #$output prefix))
>                       (moddirs (cons (string-append #$gtk prefix "/immodules")
> @@ -784,8 +789,8 @@ for both major versions of GTK+."
>  
>      ;; Don't run the hook when there's nothing to do.
>      (let ((gexp #~(begin
> -                    #$(if gtk+   (build gtk+   "3.0.0")  #t)
> -                    #$(if gtk+-2 (build gtk+-2 "2.10.0") #t))))
> +                    #$(if gtk+   (build gtk+   "3.0.0"  gtk+-bin "bin")  #t)
> +                    #$(if gtk+-2 (build gtk+-2 "2.10.0" gtk+-2 "out") #t))))
>        (if (or gtk+ gtk+-2)
>            (gexp->derivation "gtk-im-modules" gexp
>                              #:local-build? #t

Re: [PATCH -v2] Fix gtk-im-modules for Gtk+3

[Danny Milosavljevic]

Hi,

On Mon, 30 Jan 2017 11:06:49 +0800
huang_ying_caritas@163.com wrote:

> -(define* (manifest-lookup-package manifest name #:optional version)
> +(define* (manifest-lookup-package manifest name #:optional version output)
>    "Return as a monadic value the first package or store path referenced by
>  MANIFEST that is named NAME and optionally has the given VERSION prefix, or #f
>  if not found."

Please update the docstring too.

> +                     (if output
> +                         (string-suffix? output version*)
>                           #t))))

Should that include the separator between the parts? Otherwise it could maybe have false positives...

Re: [PATCH -v2] Fix gtk-im-modules for Gtk+3

[Huang, Ying]

Hi, Danny,

Thanks for comments!

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi,
>
> On Mon, 30 Jan 2017 11:06:49 +0800
> huang_ying_caritas@163.com wrote:
>
>> -(define* (manifest-lookup-package manifest name #:optional version)
>> +(define* (manifest-lookup-package manifest name #:optional version output)
>>    "Return as a monadic value the first package or store path referenced by
>>  MANIFEST that is named NAME and optionally has the given VERSION prefix, or #f
>>  if not found."
>
> Please update the docstring too.

Sure.

>> +                     (if output
>> +                         (string-suffix? output version*)
>>                           #t))))
>
> Should that include the separator between the parts? Otherwise it
> could maybe have false positives...

OK.

Best Regards,
Huang, Ying

Re: [PATCH -v2] Fix gtk-im-modules for Gtk+3

[Ricardo Wurmus]

huang_ying_caritas@163.com writes:

> From: Huang Ying <huang.ying.caritas@gmail.com>
>
> Gtk+3 now have multiple outputs, so the gtk-query-immodules-3.0 should be find
> in bin output.

Ah, I was wondering why this broke!  Thanks for investigating it.

> The fix works, but appears hacky, because I haven't read much guix source
> code, so I don't know the best solution.

It’s only hacky because I tried to reuse the code for both versions of
Gtk.  Maybe “gtk+-2” should also get a “bin” output…?

This is acceptable with the changes Danny suggested.  Maybe we can find
a way to make this prettier in the future.

Thanks again for taking the time to investigate and fix it, and to
submit a patch!  I’m very grateful for this contribution.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

Re: [PATCH -v2] Fix gtk-im-modules for Gtk+3

[Huang, Ying]

Hi, Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

> huang_ying_caritas@163.com writes:
>
>> From: Huang Ying <huang.ying.caritas@gmail.com>
>>
>> Gtk+3 now have multiple outputs, so the gtk-query-immodules-3.0 should be find
>> in bin output.
>
> Ah, I was wondering why this broke!  Thanks for investigating it.
>
>> The fix works, but appears hacky, because I haven't read much guix source
>> code, so I don't know the best solution.
>
> It’s only hacky because I tried to reuse the code for both versions of
> Gtk.  Maybe “gtk+-2” should also get a “bin” output…?

That sounds reasonable for me.

> This is acceptable with the changes Danny suggested.  Maybe we can find
> a way to make this prettier in the future.

I have sent out -v3 which addressed Danny's comments with the following
subject,

[PATCH -v3] Fix gtk-im-modules for gtk+3

> Thanks again for taking the time to investigate and fix it, and to
> submit a patch!  I’m very grateful for this contribution.

You are welcome!

Best Regards,
Huang, Ying

> --
> Ricardo
>
> GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
> https://elephly.net