[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#45409] [PATCH 1/3] guix: Move narinfo code from substitute script t
From: |
Christopher Baines |
Subject: |
[bug#45409] [PATCH 1/3] guix: Move narinfo code from substitute script to module. |
Date: |
Sun, 03 Jan 2021 18:16:13 +0000 |
User-agent: |
mu4e 1.4.13; emacs 27.1 |
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.
signature.asc
Description: PGP signature