Some options are really subcommands + ignore arguments

  • Open
  • quality assurance status badge
Details
4 participants
  • Perry, Daniel J
  • Maxim Cournoyer
  • Tobias Geerinckx-Rice
  • Simon Tournier
Owner
unassigned
Submitted by
Perry, Daniel J
Severity
normal
P
P
Perry, Daniel J wrote on 5 Oct 2023 05:35
Can't import package using archive command
(name . bug-guix@gnu.org)(address . bug-guix@gnu.org)
BLAPR07MB82894BA6CA6336CE4EE3EB77B5CAA@BLAPR07MB8289.namprd07.prod.outlook.com
1. guix archive --export hello > hello.nar
2. guix archive --import --authorize hello < hello.nar

This gives a fatal error on my machine (Debian on wsl) and was reproduced on Guix proper my a member of the IRC chat.
Attachment: file
T
T
Tobias Geerinckx-Rice wrote on 9 Oct 2023 14:25
(name . Perry, Daniel J)(address . dperry45@gatech.edu)
74541e429369739b82716cd72ee3b511@tobias.gr
retitle 66358 ‘guix archive --{authorize,import,…}’ are really
subcommands + ignore some arguments
thanks

Hi Daniel,

The error is correct, so I sure hope it's reproducible!

Later options cancel out previous ones, so your ‘--authorize’ action
takes precedence over ‘--import’. You're passing it a binary .nar
archive as a private key (which is an s-expression).

Boom.

Instead:

1. guix archive --export hello > hello.nar
2. guix archive --authorize < export-host.private-key
3. guix archive --import < hello.nar

Note that you had an extra ‘hello’ in your --import command as well.
Please note that it does nothing. Option parsing in Guix is
surprisingly lax.

I started thinking about a nice way to make ‘--action’ options mutually
exclusive, but reconsidered. I think it would violate POLA if not
POSIX.

I think it would be less surprising if these ‘single, mutually exclusive
actions’ should always be (sub)subcommands, e.g., ‘guix archive import’,
‘guix archive authorize’, …

I don't know if that change is still worth making here.

Kind regards,

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.
T
T
Tobias Geerinckx-Rice wrote on 9 Oct 2023 14:32
Re: bug#66358: Some options are really subcommands + ignore arguments
1ab2b75923c210dd7e7abc0ca0886f8b@tobias.gr
retitle 66358 Some options are really subcommands + ignore arguments
thanks

Today, we learn that ‘ and ’ are non-free:

Toggle quote (5 lines)
> Processing commands for control@debbugs.gnu.org:
>> retitle 66358 ‘guix archive --{authorize,import,…}’ are really
> Failed to set the title of 66358: Non-printable characters are not
> allowed in bug titles.

All are bugs, bugs are all.

Kind regards,

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.
P
P
Perry, Daniel J wrote on 9 Oct 2023 18:52
Now what about firmware?
(name . me@tobias.gr)(address . me@tobias.gr)(name . 66358@debbugs.gnu.org)(address . 66358@debbugs.gnu.org)
BLAPR07MB8289D2A3A8AC968831B837EAB5CEA@BLAPR07MB8289.namprd07.prod.outlook.com
Okay, great! I didn't think that they were mutually exclusive.
However, I still have a problem. You see, the reason why I'm using "guix archive" in the first place is because I don't have the wifi drivers for my laptop and I'm trying to install it without any Internet.

I don't think that simply adding the files to the store will work in this case, so I'll probably have to copy the source code, copy gcc-toolchain, copy the lisp code that describes the firmware package, and build there.

In any case, thanks a lot.
Attachment: file
S
S
Simon Tournier wrote on 12 Oct 2023 01:27
Re: bug#66358: Can't import package using archive command
86pm1kd9kd.fsf@gmail.com
Hi,

On Mon, 09 Oct 2023 at 14:25, Tobias Geerinckx-Rice via Bug reports for GNU Guix <bug-guix@gnu.org> wrote:

Toggle quote (4 lines)
> I think it would be less surprising if these ‘single, mutually exclusive
> actions’ should always be (sub)subcommands, e.g., ‘guix archive import’,
> ‘guix archive authorize’, …

I am proposing to error for ambiguous cases as,

$ ./pre-inst-env guix archive --import --authorize hello < /tmp/hello.nar
guix archive: error: ambiguous options: "authorize" with "import"

See attached patch. WDYT?

Please note that it errors when at least 2 options are ambiguous. So if
there is 3, you get the “two first ones“.

Toggle snippet (4 lines)
$ ./pre-inst-env guix archive --import --export --authorize hello < /tmp/hello.nar
guix archive: error: ambiguous options: "export" with "import"

Well, if the idea is fine, then maybe it could be worth to add one or
two sentences in the manual.

