Subject: [bug#32102] [PATCH v2 2/2] gnu: gajim: Combine wrap-program phases.

From: [email-masked] (Ludovic Courtès)
Date: Sun, 29 Jul 2018 16:20:03 +0200

Hello Arun,

I’m a bit late to the party, but hey!

Arun Isaac <[email-masked]> skribis:

> From 6ee5cf4423109ab64df58c85f4114e456dda098b Mon Sep 17 00:00:00 2001
> From: Arun Isaac <[email-masked]>
> Date: Wed, 11 Jul 2018 13:03:33 +0530
> Subject: [PATCH v3 1/3] build-system: python: Do not double wrap executables.
> To: [email-masked]
> Cc: [email-masked],
>     [email-masked],
>     [email-masked]

Hmm, weird!

> * guix/build/python-build-system.scm (wrap): Only wrap executables that have
> not already been wrapped.
> * guix/build/utils.scm (is-wrapped?): New function.

[...]

> --- a/guix/build/python-build-system.scm
> +++ b/guix/build/python-build-system.scm

[...]

>  (define* (wrap #:key inputs outputs #:allow-other-keys)
>    (define (list-of-files dir)
> -    (map (cut string-append dir "/" <>)
> -         (or (scandir dir (lambda (f)
> -                            (let ((s (stat (string-append dir "/" f))))
> -                              (eq? 'regular (stat:type s)))))
> -             '())))
> +    (find-files dir (lambda (file stat)
> +                      (and (eq? 'regular (stat:type stat))
> +                           (not (is-wrapped? file))))))

Something I don’t get is that ‘wrap-program’ itself is supposed to
detect already-wrapped program.  I vaguely remember discussing it before
but I forgot what the conclusions were; do we really need extra
‘wrapped?’ checks?  Can’t we fix ‘wrap-program’ itself?

>    (define bindirs
>      (append-map (match-lambda
> diff --git a/guix/build/utils.scm b/guix/build/utils.scm
> index c58a1afd1..c310b792c 100644
> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm

[...]

> +(define (is-wrapped? prog)
> +  "Return #t if PROG is already wrapped using wrap-program, else return #f."
> +  (with-directory-excursion (dirname prog)
> +    (and-let* ((match-record (string-match "^\\.(.*)-real$" (basename prog))))
> +      (access? (match:substring match-record 1) X_OK))))

By convention I’d suggest calling it ‘wrapped?’ rather than
‘is-wrapped?’.  In fact, a more accurate name would be ‘wrapper?’.

Also I’d suggest not using SRFI-2 because IMO it doesn’t bring much and
it’s not used anywhere in Guix currently.  Also, ‘file-exists?’ rather
than ‘access?’, and no need to change directories.  So:

  (define (wrapper? prog)
    "Return #t if PROG is a wrapper as produced by 'wrap-program'."
    (and (file-exists? prog)
         (let ((base (basename prog)))
           (and (string-prefix? "." base)
                (string-suffix? "-real" base)))))

> From 71a7f6e39bd5b68b596bda2a75b1d245179349d0 Mon Sep 17 00:00:00 2001
> From: Arun Isaac <[email-masked]>
> Date: Sat, 28 Jul 2018 17:31:44 +0530
> Subject: [PATCH v3 3/3] gnu: gajim: Return #t from wrap-gsettings-schema-dir
>  phase.
> To: [email-masked]
> Cc: [email-masked],
>     [email-masked],
>     [email-masked]
>
> * gnu/packages/messaging.scm (gajim)[arguments]: Return #t from
> wrap-gsettings-schema-dir phase.

LGTM!

Thanks,
Ludo’.