[bug#34122] [PATCH 0/3] Build channel modules in the corresponding Guix

[bug#34122] [PATCH 0/3] Build channel modules in the corresponding Guix

[Ludovic Courtès]

Hello!

Until now, we’d be building channel modules with the calling Guix
rather than with the Guix of the 'guix channel we’re targeting.
That led to breakage, for instance, when we added new dependencies
to Guix itself and the channel modules would thus not see them; see
commits 3c0e16391ed9a3e3e4611b940fb393c5f2ecea63 and
cb341c121919877ae6267a6460c0c17536d06eff.

These patches fix that.

Ludo’.

Ludovic Courtès (3):
  channels: Don't pull from the same channel more than once.
  inferior: 'gexp->derivation-in-inferior' honors EXP's load path.
  channels: Build channel modules in an inferior.

 guix/channels.scm  | 139 +++++++++++++++++++++++++--------------------
 guix/inferior.scm  |  13 ++++-
 tests/channels.scm |  88 +++++++++++++++++++++++++++-
 3 files changed, 173 insertions(+), 67 deletions(-)

-- 
2.20.1

[bug#34122] [PATCH 1/3] channels: Don't pull from the same channel more than once.

[Ludovic Courtès]

Previous 'channel-instance->manifest' would call
'latest-channel-derivation', which could trigger another round of
'latest-repository-commit' for no good reason.

* guix/channels.scm (resolve-dependencies): New procedure.
(channel-instance-derivations)[edges]: New variable.
[instance->derivation]: New procedure.
* tests/channels.scm (make-instance): Use 'checkout->channel-instance'
instead of 'channel-instance'.
("channel-instances->manifest"): New test.
---
 guix/channels.scm  | 64 ++++++++++++++++++++++++-----------
 tests/channels.scm | 84 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 126 insertions(+), 22 deletions(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index cd8a0131bd..b9ce2aa024 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -35,6 +35,7 @@
   #:autoload   (guix self) (whole-package make-config.scm)
   #:autoload   (guix inferior) (gexp->derivation-in-inferior) ;FIXME: circular dep
   #:use-module (ice-9 match)
+  #:use-module (ice-9 vlist)
   #:export (channel
             channel?
             channel-name
@@ -289,6 +290,34 @@ INSTANCE depends on."
                      #:commit (channel-instance-commit instance)
                      #:dependencies dependencies))
 
+(define (resolve-dependencies instances)
+  "Return a procedure that, given one of the elements of INSTANCES, returns
+list of instances it depends on."
+  (define channel-instance-name
+    (compose channel-name channel-instance-channel))
+
+  (define table                                   ;map a name to an instance
+    (fold (lambda (instance table)
+            (vhash-consq (channel-instance-name instance)
+                         instance table))
+          vlist-null
+          instances))
+
+  (define edges
+    (fold (lambda (instance edges)
+            (fold (lambda (channel edges)
+                    (let ((name (channel-name channel)))
+                      (match (vhash-assq name table)
+                        ((_ . target)
+                         (vhash-consq instance target edges)))))
+                  edges
+                  (channel-instance-dependencies instance)))
+          vlist-null
+          instances))
+
+  (lambda (instance)
+    (vhash-foldq* cons '() instance edges)))
+
 (define (channel-instance-derivations instances)
   "Return the list of derivations to build INSTANCES, in the same order as
 INSTANCES."
@@ -310,27 +339,22 @@ INSTANCES."
           (module-ref (resolve-interface '(gnu packages guile))
                       'guile-bytestructures)))
 
-  (mlet %store-monad ((core (build-channel-instance core-instance)))
-    (mapm %store-monad
-          (lambda (instance)
-            (if (eq? instance core-instance)
-                (return core)
-                (match (channel-instance-dependencies instance)
-                  (()
+  (define edges
+    (resolve-dependencies instances))
+
+  (define (instance->derivation instance)
+    (mcached (if (eq? instance core-instance)
+                 (build-channel-instance instance)
+                 (mlet %store-monad ((core (instance->derivation core-instance))
+                                     (deps (mapm %store-monad instance->derivation
+                                                 (edges instance))))
                    (build-channel-instance instance
-                                           (cons core dependencies)))
-                  (channels
-                   (mlet %store-monad ((dependencies-derivation
-                                        (latest-channel-derivation
-                                         ;; %default-channels is used here to
-                                         ;; ensure that the core channel is
-                                         ;; available for channels declared as
-                                         ;; dependencies.
-                                         (append channels %default-channels))))
-                     (build-channel-instance instance
-                                             (cons dependencies-derivation
-                                                   (cons core dependencies))))))))
-          instances)))
+                                           (cons core
+                                                 (append deps
+                                                         dependencies)))))
+             instance))
+
+  (mapm %store-monad instance->derivation instances))
 
 (define (whole-package-for-legacy name modules)
   "Return a full-blown Guix package for MODULES, a derivation that builds Guix
diff --git a/tests/channels.scm b/tests/channels.scm
index f3fc383ac3..7df1b8c5fe 100644
--- a/tests/channels.scm
+++ b/tests/channels.scm
@@ -18,9 +18,15 @@
 
 (define-module (test-channels)
   #:use-module (guix channels)
+  #:use-module (guix profiles)
   #:use-module ((guix build syscalls) #:select (mkdtemp!))
   #:use-module (guix tests)
+  #:use-module (guix store)
+  #:use-module ((guix grafts) #:select (%graft?))
+  #:use-module (guix derivations)
+  #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-64)
   #:use-module (ice-9 match))
 
@@ -34,8 +40,9 @@
   (and spec
        (with-output-to-file (string-append instance-dir "/.guix-channel")
          (lambda _ (format #t "~a" spec))))
-  ((@@ (guix channels) channel-instance)
-   name commit instance-dir))
+  (checkout->channel-instance instance-dir
+                              #:commit commit
+                              #:name name))
 
 (define instance--boring (make-instance))
 (define instance--no-deps
@@ -136,4 +143,77 @@
                                    'abc1234)))
                        instances))))))
 
+(test-assert "channel-instances->manifest"
+  ;; Compute the manifest for a graph of instances and make sure we get a
+  ;; derivation graph that mirrors the instance graph.  This test also ensures
+  ;; we don't try to access Git repositores at all at this stage.
+  (let* ((spec      (lambda deps
+                      `(channel (version 0)
+                                (dependencies
+                                 ,@(map (lambda (dep)
+                                          `(channel
+                                            (name ,dep)
+                                            (url "http://example.org")))
+                                        deps)))))
+         (guix      (make-instance #:name 'guix))
+         (instance0 (make-instance #:name 'a))
+         (instance1 (make-instance #:name 'b #:spec (spec 'a)))
+         (instance2 (make-instance #:name 'c #:spec (spec 'b)))
+         (instance3 (make-instance #:name 'd #:spec (spec 'c 'a))))
+    (%graft? #f)                                    ;don't try to build stuff
+
+    ;; Create 'build-self.scm' so that GUIX is recognized as the 'guix' channel.
+    (let ((source (channel-instance-checkout guix)))
+      (mkdir (string-append source "/build-aux"))
+      (call-with-output-file (string-append source
+                                            "/build-aux/build-self.scm")
+        (lambda (port)
+          (write '(begin
+                    (use-modules (guix) (gnu packages bootstrap))
+
+                    (lambda _
+                      (package->derivation %bootstrap-guile)))
+                 port))))
+
+    (with-store store
+      (let ()
+        (define manifest
+          (run-with-store store
+            (channel-instances->manifest (list guix
+                                               instance0 instance1
+                                               instance2 instance3))))
+
+        (define entries
+          (manifest-entries manifest))
+
+        (define (depends? drv in out)
+          ;; Return true if DRV depends on all of IN and none of OUT.
+          (let ((lst (map derivation-input-path (derivation-inputs drv)))
+                (in  (map derivation-file-name in))
+                (out (map derivation-file-name out)))
+            (and (every (cut member <> lst) in)
+                 (not (any (cut member <> lst) out)))))
+
+        (define (lookup name)
+          (run-with-store store
+            (lower-object
+             (manifest-entry-item
+              (manifest-lookup manifest
+                               (manifest-pattern (name name)))))))
+
+        (let ((drv-guix (lookup "guix"))
+              (drv0     (lookup "a"))
+              (drv1     (lookup "b"))
+              (drv2     (lookup "c"))
+              (drv3     (lookup "d")))
+          (and (depends? drv-guix '() (list drv0 drv1 drv2 drv3))
+               (depends? drv0
+                         (list) (list drv1 drv2 drv3))
+               (depends? drv1
+                         (list drv0) (list drv2 drv3))
+               (depends? drv2
+                         (list drv1) (list drv0 drv3))
+               (depends? drv3
+                         (list drv2 drv0) (list drv1))))))))
+
 (test-end "channels")
-- 
2.20.1

[bug#34122] [PATCH 2/3] inferior: 'gexp->derivation-in-inferior' honors EXP's load path.

[Ludovic Courtès]

Previously the imported modules and extensions of EXP would be missing
from the load path of 'guix repl'.

* guix/inferior.scm (gexp->derivation-in-inferior)[script]: New
variable.
[trampoline]: Write (primitive-load #$script) to PIPE.  Add #$output.
* tests/channels.scm ("channel-instances->manifest")[depends?]: Check
for requisites rather than direct references.
Adjust callers accordingly.
---
 guix/inferior.scm  | 13 ++++++++++---
 tests/channels.scm | 16 ++++++++++------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/guix/inferior.scm b/guix/inferior.scm
index 4dfb242e44..9f19e7d316 100644
--- a/guix/inferior.scm
+++ b/guix/inferior.scm
@@ -491,6 +491,10 @@ PACKAGE must be live."
   "Return a derivation that evaluates EXP with GUIX, an instance of Guix as
 returned for example by 'channel-instances->derivation'.  Other arguments are
 passed as-is to 'gexp->derivation'."
+  (define script
+    ;; EXP wrapped with a proper (set! %load-path …) prologue.
+    (scheme-file "inferior-script.scm" exp))
+
   (define trampoline
     ;; This is a crude way to run EXP on GUIX.  TODO: use 'raw-derivation' and
     ;; make 'guix repl' the "builder"; this will require "opening up" the
@@ -501,9 +505,12 @@ passed as-is to 'gexp->derivation'."
         (let ((pipe (open-pipe* OPEN_WRITE
                                 #+(file-append guix "/bin/guix")
                                 "repl" "-t" "machine")))
-          ;; Unquote EXP right here so that its references to #$output
-          ;; propagate to the surrounding gexp.
-          (write '#$exp pipe)                     ;XXX: load path for EXP?
+
+          ;; XXX: EXP presumably refers to #$output but that reference is lost
+          ;; so explicitly reference it here.
+          #$output
+
+          (write `(primitive-load #$script) pipe)
 
           (unless (zero? (close-pipe pipe))
             (error "inferior failed" #+guix)))))
diff --git a/tests/channels.scm b/tests/channels.scm
index 7df1b8c5fe..8540aef435 100644
--- a/tests/channels.scm
+++ b/tests/channels.scm
@@ -24,6 +24,7 @@
   #:use-module (guix store)
   #:use-module ((guix grafts) #:select (%graft?))
   #:use-module (guix derivations)
+  #:use-module (guix sets)
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
@@ -187,12 +188,15 @@
           (manifest-entries manifest))
 
         (define (depends? drv in out)
-          ;; Return true if DRV depends on all of IN and none of OUT.
-          (let ((lst (map derivation-input-path (derivation-inputs drv)))
+          ;; Return true if DRV depends (directly or indirectly) on all of IN
+          ;; and none of OUT.
+          (let ((set (list->set
+                      (requisites store
+                                  (list (derivation-file-name drv)))))
                 (in  (map derivation-file-name in))
                 (out (map derivation-file-name out)))
-            (and (every (cut member <> lst) in)
-                 (not (any (cut member <> lst) out)))))
+            (and (every (cut set-contains? set <>) in)
+                 (not (any (cut set-contains? set <>) out)))))
 
         (define (lookup name)
           (run-with-store store
@@ -212,8 +216,8 @@
                (depends? drv1
                          (list drv0) (list drv2 drv3))
                (depends? drv2
-                         (list drv1) (list drv0 drv3))
+                         (list drv1) (list drv3))
                (depends? drv3
-                         (list drv2 drv0) (list drv1))))))))
+                         (list drv2 drv0) (list))))))))
 
 (test-end "channels")
-- 
2.20.1

[bug#34122] [PATCH 3/3] channels: Build channel modules in an inferior.

[Ludovic Courtès]

This ensures that channel modules are compiled with the right Guile,
that they get to see the right modules, and so on.  IOW, it avoids bugs
such as those addressed by commits
3c0e16391ed9a3e3e4611b940fb393c5f2ecea63 and
cb341c121919877ae6267a6460c0c17536d06eff.

* guix/channels.scm (standard-module-derivation): Add 'core'
parameter.  Rewrite in terms of 'gexp->derivation-in-inferior'.
(build-from-source): Add #:core parameter and pass it to
'standard-module-derivation'.
(build-channel-instance): Add 'core' parameter and pass it on.
(channel-instance-derivations)[dependencies]: Remove.
Adjust 'build-channel-instance' call.
---
 guix/channels.scm | 83 +++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 46 deletions(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index b9ce2aa024..eb56c821e5 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -218,45 +218,48 @@ of COMMIT at URL.  Use NAME as the channel name."
   ;; place a set of compiled Guile modules in ~/.config/guix/latest.
   1)
 
-(define (standard-module-derivation name source dependencies)
-  "Return a derivation that builds the Scheme modules in SOURCE and that
-depend on DEPENDENCIES, a list of lowerable objects.  The assumption is that
-SOURCE contains package modules to be added to '%package-module-path'."
-  (define modules
-    (scheme-modules* source))
-
+(define (standard-module-derivation name source core dependencies)
+  "Return a derivation that builds with CORE, a Guix instance, the Scheme
+modules in SOURCE and that depend on DEPENDENCIES, a list of lowerable
+objects.  The assumption is that SOURCE contains package modules to be added
+to '%package-module-path'."
   ;; FIXME: We should load, say SOURCE/.guix-channel.scm, which would allow
   ;; channel publishers to specify things such as the sub-directory where .scm
   ;; files live, files to exclude from the channel, preferred substitute URLs,
   ;; etc.
-  (mlet* %store-monad ((compiled
-                        (compiled-modules modules
-                                          #:name name
-                                          #:module-path (list source)
-                                          #:extensions dependencies)))
 
-    (gexp->derivation name
-                      (with-extensions dependencies
-                        (with-imported-modules '((guix build utils))
-                          #~(begin
-                              (use-modules (guix build utils))
+  (define build
+    ;; This is code that we'll run in CORE, a Guix instance, with its own
+    ;; modules and so on.  That way, we make sure these modules are built for
+    ;; the right Guile version, with the right dependencies, and that they get
+    ;; to see the right (gnu packages …) modules.
+    (with-extensions dependencies
+      #~(begin
+          (use-modules (guix build compile)
+                       (guix build utils)
+                       (srfi srfi-26))
 
-                              (let ((go  (string-append #$output "/lib/guile/"
-                                                        (effective-version)
-                                                        "/site-ccache"))
-                                    (scm (string-append #$output
-                                                        "/share/guile/site/"
-                                                        (effective-version))))
-                                (mkdir-p (dirname go))
-                                (symlink #$compiled go)
-                                (mkdir-p (dirname scm))
-                                (symlink #$source scm))))))))
+          (define go
+            (string-append #$output "/lib/guile/" (effective-version)
+                           "/site-ccache"))
+          (define scm
+            (string-append #$output "/share/guile/site/"
+                           (effective-version)))
+
+          (compile-files #$source go
+                         (find-files #$source "\\.scm$"))
+          (mkdir-p (dirname scm))
+          (symlink #$source scm)
+          scm)))
+
+  (gexp->derivation-in-inferior name build core))
 
 (define* (build-from-source name source
-                            #:key verbose? commit
+                            #:key core verbose? commit
                             (dependencies '()))
   "Return a derivation to build Guix from SOURCE, using the self-build script
-contained therein.  Use COMMIT as the version string."
+contained therein; use COMMIT as the version string.  When CORE is true, build
+package modules under SOURCE using CORE, an instance of Guix."
   ;; Running the self-build script makes it easier to update the build
   ;; procedure: the self-build script of the Guix-to-be-installed contains the
   ;; right dependencies, build procedure, etc., which the Guix-in-use may not
@@ -278,9 +281,10 @@ contained therein.  Use COMMIT as the version string."
                #:pull-version %pull-version))
 
       ;; Build a set of modules that extend Guix using the standard method.
-      (standard-module-derivation name source dependencies)))
+      (standard-module-derivation name source core dependencies)))
 
-(define* (build-channel-instance instance #:optional (dependencies '()))
+(define* (build-channel-instance instance
+                                 #:optional core (dependencies '()))
   "Return, as a monadic value, the derivation for INSTANCE, a channel
 instance.  DEPENDENCIES is a list of extensions providing Guile modules that
 INSTANCE depends on."
@@ -288,6 +292,7 @@ INSTANCE depends on."
                       (channel-name (channel-instance-channel instance)))
                      (channel-instance-checkout instance)
                      #:commit (channel-instance-commit instance)
+                     #:core core
                      #:dependencies dependencies))
 
 (define (resolve-dependencies instances)
@@ -328,17 +333,6 @@ INSTANCES."
             (guix-channel? (channel-instance-channel instance)))
           instances))
 
-  (define dependencies
-    ;; Dependencies of CORE-INSTANCE.
-    ;; FIXME: It would be best not to hard-wire this information here and
-    ;; instead query it to CORE-INSTANCE.
-    (list (module-ref (resolve-interface '(gnu packages gnupg))
-                      'guile-gcrypt)
-          (module-ref (resolve-interface '(gnu packages guile))
-                      'guile-git)
-          (module-ref (resolve-interface '(gnu packages guile))
-                      'guile-bytestructures)))
-
   (define edges
     (resolve-dependencies instances))
 
@@ -348,10 +342,7 @@ INSTANCES."
                  (mlet %store-monad ((core (instance->derivation core-instance))
                                      (deps (mapm %store-monad instance->derivation
                                                  (edges instance))))
-                   (build-channel-instance instance
-                                           (cons core
-                                                 (append deps
-                                                         dependencies)))))
+                   (build-channel-instance instance core deps)))
              instance))
 
   (mapm %store-monad instance->derivation instances))
-- 
2.20.1

bug#34122: [PATCH 0/3] Build channel modules in the corresponding Guix

[Ludovic Courtès]

Ludovic Courtès <ludo@gnu.org> skribis:

>   channels: Don't pull from the same channel more than once.
>   inferior: 'gexp->derivation-in-inferior' honors EXP's load path.
>   channels: Build channel modules in an inferior.

Pushed!

  acefa7408b channels: Build channel modules in an inferior.
  1fafc383b1 inferior: 'gexp->derivation-in-inferior' honors EXP's load path.
  ed75bdf35c channels: Don't pull from the same channel more than once.

Ludo’.