Cheers,
simon
From 673afe0384427bc92fa47870e1dfa092e04aec0b Mon Sep 17 00:00:00 2001
Message-Id: <673afe0384427bc92fa47870e1dfa092e04aec0b.1697066456.git.zimon.toutoune@gmail.com>
From: Simon Tournier <zimon.toutoune@gmail.com>
Date: Thu, 12 Oct 2023 01:16:53 +0200
Subject: [PATCH] scripts: archive: Check compatibility of command line
options.

Reported by Perry, Daniel J <dperry45@gatech.edu>.

* guix/scripts/archive.scm (compatible-option): New procedure.
(%options): Use it.
---
guix/scripts/archive.scm | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

Toggle diff (60 lines)
diff --git a/guix/scripts/archive.scm b/guix/scripts/archive.scm
index e32f22ec99..65932ae152 100644
--- a/guix/scripts/archive.scm
+++ b/guix/scripts/archive.scm
@@ -1,6 +1,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -115,6 +116,18 @@ (define %key-generation-parameters
"(genkey (ecdsa (curve Ed25519) (flags rfc6979)))"
"(genkey (rsa (nbits 4:4096)))"))
+(define (compatible-option action result other-actions)
+ "Return the RESULT if ACTION is compatible with the list of OTHER-ACTIONS."
+ (let ((other-action (fold (lambda (other answer)
+ (if (assoc-ref result (string->symbol other))
+ other
+ answer))
+ #f
+ other-actions)))
+ (if other-action
+ (leave (G_ "ambiguous options: ~s with ~s~%") action other-action)
+ (alist-cons (string->symbol action) #t result))))
+
(define %options
;; Specifications of the command-line options.
(cons* (option '(#\h "help") #f #f
@@ -126,13 +139,13 @@ (define %options
(show-version-and-exit "guix archive")))
(option '("export") #f #f
(lambda (opt name arg result)
- (alist-cons 'export #t result)))
+ (compatible-option "export" result (list "import" "authorize"))))
(option '(#\r "recursive") #f #f
(lambda (opt name arg result)
(alist-cons 'export-recursive? #t result)))
(option '("import") #f #f
(lambda (opt name arg result)
- (alist-cons 'import #t result)))
+ (compatible-option "import" result (list "export" "authorize"))))
(option '("missing") #f #f
(lambda (opt name arg result)
(alist-cons 'missing #t result)))
@@ -158,7 +171,7 @@ (define %options
(error-string err))))))
(option '("authorize") #f #f
(lambda (opt name arg result)
- (alist-cons 'authorize #t result)))
+ (compatible-option "authorize" result (list "import" "export"))))
(option '(#\S "source") #f #f
(lambda (opt name arg result)

base-commit: 0024ef320eed89468369ece3df05064a2afaabd1
--
2.38.1
M
M
Maxim Cournoyer wrote on 24 Oct 2023 16:48
Re: bug#66358: Some options are really subcommands + ignore arguments
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87h6mg3wlp.fsf_-_@gmail.com
Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

Toggle quote (19 lines)
> Hi,
>
> On Mon, 09 Oct 2023 at 14:25, Tobias Geerinckx-Rice via Bug reports
> for GNU Guix <bug-guix@gnu.org> wrote:
>
>> I think it would be less surprising if these ‘single, mutually exclusive
>> actions’ should always be (sub)subcommands, e.g., ‘guix archive import’,
>> ‘guix archive authorize’, …
>
> I am proposing to error for ambiguous cases as,
>
> $ ./pre-inst-env guix archive --import --authorize hello < /tmp/hello.nar
> guix archive: error: ambiguous options: "authorize" with "import"
>
> See attached patch. WDYT?
>
> Please note that it errors when at least 2 options are ambiguous. So if
> there is 3, you get the “two first ones“.

It seems a reasonable improvement to me, but I think *all* the
incompatible errors should be accumulated and printed in one go.

--
Thanks,
Maxim
S
S
Simon Tournier wrote on 24 Oct 2023 18:39
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87o7goht4u.fsf@gmail.com
Hi,

On Tue, 24 Oct 2023 at 10:48, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

Toggle quote (3 lines)
> It seems a reasonable improvement to me, but I think *all* the
> incompatible errors should be accumulated and printed in one go.

Toggle snippet (4 lines)
$ ./pre-inst-env guix archive --import --authorize hello --export < /tmp/hello.nar
guix archive: error: the options 'import' 'export' 'authorize' are exclusive

See v2 in #66592.

[bug#66592] [PATCH v2] scripts: archive: Check compatibility of command line options.
Simon Tournier <zimon.toutoune@gmail.com>
Tue, 24 Oct 2023 18:33:10 +0200
id:5c26f17bbf1b4cf9872b4a782295260ce337d3fd.1698165008.git.zimon.toutoune@gmail.com

Cheers,
simon
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 66358@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 66358
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch