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

Previous Next

Package: guix-patches;

Reported by: Christopher Baines <mail <at> cbaines.net>

Date: Thu, 24 Dec 2020 17:19:02 UTC

Severity: normal

Tags: patch

Done: Christopher Baines <mail <at> cbaines.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Christopher Baines <mail <at> cbaines.net>
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 16:03:07 +0100
Hi!

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.

> +(define-module (guix narinfo)
> +  #:use-module (guix ui)

We should try and avoid (guix ui); is (guix diagnostics) enough?

> +  #: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).

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

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

Thanks,
Ludo’.




This bug report was last modified 4 years and 134 days ago.

Previous Next


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