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