| Message ID | 8dda4413505b28fedb9588a4064812fe69c19a37.camel@telenet.be |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#45991,core-updates] Move 'mkdir-p/perms' to gnu/build/utils.scm | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/submitting builds | success | |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
I forgot to mention some atomicity issues the current mkdir-p/perms has. Excerpt from IRC: (15:17:25) mdevos: I see ‘mkdir-p/perms’ doesn't create the directory and set the permissions atomically; there's a tiny window where a freshly-created directory has the permissions that would be expected from the umask. Is this something to be concerned about (and to be fixed in the patch)? (15:40:46) civodul: mdevos: it's a good idea to be concerned about this, yes :-) (15:41:27) civodul: in general, given that changes in (guix build utils) take time to trickle in, we should be extra cautious about interfaces and implementation details This patch doesn't address these potential issues. Also, %dovecot-activation has an anomalous mkdir-p/perms.
Hi Maxime, Maxime Devos <maximedevos@telenet.be> skribis: > This is the patch I talked about on IRC. It moves the various inline > definitions of 'mkdir-p/perms' from gnu/services/... to gnu/build/utils.scm. > I've also written a few tests. As this change entails a world rebuild, > this should be applied to core-updates instead of master (as civodul > pointed out). Since (gnu build activation) now has a variant of ‘mkdir-p/perms’ that verifies that directory components are not symlinks, should we still include this one in (guix build utils)? Thanks for all the parens! :-) Ludo’.
Hi, Ludovic Courtès <ludo@gnu.org> writes: > Hi Maxime, > > Maxime Devos <maximedevos@telenet.be> skribis: > >> This is the patch I talked about on IRC. It moves the various inline >> definitions of 'mkdir-p/perms' from gnu/services/... to gnu/build/utils.scm. >> I've also written a few tests. As this change entails a world rebuild, >> this should be applied to core-updates instead of master (as civodul >> pointed out). > > Since (gnu build activation) now has a variant of ‘mkdir-p/perms’ that > verifies that directory components are not symlinks, should we still > include this one in (guix build utils)? > > Thanks for all the parens! :-) I guess (gnu build activation) is an appropriate home, given configuring permissions is not typically useful for packaging purposes.
From 6ebc5f9e390af1d2efaee1c6640724b358434029 Mon Sep 17 00:00:00 2001 From: Maxime Devos <maximedevos@telenet.be> Date: Tue, 19 Jan 2021 19:19:54 +0100 Subject: [PATCH 2/2] gnu: remove inline 'mkdir-p/perms' definitions * gnu/services/mail.scm (%dovecot-activation): Leave this anomalous definition for someone else to figure out. * gnu/services/dns.scm (%knot-activation): Remove inline definition of 'mkdir-p/perms'. * gnu/services/cups.scm (%cups-activation): Likewise. --- gnu/services/cups.scm | 4 ---- gnu/services/dns.scm | 4 ---- gnu/services/mail.scm | 13 ++++++++----- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/gnu/services/cups.scm b/gnu/services/cups.scm index 17ed04e58b..5099bbe421 100644 --- a/gnu/services/cups.scm +++ b/gnu/services/cups.scm @@ -874,10 +874,6 @@ IPP specifications.") (with-imported-modules '((guix build utils)) #~(begin (use-modules (guix build utils)) - (define (mkdir-p/perms directory owner perms) - (mkdir-p directory) - (chown directory (passwd:uid owner) (passwd:gid owner)) - (chmod directory perms)) (define (build-subject parameters) (string-concatenate (map (lambda (pair) diff --git a/gnu/services/dns.scm b/gnu/services/dns.scm index b339eb0619..cf8e9dac7f 100644 --- a/gnu/services/dns.scm +++ b/gnu/services/dns.scm @@ -609,10 +609,6 @@ (define (knot-activation config) #~(begin (use-modules (guix build utils)) - (define (mkdir-p/perms directory owner perms) - (mkdir-p directory) - (chown directory (passwd:uid owner) (passwd:gid owner)) - (chmod directory perms)) (mkdir-p/perms #$(knot-configuration-run-directory config) (getpwnam "knot") #o755) (mkdir-p/perms "/var/lib/knot" (getpwnam "knot") #o755) diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm index c0f6371104..e17be3197c 100644 --- a/gnu/services/mail.scm +++ b/gnu/services/mail.scm @@ -1484,7 +1484,10 @@ greyed out, instead of only later giving \"not selectable\" popup error. dovecot-configuration-fields))))))) #~(begin (use-modules (guix build utils)) - (define (mkdir-p/perms directory owner perms) + ;; XXX someone please take a look + ;; if the hardcoding of /var/run/dovecot + ;; is intended, or a bug. idk + (define (mkdir-p/perms-xxx directory owner perms) (mkdir-p directory) (chown "/var/run/dovecot" (passwd:uid owner) (passwd:gid owner)) (chmod directory perms)) @@ -1529,12 +1532,12 @@ greyed out, instead of only later giving \"not selectable\" popup error. (format (current-error-port) "Failed to create public key at ~a.\n" public-key))))) (let ((user (getpwnam "dovecot"))) - (mkdir-p/perms "/var/run/dovecot" user #o755) - (mkdir-p/perms "/var/lib/dovecot" user #o755) - (mkdir-p/perms "/etc/dovecot" user #o755) + (mkdir-p/perms-xxx "/var/run/dovecot" user #o755) + (mkdir-p/perms-xxx "/var/lib/dovecot" user #o755) + (mkdir-p/perms-xxx "/etc/dovecot" user #o755) (copy-file #$(plain-file "dovecot.conf" config-str) "/etc/dovecot/dovecot.conf") - (mkdir-p/perms "/etc/dovecot/private" user #o700) + (mkdir-p/perms-xxx "/etc/dovecot/private" user #o700) (create-self-signed-certificate-if-absent #:private-key "/etc/dovecot/private/default.pem" #:public-key "/etc/dovecot/default.pem" -- 2.30.0