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

[Arun Isaac <arunisaac@systemreboot.net>]

ludo@gnu.org (Ludovic Courtès) writes:

>> From 6ee5cf4423109ab64df58c85f4114e456dda098b Mon Sep 17 00:00:00 2001
>> From: Arun Isaac <arunisaac@systemreboot.net>
>> Date: Wed, 11 Jul 2018 13:03:33 +0530
>> Subject: [PATCH v3 1/3] build-system: python: Do not double wrap executables.
>> To: clement@lassieur.org
>> Cc: mhw@netris.org,
>>     andreas@enge.fr,
>>     32102@debbugs.gnu.org
>
> Hmm, weird!

What's weird? Are you referring to the Cc field? The people in the Cc
field were originally referred to by Clement. So, I put them there to
keep them in the loop.

>>  (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?

Could you refer to our earlier discussion on 32102?

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32102

In the case of Gajim, our current wrapping ends up double wrapping and
creating bin/.gajim-real-real. The original fix I proposed was to modify
`wrap-program` to fix already-wrapped detection. But, after discussion
with Clement, we decided to go with a is-wrapped? check in the python
build system. Do check out our earlier discussion and let us know what
you think.

>> +(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?’.

Sure, will do.

> 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)))))

Sure, will do.