[bug#45409] [PATCH 1/3] guix: Move narinfo code from substitute script to module.

[Christopher Baines <mail@cbaines.net>]

[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]


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

> Christopher Baines <mail@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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]