From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH 2/2] profiles: Build GTK+ input module cache.
Date: Mon, 26 Sep 2016 13:04:13 +0200
Message-ID: <878tuekjma.fsf@gnu.org>
References: <20160922202827.22039-1-rekado@elephly.net>
	<20160922202827.22039-2-rekado@elephly.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:51122)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1boYBE-0003q9-0w
	for guix-devel@gnu.org; Mon, 26 Sep 2016 11:51:00 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1boYBC-0005DU-Qd
	for guix-devel@gnu.org; Mon, 26 Sep 2016 11:50:55 -0400
Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:5250)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <ludo@gnu.org>)
	id 1boYBC-00057v-Kz
	for guix-devel@gnu.org; Mon, 26 Sep 2016 11:50:54 -0400
In-Reply-To: <20160922202827.22039-2-rekado@elephly.net> (Ricardo Wurmus's
	message of "Thu, 22 Sep 2016 22:28:27 +0200")
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel/>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: "Guix-devel" <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
To: Ricardo Wurmus <rekado@elephly.net>
Cc: guix-devel@gnu.org

Ricardo Wurmus <rekado@elephly.net> 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 modu=
les
> +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).

> +                ;; 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 =E2=80=A6))))

   ;; Fail when gtk-immodules-query fails.
   (zero? result)

> +    ;; 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 =E2=80=A6))
                    (else #f))))
    =E2=80=A6)
=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20
We should avoid the hardcoded version numbers though.

Otherwise LGTM.

Could you send an updated patch?

Thank you!

Ludo=E2=80=99.