GNU bug report logs - #45409
[PATCH 0/3] Move some (guix scripts substitute) code to two new modules

Previous Next

Full log

View this message in rfc822 format

From: Christopher Baines <mail <at> cbaines.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45409 <at> debbugs.gnu.org
Subject: [bug#45409] [PATCH 1/3] guix: Move narinfo code from substitute script to module.
Date: Sun, 03 Jan 2021 18:16:13 +0000

[Message part 1 (text/plain, inline)]

Ludovic Courtès <ludo <at> gnu.org> writes:

> Christopher Baines <mail <at> cbaines.net> skribis:
>
>> This separation between the code for dealing with narinfos from the code doing
>> that for a purpose should make things clearer, and better support components
>> other that the substitute script in using this code.
>>
>> This is just moving the code around, no code should have been significantly
>> changed.
>>
>> * guix/scripts/substitute.scm (<narinfo>): Move record type to (guix narinfo).
>> (fields->alist, narinfo-hash-algorithm+value, narinfo-hash->sha256,
>> narinfo-signature->canonical-sexp, narinfo-maker, read-narinfo,
>> narinfo-sha256, valid-narinfo?, write-narinfo, narinfo->string,
>> string->narinfo, equivalent-narinfo?, supported-compression?,
>> compresses-better?, narinfo-best-uri): Move procedures to (guix narinfo).
>> (%compression-methods): Move variable to (guix narinfo).
>> * guix/narinfo.scm: New file.
>> * Makefile.am (MODULES): Add it.
>
> That’s a good idea!
>
> Please add guix/narinfo.scm to po/guix/POTFILES.in so it can be
> translated.

I've sent some updated patches now, and I've fixed this in them.

>> +(define-module (guix narinfo)
>> +  #:use-module (guix ui)
>
> We should try and avoid (guix ui); is (guix diagnostics) enough?

Yep, that seems to work fine.

>> +  #:use-module (guix scripts substitute)
>
> (guix …) modules must not depend on (guix scripts …).
>
> Perhaps that’s just for ‘%allow-unauthenticated-substitutes?’, no?  If
> so, let’s just not refer to ‘%allow-unauthenticated-substitutes?’ here.
> It’s a hack to allow for tests, so better keep it local to (guix scripts
> substitute).

I've moved the commit where I fix this to be the first one, so this
should be clearer now.

>> +(define* (valid-narinfo? narinfo #:optional (acl (current-acl))
>> +                         #:key verbose?)
>> +  "Return #t if NARINFO's signature is not valid."
>> +  (or (%allow-unauthenticated-substitutes?)
>
> Yeah, let’s remove it from here.  At worst, we can always use ‘mock’ in
> tests to make ‘valid-narinfo?’ return #t unconditionally.
>
> OK with these changes.
>
> After the change, please make sure “make check” and “make as-derivation”
> still pass.  For “make as-derivation”, we should also make sure
> ‘guix-core’ doesn’t pull in everything via (guix scripts substitute).

Both seem to work for me.

> (The zstd patches will conflict with this series but I’ll take care of
> it once it’s applied.)

Sounds good.

[signature.asc (application/pgp-signature, inline)]

Previous Next

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