Re: New “guix refresh” command

[ludo@gnu.org (Ludovic Courtès)]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> That could be done by changing ‘gnupg-verify*’.  An optional argument
>> could be added to select between interactive behavior (“do you want to
>> download this key and add it to your keyring?”), always-download, and
>> never-download.
>
> I'm attaching my attempt.

Thanks for looking into it!

> There are two similar but unrelated problems:
>
> 1. The following function doesn't print the message.
>
> (begin (format #t (_ "~a~a~!")
> 			   "Would you like to download this key "
> 			   "and add it to your keyring? (y/N) ")
> 	   (read-line))

First the whole string should be enclosed in (_ ...), otherwise xgettext
will just extract "~a~a" for translation.

Second, use ~% (instead of ~!); that will add a newline, and presumably
cause the string to be output.

Lastly, the indentation of (read-line) is incorrect.

> To-do list:
>
> 1. Any argument except 'always', 'never', and 'interactive' should raise
>    an error.
>
> 2. Fetch signatures first and don't download tarballs which can't be
>    authenticated (when signatures are missing and 'never' is used).
>
> 3. How should I change 'receive?' to support i18n?
>
> Anything else?

Comments below:

> +  #:use-module (ice-9 optargs)

This module is not needed (it’s for command-line argument processing.)

>  (define* (download-tarball store project directory version
> -                           #:optional (archive-type "gz"))
> +                           #:optional (archive-type "gz") download-sigs)

Perhaps change it to

  #:key (key-download 'interactive)

and document KEY-DOWNLOAD in the docstring.

> +(define* (package-update store package #:optional download-sigs)
>    "Return the new version and the file name of the new version tarball for
>  PACKAGE, or #f and #f when PACKAGE is up-to-date."

Likewise.

> +(define* (gnupg-verify* sig file #:optional download-sigs
> +                                            (server (%openpgp-key-server)))
>    "Like `gnupg-verify', but try downloading the public key if it's missing.
>  Return #t if the signature was good, #f otherwise."

Likewise.

> +          (define (receive?)
> +            (string=? "y"               ; XXX: i18n

Guile’s (ice-9 i18n) exports ‘locale-yes-regexp’ and ‘locale-no-regexp’
(info "(guile) Accessing Locale Information").

> +          (and
> +           missing
> +           ;; XXX: 'else' doesn't work.
> +           (cond ((string=? download-sigs "always")
> +                  (download-and-try-again))
> +                 ((string=? download-sigs "never")
> +                  #f)

‘download-sigs’ (rather, ‘key-download’) should be a symbol, not a
string (this is a common convention).  So this will read:

  (case key-download
    ((never) #f)
    ((always)
     (download-and-try-again))
    (else
     ;; ...
     ))

> +        (option '(#\d "download-sigs") #t #f

"key-download".

> +                (lambda (opt name arg result)
> +                  (alist-cons 'download-sigs arg result)))

Ditto.

>  When PACKAGE... is given, update only the specified packages.  Otherwise
>  update all the packages of the distribution, or the subset thereof
> -specified with `--select'.\n"))
> +specified with `--select'.
> +
> +'download-sigs' accepts one of the following arguments: 'interactive',
> +'always', and 'never'.  When 'download-sigs' is not specified, assume
> +'interactive'.\n"))

This should go...

> +  (display (_ "
> +  -d, --download-sigs=ARG
> +                         download and add signatures to your keyring"))

... here, IMO.

The string should rather be:

  --key-download=POLICY
         handle missing OpenPGP keys according to POLICY (one of ...)

> +(define* (update-package store package #:optional download-sigs)

Rename accordingly.

Thanks,
Ludo’.