“sudo guix system reconfigure” triggers re-clone/update of Git checkout

  • Open
  • quality assurance status badge
Details
3 participants
  • Jorge Acereda
  • Ludovic Courtès
  • Maxime Devos
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important

Debbugs page

L
L
Ludovic Courtès wrote on 17 Dec 2020 06:01
“sudo guix system reconfigure ” triggers re-clone/update of Git checkout
(address . bug-guix@gnu.org)
87mtycila0.fsf@inria.fr
Hi!

If you do, as a regular user:

guix pull
sudo guix system reconfigure …

the ‘guix system reconfigure’, as part of the downgrade-detection
machinery, triggers an update of the channel checkout(s) in
~root/.cache, even though ~USER/.cache is already up-to-date.

One way to avoid it might be to special-case the checkout cache
directory for when ‘SUDO_USER’ is set.

Thoughts?

Ludo’.
L
L
Ludovic Courtès wrote on 23 Dec 2020 15:15
control message for bug #45295
(address . control@debbugs.gnu.org)
87sg7wun9w.fsf@gnu.org
severity 45295 important
quit
L
L
Ludovic Courtès wrote on 17 Jan 2021 14:06
Re: bug#45295: “sudo guix system re configure” triggers re-clone/update of Git checkout
(address . 45295@debbugs.gnu.org)
87bldnusks.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (12 lines)
> If you do, as a regular user:
>
> guix pull
> sudo guix system reconfigure …
>
> the ‘guix system reconfigure’, as part of the downgrade-detection
> machinery, triggers an update of the channel checkout(s) in
> ~root/.cache, even though ~USER/.cache is already up-to-date.
>
> One way to avoid it might be to special-case the checkout cache
> directory for when ‘SUDO_USER’ is set.

Attached is a prototype that first clones/fetches from ~USER/.cache into
~root/.cache, in the hope that this avoids the need to access the
upstream repo. (It requires ‘set-remote-url!’, which is only in
Guile-Git ‘master’.)

It’s a bit hacky but I can’t think of a better way to address this
issue. In particular, having root use ~USER/.cache directly is not an
option: it could end up creating root-owned files there.

Thoughts?

