Today's Guile 3 'core-updates' cannot pull yesterday's 2.2 master

DoneSubmitted by Christopher Baines.
Details
4 participants
  • Ludovic Courtès
  • Christopher Baines
  • Marius Bakke
  • zimoun
Owner
unassigned
Severity
important
C
C
Christopher Baines wrote on 2 May 2020 17:47
Channel/inferior error with core-updates: Unbound variable: call-with-new-thread
(address . bug-guix@gnu.org)
87h7wymj8a.fsf@cbaines.net
Noticed this when testing guix system build with core-updates. Here's asmall example which reproduces the issue:

(use-modules (guix channels) (guix inferior))
(define channels (list (channel (name 'guix) (url "https://git.savannah.gnu.org/git/guix.git") (commit "e02c2f85b36ce1c733bd908a210ce1182bdd2560"))))
(define inferior (inferior-for-channels channels))
(first (lookup-inferior-packages inferior "linux-libre" "5.2.21"))

If you save that as a file then attempt to build it:

→ guix build -f test.scm Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...Backtrace: 4 (primitive-load "/gnu/store/8mv5bpjgxg9c369xnbb5rf1kv9r?")In ice-9/eval.scm: 619:8 3 (_ #(#(#(#(#(#(#(#(#(#(#(?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?)) 182:19 2 (proc #(#(#(#(#(#(#(#(#(#(# ?) ?) ?) ?) ?) ?) ?) ?) ?) ?)) 142:16 1 (compile-top-call #<directory (guile-user) 7f1e0abc1f00> ?)In unknown file: 0 (%resolve-variable (7 . call-with-new-thread) #<directo?>)
ERROR: In procedure %resolve-variable:Unbound variable: call-with-new-threadguix build: error: You found a bug: the program '/gnu/store/8mv5bpjgxg9c369xnbb5rf1kv9r6z5hw-compute-guix-derivation'failed to compute the derivation for Guix (version: "e02c2f85b36ce1c733bd908a210ce1182bdd2560"; system: "x86_64-linux";host version: "a8cb1e72ef351330d1521833c1b270dcc0da593f"; pull-version: 1).Please report it by email to <bug-guix@gnu.org>.
-----BEGIN PGP SIGNATURE-----
iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl6tliVfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNFODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE9XfE1g/9H69j+hNwEfbN/lS3gA5ErpkPvI85iRIyKwEDLKSu6PcjalvwsXdnSM4l9JC6kTFJiWpBGnXnYTiwLP8D/LGo6XBNyqVw0nbtsB8MQlR1DxMOo4aHroOMzAFvPOoSdVvVuDK+tNY/XePiI8fk67hZ7wWor83WUueL355d3vSVZvmyWQia1z2tx6DWk8Pb5rmCY+/KX3kxh7T2LTLUAZcnP78zUKiJVujnzYgci9++cmbsfe2svj0juwgg1Aqu+zKx+kfjMaHHEfak/a9DGwh0K13u+Qf80d5kIb8+lFkPCbZDcIcjT4AG2dQ3GZcXSCNnOjJQBH9jtCikvNgUOA5b47nwuAxSb9APVlTdSWwiJZk7JcnIBFhm8KozmPOFIILKirErKkPlBsDX/eiOnL/jkcNzfCpfcjMQn5lcVry6thXVwnW9992KnCLME1CuH/oo9emLLfmVDW4PrFBHma0zJ6cjw4mmP4t8P20JXuhPWMPVqMoTaxy2UGj61kJQ9tH+vBoI5KuOYFZkXzz4JMqPkUDJWyDmJx/XtSZBGpXutXz56+yNO/djK1N34H+D7/DUciigdznofDVwefqfSi7j3F3zJcrwz40H1eb8C8Wmn2WxPkDdMUjQt5G1bQxMQ27JEdTWzjE8VmxWdaZR81pl45WAg3DTVpBmNr8qYpnD6y0==AyHo-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote on 5 May 2020 21:34
control message for bug #41028
(address . control@debbugs.gnu.org)
87368ekwfj.fsf@gnu.org
severity 41028 importantquit
L
L
Ludovic Courtès wrote on 5 May 2020 23:24
Re: bug#41028: Channel/inferior error with core-updates: Unbound variable: call-with-new-thread
(name . Christopher Baines)(address . mail@cbaines.net)(address . 41028@debbugs.gnu.org)
87r1vyjcs9.fsf@gnu.org
Hey!
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (17 lines)> → guix build -f test.scm > Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...> Backtrace:> 4 (primitive-load "/gnu/store/8mv5bpjgxg9c369xnbb5rf1kv9r?")> In ice-9/eval.scm:> 619:8 3 (_ #(#(#(#(#(#(#(#(#(#(#(?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?))> 182:19 2 (proc #(#(#(#(#(#(#(#(#(#(# ?) ?) ?) ?) ?) ?) ?) ?) ?) ?))> 142:16 1 (compile-top-call #<directory (guile-user) 7f1e0abc1f00> ?)> In unknown file:> 0 (%resolve-variable (7 . call-with-new-thread) #<directo?>)>> ERROR: In procedure %resolve-variable:> Unbound variable: call-with-new-thread> guix build: error: You found a bug: the program '/gnu/store/8mv5bpjgxg9c369xnbb5rf1kv9r6z5hw-compute-guix-derivation'> failed to compute the derivation for Guix (version: "e02c2f85b36ce1c733bd908a210ce1182bdd2560"; system: "x86_64-linux";> host version: "a8cb1e72ef351330d1521833c1b270dcc0da593f"; pull-version: 1).
A summary of the IRC discussion and experiments:
1. The underlying problem is a missing (ice-9 threads) import in the ‘compute-guix-derivation’ script, fixed in 05e783871c2c69b402e088863d46f5be7915ac74.
2. The ‘%quirks’ mechanism in (guix channels) doesn’t work as is here because what we would need to change is the #:guile parameter passed to ‘gexp->script’ (the one defined in build-self.scm). Attached a patch to add a quirk but that doesn’t solve the problem.
Possible solutions include:
a. Changing the value returned by ‘default-guile’ as used by ‘gexp->script’.
b. Supporting the definition of quirks that patch the code.
Thanks,Ludo’.
Toggle diff (92 lines)diff --git a/guix/channels.scm b/guix/channels.scmindex 041fae2a9c..cbb0a97546 100644--- a/guix/channels.scm+++ b/guix/channels.scm@@ -328,16 +328,34 @@ to '%package-module-path'." #f (apply throw args))))) +(define (missing-ice-9-threads-import? source)+ "Return true of %SELF-BUILD-FILE is missing an (ice-9 threads) import as+described at <https://bugs.gnu.org/41028>."+ (define content+ (call-with-input-file (string-append source "/" %self-build-file)+ read-string))++ ;; The faulty code uses 'call-with-new-thread' without importing (ice-9+ ;; threads). However, the 'call-with-new-thread' binding is no longer+ ;; available in the default name space on Guile 3.0.+ (and (string-contains content "(call-with-new-thread")+ (not (string-contains content "(ice-9 threads)"))))+ (define (guile-2.2.4) (module-ref (resolve-interface '(gnu packages guile)) 'guile-2.2.4)) +(define (guile-2.2)+ (module-ref (resolve-interface '(gnu packages guile))+ 'guile-2.2))+ (define %quirks ;; List of predicate/package pairs. This allows us provide information ;; about specific Guile versions that old Guix revisions might need to use ;; just to be able to build and run the trampoline in %SELF-BUILD-FILE. See ;; <https://bugs.gnu.org/37506>- `((,syscalls-reexports-local-variables? . ,guile-2.2.4)))+ `((,syscalls-reexports-local-variables? . ,guile-2.2.4)+ (,missing-ice-9-threads-import? . ,guile-2.2))) (define* (guile-for-source source #:optional (quirks %quirks)) "Return the Guile package to use when building SOURCE or #f if the default@@ -372,32 +390,32 @@ package modules under SOURCE using CORE, an instance of Guix." (string-append source "/" %self-build-file)) (if (file-exists? script)- (let ((build (save-module-excursion- (lambda ()- ;; Disable deprecation warnings; it's OK for SCRIPT to- ;; use deprecated APIs and the user doesn't have to know- ;; about it.- (parameterize ((guix-warning-port- (%make-void-port "w")))- (primitive-load script)))))- (guile (guile-for-source source)))+ (mlet* %store-monad ((guile -> (guile-for-source source))+ (_ (mwhen guile+ (set-guile-for-build (pk 'G guile))))+ (build -> (save-module-excursion+ (lambda ()+ ;; Disable deprecation warnings; it's+ ;; OK for SCRIPT to use deprecated+ ;; APIs and the user doesn't have to+ ;; know about it.+ (parameterize ((guix-warning-port+ (%make-void-port "w")))+ (primitive-load script)))))) ;; BUILD must be a monadic procedure of at least one argument: the ;; source tree. ;; ;; Note: BUILD can return #f if it does not support %PULL-VERSION. In ;; the future we'll fall back to a previous version of the protocol ;; when that happens.- (mbegin %store-monad- (mwhen guile- (set-guile-for-build guile)) - ;; BUILD is usually quite costly. Install a "trivial" build handler- ;; so we don't bounce an outer build-accumulator handler that could- ;; cause us to redo half of the BUILD computation several times just- ;; to realize it gives the same result.- (with-trivial-build-handler- (build source #:verbose? verbose? #:version commit- #:pull-version %pull-version))))+ ;; BUILD is usually quite costly. Install a "trivial" build handler+ ;; so we don't bounce an outer build-accumulator handler that could+ ;; cause us to redo half of the BUILD computation several times just+ ;; to realize it gives the same result.+ (with-trivial-build-handler+ (build source #:verbose? verbose? #:version commit+ #:pull-version %pull-version))) ;; Build a set of modules that extend Guix using the standard method. (standard-module-derivation name source core dependencies)))
L
L
Ludovic Courtès wrote on 6 May 2020 23:42
(address . 41028@debbugs.gnu.org)
87a72kg2qd.fsf@gnu.org
Comrades,
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (33 lines)> (use-modules (guix channels)> (guix inferior))>> (define channels> (list (channel> (name 'guix)> (url "https://git.savannah.gnu.org/git/guix.git")> (commit> "e02c2f85b36ce1c733bd908a210ce1182bdd2560"))))>> (define inferior> (inferior-for-channels channels))>> (first (lookup-inferior-packages inferior "linux-libre" "5.2.21"))>>> If you save that as a file then attempt to build it:>>> → guix build -f test.scm > Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...> Backtrace:> 4 (primitive-load "/gnu/store/8mv5bpjgxg9c369xnbb5rf1kv9r?")> In ice-9/eval.scm:> 619:8 3 (_ #(#(#(#(#(#(#(#(#(#(#(?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?))> 182:19 2 (proc #(#(#(#(#(#(#(#(#(#(# ?) ?) ?) ?) ?) ?) ?) ?) ?) ?))> 142:16 1 (compile-top-call #<directory (guile-user) 7f1e0abc1f00> ?)> In unknown file:> 0 (%resolve-variable (7 . call-with-new-thread) #<directo?>)>> ERROR: In procedure %resolve-variable:> Unbound variable: call-with-new-thread
The attached patches add a mechanism to patch the Guix source tree, andthen use that mechanism to add the missing (ice-9 threads) import. Withthis 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.
If we think a bit long-term, I think the %quirks and %patches mechanismsare a necessary evil. The goal is for ‘build-self.scm’ to use as littleof the Guix API, as well as Guile APIs that are presumably here to stay.But as we’re already seeing, there are unavoidably incompatibilitiesthat arise here and there. The good news is that we know today how tomodify yesterday’s code to work well, or which Guile version to use toget it to run.
Thoughts?
Thanks,Ludo’.
From 4ec2c3b5527b2189b3f767a980f80dee3c6717ae Mon Sep 17 00:00:00 2001From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>Date: Wed, 6 May 2020 22:18:52 +0200Subject: [PATCH 1/3] channels: Add 'latest-channel-instance'.
* guix/channels.scm (latest-channel-instance): New procedure.(latest-channel-instances): Use it.--- guix/channels.scm | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-)
Toggle diff (52 lines)diff --git a/guix/channels.scm b/guix/channels.scmindex 041fae2a9c..4ffc366d6a 100644--- a/guix/channels.scm+++ b/guix/channels.scm@@ -199,6 +199,14 @@ description file or its default value." channel INSTANCE." (channel-metadata-dependencies (channel-instance-metadata instance))) +(define (latest-channel-instance store channel)+ "Return the latest channel instance for CHANNEL."+ (let-values (((checkout commit)+ (latest-repository-commit store (channel-url channel)+ #:ref (channel-reference+ channel))))+ (channel-instance channel commit checkout)))+ (define* (latest-channel-instances store channels #:optional (previous-channels '())) "Return a list of channel instances corresponding to the latest checkouts of CHANNELS and the channels on which they depend. PREVIOUS-CHANNELS is a list@@ -224,20 +232,16 @@ of previously processed channels." (G_ "Updating channel '~a' from Git repository at '~a'...~%") (channel-name channel) (channel-url channel))- (let-values (((checkout commit)- (latest-repository-commit store (channel-url channel)- #:ref (channel-reference- channel))))- (let ((instance (channel-instance channel commit checkout)))- (let-values (((new-instances new-channels)- (latest-channel-instances- store- (channel-instance-dependencies instance)- previous-channels)))- (values (append (cons channel new-channels)- previous-channels)- (append (cons instance new-instances)- instances))))))))+ (let ((instance (latest-channel-instance store channel)))+ (let-values (((new-instances new-channels)+ (latest-channel-instances+ store+ (channel-instance-dependencies instance)+ previous-channels)))+ (values (append (cons channel new-channels)+ previous-channels)+ (append (cons instance new-instances)+ instances))))))) previous-channels '() ;instances channels))-- 2.26.2
From 18f2efd43b824f87b3d3a5dbcba9e8c363798f5b Mon Sep 17 00:00:00 2001From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>Date: Wed, 6 May 2020 22:45:31 +0200Subject: [PATCH 2/3] channels: Add mechanism to patch checkouts of the 'guix channel.
* guix/channels.scm (<patch>): New record type.(apply-patches): New procedure.(latest-channel-instance)[dot-git?]: New procedure.Use 'update-cached-checkout' and 'add-to-store' instead of'latest-repository-commit'. Call 'apply-patches' when CHANNEL is the'guix channel.(%patches): New variable.* guix/git.scm (url+commit->name): Make public.* tests/channels.scm ("latest-channel-instances includes channel dependencies")("latest-channel-instances excludes duplicate channel dependencies"):Mock 'update-cached-checkout' instead of 'latest-repository-commit'.Wrap body in 'with-store' and pass the store to 'latest-channel-instances'.--- guix/channels.scm | 50 +++++++++++++++++++++++++++++++----- guix/git.scm | 1 + tests/channels.scm | 64 ++++++++++++++++++++++++---------------------- 3 files changed, 79 insertions(+), 36 deletions(-)
Toggle diff (172 lines)diff --git a/guix/channels.scm b/guix/channels.scmindex 4ffc366d6a..75b53c3a8e 100644--- a/guix/channels.scm+++ b/guix/channels.scm@@ -199,13 +199,45 @@ description file or its default value." channel INSTANCE." (channel-metadata-dependencies (channel-instance-metadata instance))) -(define (latest-channel-instance store channel)+;; Patch to apply to a source tree.+(define-record-type <patch>+ (patch predicate application)+ patch?+ (predicate patch-predicate) ;procedure+ (application patch-application)) ;procedure++(define (apply-patches checkout commit patches)+ "Apply the matching PATCHES to CHECKOUT, modifying files in place. The+result is unspecified."+ (let loop ((patches patches))+ (match patches+ (() #t)+ ((($ <patch> predicate modify) rest ...)+ ;; PREDICATE is passed COMMIT so that it can choose to only apply to+ ;; ancestors.+ (when (predicate checkout commit)+ (modify checkout))+ (loop rest)))))++(define* (latest-channel-instance store channel+ #:key (patches %patches)) "Return the latest channel instance for CHANNEL."+ (define (dot-git? file stat)+ (and (string=? (basename file) ".git")+ (eq? 'directory (stat:type stat))))+ (let-values (((checkout commit)- (latest-repository-commit store (channel-url channel)- #:ref (channel-reference- channel))))- (channel-instance channel commit checkout)))+ (update-cached-checkout (channel-url channel)+ #:ref (channel-reference channel))))+ (when (guix-channel? channel)+ ;; Apply the relevant subset of PATCHES directly in CHECKOUT. This is+ ;; safe to do because 'switch-to-ref' eventually does a hard reset.+ (apply-patches checkout commit patches))++ (let* ((name (url+commit->name (channel-url channel) commit))+ (checkout (add-to-store store name #t "sha256" checkout+ #:select? (negate dot-git?))))+ (channel-instance channel commit checkout)))) (define* (latest-channel-instances store channels #:optional (previous-channels '())) "Return a list of channel instances corresponding to the latest checkouts of@@ -337,12 +369,18 @@ to '%package-module-path'." 'guile-2.2.4)) (define %quirks- ;; List of predicate/package pairs. This allows us provide information+ ;; List of predicate/package pairs. This allows us to provide information ;; about specific Guile versions that old Guix revisions might need to use ;; just to be able to build and run the trampoline in %SELF-BUILD-FILE. See ;; <https://bugs.gnu.org/37506> `((,syscalls-reexports-local-variables? . ,guile-2.2.4))) +(define %patches+ ;; Bits of past Guix revisions can become incompatible with newer Guix and+ ;; Guile. This variable lists <patch> records for the Guix source tree that+ ;; apply to the Guix source.+ '())+ (define* (guile-for-source source #:optional (quirks %quirks)) "Return the Guile package to use when building SOURCE or #f if the default '%guile-for-build' should be good enough."diff --git a/guix/git.scm b/guix/git.scmindex 5fffd429bd..92121156cf 100644--- a/guix/git.scm+++ b/guix/git.scm@@ -40,6 +40,7 @@ with-repository update-cached-checkout+ url+commit->name latest-repository-commit commit-difference diff --git a/tests/channels.scm b/tests/channels.scmindex f5a7955483..910088ba15 100644--- a/tests/channels.scm+++ b/tests/channels.scm@@ -135,44 +135,48 @@ (name 'test) (url "test"))) (test-dir (channel-instance-checkout instance--simple)))- (mock ((guix git) latest-repository-commit- (lambda* (store url #:key ref)+ (mock ((guix git) update-cached-checkout+ (lambda* (url #:key ref) (match url- ("test" (values test-dir 'whatever))- (_ (values "/not-important" 'not-important)))))- (let ((instances (latest-channel-instances #f (list channel))))- (and (eq? 2 (length instances))- (lset= eq?- '(test test-channel)- (map (compose channel-name channel-instance-channel)- instances)))))))+ ("test" (values test-dir "caf3cabba9e"))+ (_ (values (channel-instance-checkout instance--no-deps)+ "abcde1234")))))+ (with-store store+ (let ((instances (latest-channel-instances store (list channel))))+ (and (eq? 2 (length instances))+ (lset= eq?+ '(test test-channel)+ (map (compose channel-name channel-instance-channel)+ instances)))))))) (test-assert "latest-channel-instances excludes duplicate channel dependencies" (let* ((channel (channel (name 'test) (url "test"))) (test-dir (channel-instance-checkout instance--with-dupes)))- (mock ((guix git) latest-repository-commit- (lambda* (store url #:key ref)+ (mock ((guix git) update-cached-checkout+ (lambda* (url #:key ref) (match url- ("test" (values test-dir 'whatever))- (_ (values "/not-important" 'not-important)))))- (let ((instances (latest-channel-instances #f (list channel))))- (and (= 2 (length instances))- (lset= eq?- '(test test-channel)- (map (compose channel-name channel-instance-channel)- instances))- ;; only the most specific channel dependency should remain,- ;; i.e. the one with a specified commit.- (find (lambda (instance)- (and (eq? (channel-name- (channel-instance-channel instance))- 'test-channel)- (string=? (channel-commit- (channel-instance-channel instance))- "abc1234")))- instances))))))+ ("test" (values test-dir "caf3cabba9e"))+ (_ (values (channel-instance-checkout instance--no-deps)+ "abcde1234")))))+ (with-store store+ (let ((instances (latest-channel-instances store (list channel))))+ (and (= 2 (length instances))+ (lset= eq?+ '(test test-channel)+ (map (compose channel-name channel-instance-channel)+ instances))+ ;; only the most specific channel dependency should remain,+ ;; i.e. the one with a specified commit.+ (find (lambda (instance)+ (and (eq? (channel-name+ (channel-instance-channel instance))+ 'test-channel)+ (string=? (channel-commit+ (channel-instance-channel instance))+ "abc1234")))+ instances))))))) (test-assert "channel-instances->manifest" ;; Compute the manifest for a graph of instances and make sure we get a-- 2.26.2
From 66e2ef6ae0958fa45e14c3528c767cd84e24a480 Mon Sep 17 00:00:00 2001From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>Date: Wed, 6 May 2020 23:17:47 +0200Subject: [PATCH 3/3] channels: Add patch for https://bugs.gnu.org/41028.
Without this patch, we couldn't jump from here to commits before05e783871c2c69b402e088863d46f5be7915ac74 because the'compute-guix-derivation' script would crash with an unbound-variableerror for 'call-with-new-thread'.
Fixes https://bugs.gnu.org/41028.Reported by Christopher Baines <mail@cbaines.net>.
* guix/channels.scm (%bug-41028-patch): New variable.(%patches): Add it.--- guix/channels.scm | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
Toggle diff (51 lines)diff --git a/guix/channels.scm b/guix/channels.scmindex 75b53c3a8e..0fa036446c 100644--- a/guix/channels.scm+++ b/guix/channels.scm@@ -38,6 +38,7 @@ #:select (source-properties->location &error-location &fix-hint))+ #:use-module ((guix build utils) #:select (substitute*)) #:use-module (srfi srfi-1) #:use-module (srfi srfi-2) #:use-module (srfi srfi-9)@@ -375,11 +376,35 @@ to '%package-module-path'." ;; <https://bugs.gnu.org/37506> `((,syscalls-reexports-local-variables? . ,guile-2.2.4))) ++(define %bug-41028-patch+ ;; Patch for <https://bugs.gnu.org/41028>. The faulty code is the+ ;; 'compute-guix-derivation' body, which uses 'call-with-new-thread' without+ ;; importing (ice-9 threads). However, the 'call-with-new-thread' binding+ ;; is no longer available in the default name space on Guile 3.0.+ (let ()+ (define (missing-ice-9-threads-import? source commit)+ ;; Return true if %SELF-BUILD-FILE is missing an (ice-9 threads) import.+ (define content+ (call-with-input-file (string-append source "/" %self-build-file)+ read-string))++ (and (string-contains content "(call-with-new-thread")+ (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))))))++ (patch missing-ice-9-threads-import? add-missing-ice-9-threads-import)))+ (define %patches ;; Bits of past Guix revisions can become incompatible with newer Guix and ;; Guile. This variable lists <patch> records for the Guix source tree that ;; apply to the Guix source.- '())+ (list %bug-41028-patch)) (define* (guile-for-source source #:optional (quirks %quirks)) "Return the Guile package to use when building SOURCE or #f if the default-- 2.26.2
L
L
Ludovic Courtès wrote on 6 May 2020 23:42
control message for bug #41028
(address . control@debbugs.gnu.org)
878si4g2ov.fsf@gnu.org
retitle 41028 Today's Guile 3 'core-updates' cannot pull yesterday's 2.2 masterquit
L
L
Ludovic Courtès wrote on 7 May 2020 10:12
Re: bug#41028: Channel/inferior error with core-updates: Unbound variable: call-with-new-thread
(name . Christopher Baines)(address . mail@cbaines.net)
87k11oduzx.fsf@gnu.org
Hey!
Ludovic Courtès <ludo@gnu.org> skribis:
Toggle quote (9 lines)> 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?
Ludo’.
Closed
Z
Z
zimoun wrote on 7 May 2020 12:05
(address . 41028-done@debbugs.gnu.org)
CAJ3okZ3ATDRh=unpPVzVgWPbVBiZKeOnyJAR6=vvJXN1psfT7w@mail.gmail.com
Hi Ludo,
On Thu, 7 May 2020 at 10:13, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (6 lines)> 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'.
Just to be sure to well-understand: the versions before 05e783 werebroken (guix build failed) and the proposed in-place mechanism is away to fix them, right?If yes, that's awesome!

Cheers,simon
C
C
Christopher Baines wrote on 8 May 2020 11:37
(name . Ludovic Courtès)(address . ludo@gnu.org)
87sggag42e.fsf@cbaines.net
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (19 lines)> Ludovic Courtès <ludo@gnu.org> 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 wasable to reconfigure my system.
One even more niche issue is that because I'm using this channel in mysystem configuration, the patching happens as root, but it's the cachedchannel in my users home directory that's patched. This means thatbuild-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 fileownership can be recorded prior to patching, and reset afterwards ifit's changed.
Thanks again for looking at the original issue Ludo,
Chris
-----BEGIN PGP SIGNATURE-----
iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl61KGlfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNFODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE9XfF6g//UT+CFPaSPcsbzcvn2TBqy+h8tLmJHvvJ39ervySkffseCfIjTtYJ0Zqs8uzQePl3zeIaL/auRzZHV15+Rdg9pynqfxF2rMOJtiOCr7tYbBgiP2QiVUFNevkWwlRIYyYq+JXaPj3DMcbsmMcyOjXsFsJ4vRln3AExteJpR3ZnUEA83QHdNeXp65Z6BPpPLjLWc3Ne+bgSkfA/JmBNFd3EBRb7R3uPlt5Tz8uqoe28U5uBH0ayXjS7a0a05TY7FwBg5K3HSFLRa7aBl57QaHxGukDDTuRlYzXzCn7+V47PdCKQ3VnXN774NZrHXl13FJwTRF7J0N8Kx6t5HtJrKpnJ4ak57Pfqne2V0k0FlIQGC4IZtQRUbN2y0wKmPLv37R+5hDoxNmm+CRbHm4oaDZbY1QeySO9DvKDPelvKpCm/NkRqc/beRNAeg4Ui2+1V9BP5GWk2Pl3Rq2aVwZXnSPClzJ0OQLs6CXtdmsn2GHCM3611rJls6WJ48JrpG19Otpj18WJYCFN5u035tnReZyi1UddHnfIWyKZyrzMvajkp84utBwVj/inmxOXc2YllbhIWRFNWGBRbE6OmLdFnPxc5Ld96WHhc6Fjkak2XJHaNFqgC1QgbBineTEwnM4s1h0w46ZyjraUrHH6XuvR78ejMy11tjukOBy+hbpHb5QQ5Tg4==RanM-----END PGP SIGNATURE-----
Closed
M
M
Marius Bakke wrote on 8 May 2020 16:35
(address . 41028-done@debbugs.gnu.org)
875zd6336j.fsf@devup.no
Christopher Baines <mail@cbaines.net> writes:
Toggle quote (40 lines)> Ludovic Courtès <ludo@gnu.org> writes:>>> Ludovic Courtès <ludo@gnu.org> 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 2001From: Marius Bakke <mbakke@fastmail.com>Date: Fri, 8 May 2020 16:23:55 +0200Subject: [PATCH] channels: Preserve permissions when patching https://bugs.gnu.org/41028.
* guix/channels.scm (%bug-41028-patch): Record permissions before invokingSUBSTITUTE* and reset afterwards if file permissions differ from the current user.--- guix/channels.scm | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
Toggle diff (32 lines)diff --git a/guix/channels.scm b/guix/channels.scmindex 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.
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl61biQACgkQoqBt8qM6VPoFoAf/UKOIozWoTgGoPdH/k7NVKYeD1BJPMzS+1QtO9fwwnJf5bgtOIt0mPetigdgrgVYYg+Jk1WQRFMLEil5EdUgjMQiqP2OgpK/UwVmMMR3e8HESngn9jtB02hUq2sq6ZeALV4PL/gPIukObTt/BjluexVZpD5qaKcaRjC/JKLs87SM7zlMGY6Cqut5cvAOZYqjCRrggvlqwj4HIR51XavUHylIh26vQY2g9pecDrdCKkfCAGOg55+jZfogmypIrP2yUszIssZ4bUK21OIKE7+WmZtLhWyjrqCOkvDPJFbtvY4BXDzmuYXhISpdU09eE+aIhrd0hDQVEgt5BKw+bVVHFdg===xxKC-----END PGP SIGNATURE-----
Closed
C
C
Christopher Baines wrote on 8 May 2020 16:47
(name . Marius Bakke)(address . mbakke@fastmail.com)
87r1vufpqp.fsf@cbaines.net
Marius Bakke <mbakke@fastmail.com> writes:
Toggle quote (44 lines)> Christopher Baines <mail@cbaines.net> writes:>>> Ludovic Courtès <ludo@gnu.org> writes:>>>>> Ludovic Courtès <ludo@gnu.org> 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:
Awesome, thanks. I haven't tried it, but it looks good :)
-----BEGIN PGP SIGNATURE-----
iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl61cO5fFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNFODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE9XcezQ//Q+ajQjnUSufWkAbO2D/6mlicDbZW/Z3bLPjIywv8Hsb1tO145P/ZDT0hY5AmBhsWwpTeyZ3zKUNFk18tRUYeqIv9ZpYSn59HAF3pvynsFbX5M4cDD/Fxdx1cmqHECvw7V4WxUwIPNcXXH5KD6JErZZnzG0TMXcWyKHgRBan5cb1j6qVj+iWnD/zNDtHDp3PI82Op8jeAmNvr9WZ6SZhm/+bVBDj6wWgImqpm92MKgtX+eN/Xjtps8qaL6C4uqPv0WPfaFpKkCkxHOJgwCOTe31BmdJgbdBMivwnp8g9pxHtvmjXuqgnxHssF0n8rIf20tG81WBfRiNuAWvNRI+Qa0Wvr1+VPaJAFd0OzVxJp6nnY1RjMHay6xzZpUW9XIlOzAZJSozs5a42dOSiYcjGngG3msRu/2dE5vx5XWrGNXkF7908s7tptHNRc7rIVjZ7K6sprSBCJkP012btvVXf07GA7n4onnn0Efz+zG+nsyrCkCGC/ksTOu8MmQ0ls0InooL3PUYnkv+uF/lusFCQ+pLr36LCuU7EJTdNFzytDN1s3of3MFV+eOPTF4tmIR8eAn2lnIy4yISplMzBlPxOY7z88pRaqqmQrBuo9tMD851E0jVb3TbX3yxvMwQivc9A+zebq0p+ySctDsRslp63qoJnEQHulG//EtN86tHQ2Ci8==sKvg-----END PGP SIGNATURE-----
Closed
M
M
Marius Bakke wrote on 8 May 2020 20:19
(address . 41028-done@debbugs.gnu.org)
87368a2stk.fsf@devup.no
Marius Bakke <mbakke@fastmail.com> writes:
Toggle quote (62 lines)> Christopher Baines <mail@cbaines.net> writes:>>> 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 <mbakke@fastmail.com>> 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)))
Ludovic pointed out on IRC that 'sudo guix system build foo.scm' with aninferior will update the cached checkout (typically~/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq)as root, and break subsequent unprivileged 'guix pull' operations anyway.
Which is partially correct: .git/index and .git/refs/heads/master willbe owned by root in that case, but you are still allowed to unlink() andrename() such files as long as you own the directory, so libgit2 willsuccessfully reset permissions on the checkout even after pulling asroot!
It fails to do that for build-self.scm because it does not know it haschanged, and tries opening it directly with O_WRONLY (which fails). Ican confirm that this patch solves that problem, and unprivileged 'guixpull' works again even after previously using sudo.
Now, users are likely to run into other issues when using inferiors withsudo (e.g. if new directories are added), so I'm not sure if this patchis worth the effort.
Perhaps the Guix git cache should be per-uid, but that requires moreintrusive changes.
Thoughts?
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl61opcACgkQoqBt8qM6VPq/DQf/VpxtgyPipXlZiHtWs222a/Ojzh/A59h3wH7ZisDkxd4/bgkK4OpFeMQtM+tEXtAvlnzI5ViTSNKaFQI+TijT4PuHKApnJAYhELPoRCVpFk13GBphP5hdXUN4NadA8ZpK3IpTjbD3+Od/gxzqv43OYCwp8N1hEuxZFUkieE4gD/mmJ401ujUNdUHIXcb/f64vWt7YUevKEpxn8zVDILUGIQmREoxWsJmVBnGb4hJPzpvrjW/TnjIOBee1Lo8dFA4wMHRSYxTV+LK2/qhT+9L9Nq8hqIvKZwxeOSVMfBfnbg19WnX44IIDXhhmvaveKOeUgrmGwF9bNlTKMODuSq6huQ===d7Wa-----END PGP SIGNATURE-----
Closed
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send email to 41028@debbugs.gnu.org