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

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

[Mark H Weaver]

Hi Tobias,

me@tobias.gr (Tobias Geerinckx-Rice) writes:

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

> @@ -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:

--8<---------------cut here---------------start------------->8---
starting phase `install'
Backtrace:
           6 (primitive-load "/gnu/store/l507mfnkzf2rc5lmn5sgi7in5vh…")
In ice-9/eval.scm:
   191:35  5 (_ _)
In srfi/srfi-1.scm:
   863:16  4 (every1 #<procedure 935780 at /gnu/store/71d3rwa514j7v…> …)
In /gnu/store/71d3rwa514j7vy5l4vfivf68g5yxibvl-module-import/guix/build/gnu-build-system.scm:
   711:27  3 (_ _)
In ice-9/eval.scm:
    619:8  2 (_ #(#(#(#<directory (guile-user) 7ce140>) ((. #)) #) #))
In /gnu/store/71d3rwa514j7vy5l4vfivf68g5yxibvl-module-import/guix/build/utils.scm:
   587:16  1 (invoke "make" . _)
In unknown file:
           0 (system* "make" "install" "sysconfdir=/gnu/store/g597y…" …)

ERROR: In procedure system*:
Wrong type (expecting string): ()
note: keeping build directory `/tmp/guix-build-polkit-0.113.drv-1'
builder for `/gnu/store/mayw0pncrfmln3yjfm23b7xv3paaqy0m-polkit-0.113.drv' failed with exit code 1
cannot build derivation `/gnu/store/yd5n5ir4rdl9vyq31nx4qv0zzjvgwdyd-udisks-2.1.8.drv': 1 dependencies couldn't be built
cannot build derivation `/gnu/store/wwilvamvyzn41qsxq9705ghgb8cadjkg-gvfs-1.32.1.drv': 2 dependencies couldn't be built
guix system: error: build failed: build of `/gnu/store/wwilvamvyzn41qsxq9705ghgb8cadjkg-gvfs-1.32.1.drv' failed
--8<---------------cut here---------------end--------------->8---

Did you test this before pushing it?

      Mark

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

[Tobias Geerinckx-Rice]

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.