Subject: bug#41028: Channel/inferior error with core-updates: Unbound variable: call-with-new-thread

From: Marius Bakke <[email-masked]>
Date: Fri, 08 May 2020 16:35:16 +0200

Christopher Baines <[email-masked]> writes:

> Ludovic Courtès <[email-masked]> writes:
>
>> Ludovic Courtès <[email-masked]> skribis:
>>
>>> The attached patches add a mechanism to patch the Guix source tree, and
>>> then use that mechanism to add the missing (ice-9 threads) import.  With
>>> this I can do:
>>>
>>>   ./pre-inst-env guix time-machine \
>>>      --commit=e02c2f85b36ce1c733bd908a210ce1182bdd2560 -- build linux-libre
>>>
>>> … which is a simple way to do what the manifest above was about.
>>
>> Given the enthusiasm expressed on IRC, I went ahead and pushed.  :-)
>>
>>   ff3ca7979e channels: Add patch for <https://bugs.gnu.org/41028>.
>>   053b10c3ef channels: Add mechanism to patch checkouts of the 'guix channel.
>>   4ba425060a channels: Add 'latest-channel-instance'.
>>
>> So… it might be that today is merge day?
>
> Wonderful :) I've had a chance to try this out now, and it works. I was
> able to reconfigure my system.
>
> One even more niche issue is that because I'm using this channel in my
> system configuration, the patching happens as root, but it's the cached
> channel in my users home directory that's patched. This means that
> build-self.scm becomes owned by root.
>
> I noticed this when I went to pull:
>
>   → guix pull --branch=core-updates
>   Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
>   guix pull: error: Git error: could not open '/home/chris/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq/build-aux/build-self.scm' for writing: Permission denied
>
>
> I'm not sure what the neat way of addressing this is, but maybe the file
> ownership can be recorded prior to patching, and reset afterwards if
> it's changed.

I took a stab at exactly this:

From 993dd0d36ba8e67af5c60d73cb1f9d60741f5418 Mon Sep 17 00:00:00 2001
From: Marius Bakke <[email-masked]>
Date: Fri, 8 May 2020 16:23:55 +0200
Subject: [PATCH] channels: Preserve permissions when patching
 <https://bugs.gnu.org/41028>.

* guix/channels.scm (%bug-41028-patch): Record permissions before invoking
SUBSTITUTE* and reset afterwards if file permissions differ from the current user.
---
 guix/channels.scm | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index 0fa036446c..a102d5bc35 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -393,10 +393,21 @@ to '%package-module-path'."
            (not (string-contains content "(ice-9 threads)"))))
 
     (define (add-missing-ice-9-threads-import source)
-      ;; Add (ice-9 threads) import in the gexp of 'compute-guix-derivation'.
-      (substitute* (string-append source "/" %self-build-file)
-        (("^ +\\(use-modules \\(ice-9 match\\)\\)")
-         (object->string '(use-modules (ice-9 match) (ice-9 threads))))))
+      (let* ((self-build-file (string-append source "/" %self-build-file))
+             ;; Record permissions so that we can reset it afterwards in case
+             ;; we run this as the root user (see <https://bugs.gnu.org/41028#29>).
+             ;; TODO: Ideally SUBSTITUTE* would preserve permissions itself.
+             (stat  (stat self-build-file))
+             (owner (stat:uid stat))
+             (group (stat:gid stat)))
+
+        ;; Add (ice-9 threads) import in the gexp of 'compute-guix-derivation'.
+        (substitute* self-build-file
+          (("^ +\\(use-modules \\(ice-9 match\\)\\)")
+           (object->string '(use-modules (ice-9 match) (ice-9 threads)))))
+
+        (unless (and (eq? (getuid) owner) (eq? (getgid) group))
+          (chown self-build-file owner group))))
 
    (patch missing-ice-9-threads-import? add-missing-ice-9-threads-import)))
 
-- 
2.26.2

WDYT?

Currently in the process of testing it locally, feedback appreciated.