[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#70494] [PATCH 04/23] guix: store: environment: New module.
From: |
Ludovic Courtès |
Subject: |
[bug#70494] [PATCH 04/23] guix: store: environment: New module. |
Date: |
Mon, 13 May 2024 17:10:06 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi Chris,
Christopher Baines <mail@cbaines.net> skribis:
> From: Caleb Ristvedt <caleb.ristvedt@cune.org>
>
> * guix/store/environment.scm: New file.
> * guix/store.scm: Export compressed-hash.
> * guix/store/database.scm (output-path-id-sql, outputs-exist?, references-sql,
> file-closure, all-input-output-paths, all-transitive-inputs): New variables.
> (outputs-exist?, file-closure, all-transitive-inputs): Export procedures.
> * Makefile.am (STORE_MODULES): Add guix/store/environment.scm.
>
> Co-authored-by: Christopher Baines <mail@cbaines.net>
> Change-Id: I71ac38fa8596a0c05b34880ca60e8a27ef3892d8
Very cool. Some comments:
> +++ b/guix/store.scm
> @@ -192,6 +192,7 @@ (define-module (guix store)
> grafting?
>
> %store-prefix
> + compressed-hash
> store-path
> output-path
> fixed-output-path
We can keep it this way for now.
However, the suggestion I made to reepca back then was that we should
move the low-level hashing/file name computation procedures to a
separate module, say (guix store file-names), such that daemon code does
not import (guix store).
(guix store) would only contain client-side code, possibly re-exporting
some of (guix store file-names) for compatibility and convenience.
> +(define* (file-closure db path #:key (list-so-far vlist-null))
> + "Return a vlist containing the store paths referenced by PATH, the store
> +paths referenced by those paths, and so on."
s/file-closure/store-item-closure/ ?
> +(define (all-input-output-paths drv)
> + "Return a list containing the output paths this derivation's inputs need to
> +provide."
> + (apply append (map derivation-input-output-paths
Use ‘append-map’ instead.
> + #:export (<environment>
Don’t export record type descriptors in general as this exposes the ABI.
> + environment-namespaces
> + environment-variables
> + environment-temp-dirs
s/temp-dirs/temporary-directories/
> + environment-filesystems
> + environment-new-session?
> + environment-new-pgroup?
> + environment-setup-i/o-proc
> + environment-preserved-fds
> + environment-chroot
> + environment-personality
> + environment-user
> + environment-group
> + environment-hostname
> + environment-domainname
I’d write “file-systems”, “host-name”, and “domain-name”, to be
consistent with the rest of the code base (we can keep “namespaces”
because that’s how Linux spells it.)
> + build-environment-vars
s/vars/variables/
> +(define-record-type* <environment> environment
We should keep in mind that maybe we’ll want to use that in ‘guix shell
-C’ eventually.
> +(define (delete-environment env)
> + "Delete all temporary directories used in ENV."
s/delete-environment/delete-temporary-directories/
> +(define* (temp-directory tmpdir name #:optional permissions user group)
> + "Create a temporary directory under TMPDIR with permissions PERMISSIONS if
> +specified, otherwise default permissions as specified by umask, and belonging
> +to user USER and group GROUP (defaulting to current user if not specified or
> +#f). Return the full filename of the form <tmpdir>/<name>-<number>."
s/temp-directory/create-temporary-directory/
This procedure missed the fix in commit
ec7fb669945bfb47c5e1fdf7de3a5d07f7002ccf (CVE-2021-27851). It’s fine to
implement it later but we should at least leave a big FIXME comment.
Somewhere we’ll also need the fix for CVE-2024-27297 (commits
ff1251de0bc327ec478fc66a562430fbf35aef42 and
8f4ffb3fae133bb21d7991e97c2f19a7108b1143).
> +(define* (dump-port port #:optional (target-port (current-output-port)))
Use the one from (guix build utils) instead.
Thanks,
Ludo’.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [bug#70494] [PATCH 04/23] guix: store: environment: New module.,
Ludovic Courtès <=