Subject: Re: bug#51307: [PATCH 0/2] guix hash: eases conversion

From: Ludovic Courtès <[email-masked]>
Date: Sat, 30 Oct 2021 16:46:18 +0200

Hi,

zimoun <[email-masked]> skribis:

> * guix/scripts/hash.scm (guix-hash): Allow several files.
> [directory?]: New procedure.
> [file-hash]: Catch system-error.
> [hash-to-display]: New procedure.

Nice! Could you update guix.texi and tests/guix-hash.texi?

> -      (with-error-handling
> -        (if (assoc-ref opts 'recursive?)
> +      (if (and (assoc-ref opts 'recursive?)
> +               (directory? file))

This change is not related to the main purpose of the patch.

More importantly, note that ‘--recursive’ is not limited to directories:
it preserves file properties (directory, executable, or regular), so
it’s also useful for executable files for instance.  It can also be used
for regular files, even if it’s less useful.

> +    (define (hash-to-display thing)
> +      (match thing
> +        ((? file-exists? file)
> +         (fmt (file-hash file)))
> +        ("-" (with-error-handling
> +               (fmt (port-hash (assoc-ref opts 'hash-algorithm)
> +                               (current-input-port)))))

I’d swap the order of the two clauses and remove the call to
‘file-exists?’: if the file doesn’t exist, ‘file-hash’ will raise an
error.

> +        (x
> +         (leave (G_ "wrong argument~%")))))
> +
> +    (for-each
> +     (lambda (arg)
> +       (format #t "~a~%" (hash-to-display arg)))

Or just (compose display hash-to-display) ?

Maybe s/hash-to-display/formatted-hash/.

Could you send an updated patch?

Thanks,
Ludo’.