guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Tobias Geerinckx-Rice
Subject: Re: 10/18: gnu: polkit: Update phase & snippet style.
Date: Tue, 27 Feb 2018 13:34:03 +0100
User-agent: Roundcube Webmail

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 <address@hidden>
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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]