Re: [PATCH] Update ncmpcpp to v0.6.2

[Mark H Weaver]

Paul van der Walt <address@hidden> writes:

> On 2015-02-27 at 17:13, quoth Mark H Weaver:
>>> Paul van der Walt <address@hidden> writes:
>> [...]
>>>> +    (arguments
>>>> +     '(#:configure-flags
>>>> +       '("BOOST_LIB_SUFFIX=")
>>>> +       #:phases
>>>> +       (alist-cons-after
>>>> +        'unpack 'autogen
>>>> +        (lambda _
>>>> +          (substitute* "autogen.sh"
>>>> +            (("/bin/sh") (which "bash")))
>>>
>>> Is "autogen.sh" not executable?  If it starts with a shebang, Guix
>>> should patch the file appropriately.
>>
>> The problem here is that this 'autogen' phase is run before the
>> 'patch-source-shebangs' phase, at my suggestion.  It has to be that way,
>> because 'patch-source-shebangs' (and 'patch-usr-bin-file') will need to
>> be run on the files created by 'autogen.sh'.
>
> This is the case. Is this therefore the cleanest way to do it?

Actually, having looked more carefully, I see that the only occurrence
of /bin/sh in autogen.sh is the shebang at the top.  Therefore, you
shouldn't need the 'substitute*' call at all, because of the way you are
running it below:

>>>> +          (setenv "NOCONFIGURE" "true")
>>>> +          (zero? (system* "bash" "autogen.sh")))

Also, since the shebang asked for /bin/sh, it's probably marginally more
correct to use "sh" instead of "bash" here.  Bash modifies its behavior
somewhat when run as "sh".

On the whole, I think the preferable approach (and the one I've used in
a large commit that cleans up all of these autogen-style phases) is
this:

--8<---------------cut here---------------start------------->8---
       (alist-cons-after
        'unpack 'autogen
        (lambda _
          (setenv "NOCONFIGURE" "true")
          (zero? (system* "sh" "autogen.sh")))
--8<---------------cut here---------------end--------------->8---

> One last question:
>
>>> From e1382a9df3e7df94e794df8aa247920b7e28c0c6 Mon Sep 17 00:00:00 2001
>>> From: Paul van der Walt <address@hidden>
>>> ...
>>
>> Could you please add a copyright line for yourself at the top of this
>> file?
>
> Are these from lines not enough? My previous patch which i generated the
> same way was accepted.

That was my mistake.  I asked you to add a copyright line, but then
forgot about it when I looked at the updated patch.  Can you submit a
patch that adds your copyright line for your previous package update?

Or is there a reason you'd like to avoid adding your copyright?

     Mark