From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH] guix: gnu-build-system: add new phase
	patch-dot-desktop-files
Date: Sat, 24 Sep 2016 14:15:13 +0900
Message-ID: <87twd5q3oe.fsf@gnu.org>
References: <87wpig584u.fsf@gnu.org>
	<1474389194-7986-1-git-send-email-jmd@gnu.org>
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]:47776)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1bnhvD-0004bM-TR
	for guix-devel@gnu.org; Sat, 24 Sep 2016 04:02:56 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1bnhvC-0006VY-Tt
	for guix-devel@gnu.org; Sat, 24 Sep 2016 04:02:55 -0400
Received: from fencepost.gnu.org ([2001:4830:134:3::e]:34171)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <ludo@gnu.org>)
	id 1bnhvC-0006Tu-QZ
	for guix-devel@gnu.org; Sat, 24 Sep 2016 04:02:54 -0400
In-Reply-To: <1474389194-7986-1-git-send-email-jmd@gnu.org> (John Darrington's
	message of "Tue, 20 Sep 2016 18:33:14 +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: John Darrington <jmd@gnu.org>
Cc: guix-devel@gnu.org

Hello!

John Darrington <jmd@gnu.org> skribis:

> Ludo wanted something like this, I think.  To be pushed to core-updates o=
f course...

This Ludo is a very demanding person=E2=80=A6  ;-)

> * guix/build/gnu-build-system.scm (patch-dot-desktop-files): New procedur=
e.

The approach looks good to me, so I=E2=80=99m just commenting on the style:

> +      ;; Search for BINARY in the output directory,
> +      ;; then all the input directories.
> +      (let lp ((dir-list (cons output-dir (map (lambda (i) (cdr i)) inpu=
ts))))
> +        (if (null? dir-list)
> +            ;; Leave unchanged if we cannot find the binary.
> +            binary
> +            (let ((resolv (find-files
> +                           (car dir-list)
> +                           (lambda (file stat)
> +                             ;; The candidate file must be a regular fil=
e,
> +                             ;; have execute permission and the correct =
name.
> +                             (and stat
> +                                  (eq? 'regular (stat:type stat))
> +                                  (not (zero? (logand #o001 (stat:perms =
stat))))
> +                                  ((file-name-predicate
> +                                    (string-append "^" binary "$")) file=
 stat))))))
> +
> +              (if (null? resolv)
> +                  (lp (cdr dir-list))
> +                  (car resolv))))))

Please use =E2=80=98match=E2=80=99 instead of car, cdr, etc. (see =E2=80=98=
patch-shebangs=E2=80=99 in
the same file for an example), and use full words such as =E2=80=9Cdirector=
ies=E2=80=9D
instead of =E2=80=9Cdir-list=E2=80=9D (info "(guix) Coding Style").

I think you can write:

  (string=3D? binary file)

instead of using =E2=80=98file-name-predicate=E2=80=99.

> +    (for-each (match-lambda
> +                (( _ . output-dir)
> +                 (for-each (lambda (f)
> +                             (substitute* f
> +                               (("^Exec=3D([^/[:blank:]\r\n]*)(.*)$" _ b=
inary rest)
> +                                (string-append
> +                                 "Exec=3D" (find-binary binary output-di=
r inputs) rest))
> +
> +                               (("^TryExec=3D([^/[:blank:]\r\n]*)(.*)$" =
_ binary rest)
> +                                (string-append
> +                                 "TryExec=3D" (find-binary binary output=
-dir inputs) rest))))
> +                           (find-files output-dir ".desktop$"))))

The =E2=80=98find-files=E2=80=99 regexp should be "\\.desktop$", or a predi=
cate:

  (lambda (file stat)
    (string-suffix? ".desktop" file))

Could you send an updated patch?

Thanks for working on it!

Ludo=E2=80=99.