Ludo’.
Toggle diff (82 lines)
diff --git a/guix/git.scm b/guix/git.scm
index a5103547d3..467d199e37 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -346,10 +346,7 @@ definitely available in REPOSITORY, false otherwise."
(check-out? #t)
starting-commit
(log-port (%make-void-port "w"))
- (cache-directory
- (url-cache-directory
- url (%repository-cache-directory)
- #:recursive? recursive?)))
+ (cache-directory *unspecified*))
"Update the cached checkout of URL to REF in CACHE-DIRECTORY. Return three
values: the cache directory name, and the SHA1 commit (a string) corresponding
to REF, and the relation of the new commit relative to STARTING-COMMIT (if
@@ -381,12 +378,41 @@ it unchanged."
(string-append "origin/" branch))))
(_ ref)))
+ (define default-cache-directory
+ (url-cache-directory url (%repository-cache-directory)
+ #:recursive? recursive?))
+
+ (when (and (zero? (getuid)) (getenv "SUDO_USER")
+ (unspecified? cache-directory))
+ ;; Fetch from the sudoer's cache before attempting to reach URL.
+ (let* ((home (and=> (false-if-exception (getpwnam (getenv "SUDO_USER")))
+ passwd:dir))
+ (peer (and home (url-cache-directory
+ url (string-append home "/.cache/guix/checkouts")
+ #:recursive? recursive?))))
+ (when (and peer (file-exists? peer))
+ ;; Fetch from PEER. After that, the "origin" remote points to PEER,
+ ;; but we change it back to URL below.
+ (update-cached-checkout (pk 'update peer)
+ #:ref ref
+ #:recursive? recursive?
+ #:check-out? #f
+ #:cache-directory
+ default-cache-directory))))
+
(with-libgit2
- (let* ((cache-exists? (openable-repository? cache-directory))
- (repository (if cache-exists?
- (repository-open cache-directory)
- (clone* url cache-directory))))
+ (let* ((cache-directory (if (unspecified? cache-directory)
+ default-cache-directory
+ cache-directory))
+ (cache-exists? (openable-repository? cache-directory))
+ (repository (if cache-exists?
+ (repository-open cache-directory)
+ (clone* url cache-directory))))
+ ;; Ensure the "origin" remote points to URL.
+ (set-remote-url! repository "origin" url)
+
;; Only fetch remote if it has not been cloned just before.
+ (pk 'x cache-directory 'avail? (reference-available? repository ref))
(when (and cache-exists?
(not (reference-available? repository ref)))
(let ((auth-method (%make-auth-ssh-agent)))
@@ -433,8 +459,6 @@ it unchanged."
#:key
recursive?
(log-port (%make-void-port "w"))
- (cache-directory
- (%repository-cache-directory))
(ref '(branch . "master")))
"Return two values: the content of the git repository at URL copied into a
store directory and the sha1 of the top level commit in this directory. The
@@ -464,10 +488,6 @@ Log progress and checkout info to LOG-PORT."
(update-cached-checkout url
#:recursive? recursive?
#:ref ref
- #:cache-directory
- (url-cache-directory url cache-directory
- #:recursive?
- recursive?)
#:log-port log-port))
((name)
(url+commit->name url commit)))
J
J
Jorge Acereda wrote on 9 Jan 2022 11:55
Alternative
(address . 45295@debbugs.gnu.org)
87r19gy980.fsf@gmail.com
Hi,

New user here, so maybe I'm talking BS.

I'm wondering if getting rid of sudo for reconfiguration is an option.

What if instead of running all the process as root, it invoked sudo (or
doas) in the final stage, so it can perform the bits that require
permissions?

That way, it would use the user channel directly and this issue would
not exist.

Regards,
Jorge
M
M
Maxime Devos wrote on 9 Jan 2022 12:17
cf26786af911b594627d07b0467164e7d6368ee1.camel@telenet.be
Jorge Acereda schreef op zo 09-01-2022 om 20:55 [+0100]:
Toggle quote (10 lines)
> Hi,
>
> New user here, so maybe I'm talking BS.
>
> I'm wondering if getting rid of sudo for reconfiguration is an option.
>
> What if instead of running all the process as root, it invoked sudo (or
> doas) in the final stage, so it can perform the bits that require
> permissions?

A problem here is that this assumes sudo, so "guix system reconfigure"
needs to guess whether to use "su", "sudo", "sudo -E", "doas", ...

Looking at guix/scripts/system.scm, it appears that
"guix system reconfigure" interacts with shepherd directly,
so "guix system reconfigure" needs to be run as root to work;
at least currently it cannot delegate this to a separate process
to be run under "sudo" or the like.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYdtC7RccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7mF6AQCFx/jqBi75yR2LpdVBQiOqNhEN
qDrSIHy8Kblp9Q2yVQD9HVl1T+Uc4ZvgHhNFNczmehQZuhlPNmIFAj/TDlBddQk=
=Rkz0
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 9 Jan 2022 12:19
26055d1f5d06f05f96f04a6cec8a6909cd6e5fce.camel@telenet.be
Jorge Acereda schreef op zo 09-01-2022 om 20:55 [+0100]:
Toggle quote (10 lines)
> Hi,
>
> New user here, so maybe I'm talking BS.
>
> I'm wondering if getting rid of sudo for reconfiguration is an option.
>
> What if instead of running all the process as root, it invoked sudo (or
> doas) in the final stage, so it can perform the bits that require
> permissions?

A problem here is that this assumes sudo, so "guix system reconfigure"
needs to guess whether to use "su", "sudo", "sudo -E", "doas", ...

Looking at guix/scripts/system.scm, it appears that
"guix system reconfigure" interacts with shepherd directly,
so "guix system reconfigure" needs to be run as root to work;
at least currently it cannot delegate this to a separate process
to be run under "sudo" or the like.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYdtDYhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7m9ZAP48/6ma8QDzW0dm9w4czoiX838T
pkT0bTRpqTN5VInfUwEApPHleiKSRQRsL0IjcmseWs3UHOfF5v5ciNO3gQNYLAs=
=f2av
-----END PGP SIGNATURE-----


?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 45295@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 45295
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch