Re: [PATCH 2/2] profiles: Build GTK+ input module cache.

[Ricardo Wurmus]

Hi Ludo,

I had already pushed the change by the time I got this email, and then I
forgot to reply to it in time.  While trying to clean up my massive Guix
inbox I stumbled upon this unreplied email.  My apologies for the delay!

Ludovic Courtès <address@hidden> writes:

> Ricardo Wurmus <address@hidden> skribis:
>
>> * guix/profiles.scm (gtk-im-modules): New procedure.
>> (%default-profile-hooks): Add it.
>
> Very nice!
>
>> +(define (gtk-im-modules manifest)
>> +  "Return a derivation that builds the cache files for input method modules
>> +for both major versions of GTK+."
>> +
>> +  (mlet %store-monad ((gtk+   (manifest-lookup-package manifest "gtk+" "3"))
>> +                      (gtk+-2 (manifest-lookup-package manifest "gtk+" 
>> "2")))
>> +
>> +    (define (build gtk gtk-version)
>> +      (let ((major (string-take gtk-version 1)))
>
> Rather: (version-prefix gtk-version 1).

Okay.

>> +                ;; Generate a new 'immodules.cache' file.
>> +                (let ((pipe    (apply open-pipe*
>> +                                      OPEN_READ query
>> +                                      (map readlink (find-files destdir 
>> "\\.so$"))))
>> +                      (outfile (string-append #$output prefix
>> +                                              "/immodules-gtk" #$major 
>> ".cache")))
>> +                  (dynamic-wind
>> +                    (const #t)
>> +                    (lambda ()
>> +                      (call-with-output-file outfile
>> +                        (lambda (out)
>> +                          (while (not (eof-object? (peek-char pipe)))
>> +                            (write-char (read-char pipe) out))))
>> +                      #t)
>> +                    (lambda ()
>> +                      (close-pipe pipe)))))))))
>
> What about something along these lines instead:
>
>   (define result
>     (call-with-output-file "immodules.cache"
>       (lambda (port)
>         (close-fdes 1)
>         (dup->fdes port 1)
>         (system* query …))))
>
>    ;; Fail when gtk-immodules-query fails.
>    (zero? result)

This does indeed look nicer.  I’ll put this on my TODO list and revisit
the code at a later point in time.

>> +    ;; 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))))
>
> Simply:
>
>   (let ((gexp (cond (gtk+ (build gtk+ "3.0.0"))
>                     (gtk+-2 (built gtk+-2 …))
>                     (else #f))))
>     …)

Here I’m building one big gexp containing what’s needed for whatever
version of GTK is installed in the profile.  Your version would pick
only one of the expressions for a single version, which doesn’t cover
the case where both are installed.

> We should avoid the hardcoded version numbers though.

I agree but I don’t know how to avoid this elegantly.  These numbers
seem almost arbitrary.  Since we only have two major versions at this
point I don’t think it’s worth trying to generalise this.

~~ Ricardo