Subject: Re: bug#25775: Attempts to fix bootstrap Guile bug

From: [email-masked] (Ludovic Courtès)
Date: Wed, 08 Mar 2017 00:02:39 +0100

Hello,

Thanks Andy & Ricardo for the detailed explanations!

Andy Wingo <[email-masked]> skribis:

> It seems that this bug is related to the introduction of
> url-fetch/reset-patch-level.  It takes a #:guile kwarg but defaults to
> #f; if not given #:guile, that #f propagates through instead of a
> package object.

Nasty.

To reproduce the problem reported here, one can:

  1. Revert the “band-aid commit”
     9f05908fb1e3707cae593d94688748294717a546.

  2. Change download.scm to force it to behave as when talking to an old
     daemon.

This gives this:

diff --git a/guix/download.scm b/guix/download.scm
index 86f859881..811abe27b 100644
--- a/guix/download.scm
+++ b/guix/download.scm
@@ -418,10 +418,7 @@ GnuTLS itself and its dependencies.  See <http://bugs.gnu.org/22774>."
                        ;; hash of the expected result.
                        #:verify-certificate? #f)))))
 
-  (mlet %store-monad ((guile (package->derivation
-                              (or guile
-                                  (@@ (gnu packages bootstrap) %bootstrap-guile))
-                              system)))
+  (mlet %store-monad ((guile (package->derivation guile system)))
     (gexp->derivation file-name builder
                       #:guile-for-build guile
                       #:system system
@@ -472,7 +469,7 @@ in the store."
             (and uri (memq (uri-scheme uri) '(#f file))))
         (interned-file (if uri (uri-path uri) url)
                        (or name file-name))
-        (mlet* %store-monad ((builtins (built-in-builders*))
+        (mlet* %store-monad ((builtins -> '())
                              (download -> (if (member "download" builtins)
                                               built-in-download
                                               in-band-download)))

Then run something like:

  guix gc -d /gnu/store/*-bash-4.4.tar.xz
  ./pre-inst-env guix build bash -S --no-substitutes

~~~~~~

To mirror what ‘url-fetch’ does, we should change the default value of
#:guile here:

diff --git a/gnu/packages/bash.scm b/gnu/packages/bash.scm
index c3b94391e..b4d0b6777 100644
--- a/gnu/packages/bash.scm
+++ b/gnu/packages/bash.scm
@@ -243,7 +243,8 @@ without modification.")
 
 (define* (url-fetch/reset-patch-level url hash-algo hash
                                       #:optional name
-                                      #:key (system (%current-system)) guile)
+                                      #:key (system (%current-system))
+                                      (guile (default-guile)))
   "Fetch the Bash patch from URL and reset its 'PATCHLEVEL' definition so it
 can apply to a patch-level 0 Bash."
   (mlet* %store-monad ((name -> (or name (basename url)))

However that leads to a stack overflow unless we patch
‘bootstrap-origin’ the way Andy suggests (which is not desirable IMO).

So, instead, we can simply force the use of the bootstrap Guile for
these derivations, which doesn’t make any difference functionally:

--- a/gnu/packages/bash.scm
+++ b/gnu/packages/bash.scm
@@ -21,6 +21,7 @@
 (define-module (gnu packages bash)
   #:use-module (guix licenses)
   #:use-module (gnu packages)
+  #:use-module (gnu packages bootstrap)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages readline)
   #:use-module (gnu packages bison)
@@ -243,14 +244,17 @@ without modification.")
 
 (define* (url-fetch/reset-patch-level url hash-algo hash
                                       #:optional name
-                                      #:key (system (%current-system)) guile)
+                                      #:key (system (%current-system)))
   "Fetch the Bash patch from URL and reset its 'PATCHLEVEL' definition so it
 can apply to a patch-level 0 Bash."
+  ;; Note: Forcefully use %BOOTSTRAP-GUILE here to work around bootstrapping
+  ;; issues when using a daemon that lacks the "download" built-in.  See
+  ;; <https://bugs.gnu.org/25775>.
   (mlet* %store-monad ((name -> (or name (basename url)))
                        (patch (url-fetch url hash-algo hash
                                          (string-append name ".orig")
                                          #:system system
-                                         #:guile guile)))
+                                         #:guile %bootstrap-guile)))
     (gexp->derivation name
                       (with-imported-modules '((guix build utils))
                         #~(begin
@@ -259,7 +263,6 @@ can apply to a patch-level 0 Bash."
                             (substitute* #$output
                               (("PATCHLEVEL [0-6]+")
                                "PATCHLEVEL 0"))))
-                      #:guile-for-build guile
                       #:system system)))
 
 (define bash/fixed                        ;CVE-2017-5932 (RCE with completion)

And it does the job.

Pushed as 6c5b56f9fa01b7fe9034bac47b20e08a2fdb2629.  Let me know if
there are still fishy things!

Ludo’.