Re: 10/18: gnu: polkit: Update phase & snippet style.

[Tobias Geerinckx-Rice <me@tobias.gr>]

Hullo Mark,

On 2018-02-27 8:41, Mark H Weaver wrote:
>> nckx pushed a commit to branch master
>> in repository guix.
>> 
>> commit 3c4bbb4c52418c8daf8b0e4605e3912685c9f44a
>> Author: Tobias Geerinckx-Rice <me@tobias.gr>
>> Date:   Sat Feb 24 13:33:42 2018 +0100
>> 
>>     gnu: polkit: Update phase & snippet style.
>> 
>>     * gnu/packages/polkit.scm (polkit)[source]: End snippet with #t.
>>     [arguments]: Substitute INVOKE for SYSTEM* and end phases with #t.
> 
> This broke the build.

Oh no.

>> @@ -113,12 +116,12 @@
>>              ;; to install in /etc, and to instead install the 
>> skeletons in the
>>              ;; output directory.
>>              (let ((out (assoc-ref outputs "out")))
>> -             (zero? (apply system*
>> -                           "make" "install"
>> +             (invoke "make" "install"
>>                             (string-append "sysconfdir=" out "/etc")
>>                             (string-append "polkit_actiondir="
>>                                            out 
>> "/share/polkit-1/actions")
>> -                           make-flags))))))))
>> +                           make-flags)
>> +             #t))))))
> 
> It was a mistake to remove the 'apply'.  Before your change, 'apply'
> interpreted its final argument (make-flags) as a _list_ of strings to
> pass to 'system*', after the initial arguments.  Now that you've 
> removed
> the 'apply', that final list argument is going directly to 'invoke',
> which expects all of its arguments to be strings.  Hence, this:

[...]

> Did you test this before pushing it?

Well, obviously, yes, and, obviously, no :-)

I tested a working branch with more significant polkit changes (although 
that's turning out to be such a quagmire I'm not sure if it's worth it) 
on a remote machine, then pushed... whatever the hell this is from my 
signing netbook. I don't remember making or fixing this typo, but there 
it is. Nor can I really explain how they got mixed up. I'll not blame my 
complete lack of sleep again.

It seemed like a good idea to get preparatory-but-stand-alone changes 
like this out of my local branches; maybe not. This is the second time 
this week I've managed to look right at a patch and read over the 
obvious. Apply is... not that new.

Thanks (as well as to Ludo' for the revert),

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.