[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#32102] [PATCH v2 2/2] gnu: gajim: Combine wrap-program phases.
From: |
Arun Isaac |
Subject: |
[bug#32102] [PATCH v2 2/2] gnu: gajim: Combine wrap-program phases. |
Date: |
Mon, 30 Jul 2018 06:00:28 +0530 |
address@hidden (Ludovic Courtès) writes:
>> From 6ee5cf4423109ab64df58c85f4114e456dda098b Mon Sep 17 00:00:00 2001
>> From: Arun Isaac <address@hidden>
>> Date: Wed, 11 Jul 2018 13:03:33 +0530
>> Subject: [PATCH v3 1/3] build-system: python: Do not double wrap executables.
>> To: address@hidden
>> Cc: address@hidden,
>> address@hidden,
>> address@hidden
>
> 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.
- [bug#32102] [PATCH] utils: Fix wrap-program filename generation., (continued)