GNU bug report logs - #51307
[PATCH 0/2] guix hash: eases conversion

Previous Next

Package: guix-patches;

Reported by: zimoun <zimon.toutoune <at> gmail.com>

Date: Wed, 20 Oct 2021 16:51:01 UTC

Severity: normal

Tags: patch

Done: zimoun <zimon.toutoune <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: zimoun <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 51307 <at> debbugs.gnu.org
Subject: [bug#51307] [PATCH 0/2] guix hash: eases conversion
Date: Sat, 30 Oct 2021 17:40:59 +0200
Hi Ludo,

On Sat, 30 Oct 2021 at 16:46, Ludovic Courtès <ludo <at> gnu.org> wrote:
> zimoun <zimon.toutoune <at> gmail.com> 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?

I will once we agree and reach consensus. ;-)


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

I understand.  Could you provide concrete examples when it is not
directory?  In order to see if there is a pattern.

>> +    (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.

I can swap order but not remove file-exists?  Otherwise, the next clause…

>> +        (x
>> +         (leave (G_ "wrong argument~%")))))

will never happen.  And some tests are failing.

>> +    (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/.

Thanks.  Indeed both are better. :-)


> Could you send an updated patch?

Yes, but before, from my understanding, we should agree on the goal of
such patch. :-)  See the two other replies.

Cheers,
simon




This bug report was last modified 3 years and 155 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.