Mysterious error while refactoring guix/scripts/system.scm

Message ID 87wpjxhx7z.fsf@gmail.com
State New
Headers

Commit Message

Chris Marusich Aug. 4, 2016, 6:20 a.m. UTC
  Hi,

In my local Guix git repo, I've made the attached change on top of
commit 7972d8a2e98af6592050a37036c2c80a01358fcf.  I was surprised to
find that when I built it and ran './pre-inst-env guix system
list-generations', the following error occurred:

--8<---------------cut here---------------start------------->8---
Backtrace:
In ice-9/boot-9.scm:
 157: 19 [catch system-error ...]
In guix/scripts/system.scm:
 884: 18 [#<procedure 43cdba0 at guix/scripts/system.scm:876:2 ()>]
 818: 17 [process-command list-generations () ...]
In guix/store.scm:
1182: 16 [run-with-store # ...]
In guix/scripts/system.scm:
 819: 15 [#<procedure 43d9f00 at guix/scripts/system.scm:819:6 (state)> #]
In guix/packages.scm:
1092: 14 [#<procedure 43d3940 at guix/packages.scm:1091:2 (store)> #]
 734: 13 [cache! #<weak-key-hash-table 1f79180 0/113> # # ...]
1038: 12 [thunk]
 970: 11 [bag->derivation # # #]
In srfi/srfi-1.scm:
 578: 10 [map #<procedure 43dc510 at guix/packages.scm:972:30 (t-1004895)> #]
In guix/packages.scm:
 794: 9 [expand-input # # # ...]
In guix/store.scm:
1182: 8 [run-with-store # ...]
In guix/packages.scm:
1140: 7 [#<procedure 43b10c0 at guix/packages.scm:1140:5 (state)> #]
In guix/download.scm:
 326: 6 [#<procedure 43b1120 at guix/download.scm:326:8 (state)> #]
In guix/store.scm:
1105: 5 [#<procedure 43dc390 at guix/store.scm:1104:28 (store)> #]
In guix/packages.scm:
 734: 4 [cache! #<weak-key-hash-table 1f79180 0/113> # # ...]
1038: 3 [thunk]
In gnu/packages/bootstrap.scm:
 191: 2 [raw-build #<build-daemon 256.15 43c72c0> "guile-bootstrap-2.0" ...]
In gnu/packages.scm:
  91: 1 [search-bootstrap-binary "tar" #<procedure system (#:optional _)>]
In unknown file:
   ?: 0 [string-append #<procedure system (#:optional _)> "/" "tar"]

ERROR: In procedure string-append:
ERROR: In procedure string-append: Wrong type (expecting string): #<procedure system (#:optional _)>
--8<---------------cut here---------------end--------------->8---

I don't have much experience (yet!) debugging Guile programs, so I
haven't been able to understand the cause of the issue despite my
attempts.  Can anyone help me understand the problem, or provide tips on
how to debug this more effectively?

The motivation for this refactoring is as follows.  I want to eventually
add procedures 'roll-back-system' and 'switch-to-system-generation',
which respectively roll the system back to a previous generation, and
switch to an existing generation.  These new procedures will require
access to the store (e.g., to regenerate grub.cfg).  I must invoke those
procedures somewhere in (guix scripts system), and the obvious place to
do that is inside the 'process-action' procedure.

However, that procedure is written with the assumption that the action
requires an operating system configuration file.  My two new procedures
don't require that.  So, instead, I thought it would be easiest to pull
the store monad setup logic (the 'with-store' stuff) up one level into
the procedure 'process-command' and call my two new procedures inside
the 'process-command' procedure.  It looked to me like there was nothing
specific about the store monad setup logic that required it to exist in
the 'process-action' procedure, so I thought this refactoring would be
simple and clean.  I was apparently wrong, but I can't see why.

This is the last thing blocking me from submitting patches to the email
list which add system roll-back and switch-generation commands to
GuixSD.  I have a series of commits on a branch in my Guix git repo
which implement system-rollback and switch-generation.  If you'd like to
see them for more context, please let me know and I'll provide them,
too.  Thank you!
  

Comments

Andy Wingo Aug. 4, 2016, 7:56 a.m. UTC | #1
On Thu 04 Aug 2016 08:20, Chris Marusich <cmmarusich@gmail.com> writes:

> 1038: 3 [thunk]
> In gnu/packages/bootstrap.scm:
>  191: 2 [raw-build #<build-daemon 256.15 43c72c0> "guile-bootstrap-2.0" ...]
> In gnu/packages.scm:
>   91: 1 [search-bootstrap-binary "tar" #<procedure system (#:optional _)>]
> In unknown file:
>    ?: 0 [string-append #<procedure system (#:optional _)> "/" "tar"]
>
> ERROR: In procedure string-append:
> ERROR: In procedure string-append: Wrong type (expecting string): #<procedure system (#:optional _)>

This is because you moved some code around:

> --- a/guix/scripts/system.scm
> +++ b/guix/scripts/system.scm
> @@ -785,47 +785,46 @@ resulting from command-line parsing."
>                          (grub-configuration-device
>                           (operating-system-bootloader os)))))
>  
> -    (with-store store
> -      (set-build-options-from-command-line store opts)
> -
> -      (run-with-store store
> -        (mbegin %store-monad
> -          (set-guile-for-build (default-guile))
> -          (case action
> -            ((extension-graph)
> -             (export-extension-graph os (current-output-port)))
> -            ((shepherd-graph)
> -             (export-shepherd-graph os (current-output-port)))
> -            (else
> -             (perform-action action os
> -                             #:dry-run? dry?
> -                             #:derivations-only? (assoc-ref opts
> -                                                            'derivations-only?)
> -                             #:use-substitutes? (assoc-ref opts 'substitutes?)
> -                             #:image-size (assoc-ref opts 'image-size)
> -                             #:full-boot? (assoc-ref opts 'full-boot?)
> -                             #:mappings (filter-map (match-lambda
> -                                                      (('file-system-mapping . m)
> -                                                       m)
> -                                                      (_ #f))
> -                                                    opts)
> -                             #:grub? grub?
> -                             #:target target #:device device))))
> -        #:system system))))
> +    (case action
> +      ((extension-graph)
> +       (export-extension-graph os (current-output-port)))
> +      ((shepherd-graph)
> +       (export-shepherd-graph os (current-output-port)))
> +      (else
> +       (perform-action action os
> +                       #:dry-run? dry?
> +                       #:derivations-only? (assoc-ref opts
> +                                                      'derivations-only?)
> +                       #:use-substitutes? (assoc-ref opts 'substitutes?)
> +                       #:image-size (assoc-ref opts 'image-size)
> +                       #:full-boot? (assoc-ref opts 'full-boot?)
> +                       #:mappings (filter-map (match-lambda
> +                                                (('file-system-mapping . m)
> +                                                 m)
> +                                                (_ #f))
> +                                              opts)
> +                       #:grub? grub?
> +                       #:target target #:device device)))))

Here the #:system system was taking the `system' binding from within the
function -- it was lexically bound.

>  (define (process-command command args opts)
>    "Process COMMAND, one of the 'guix system' sub-commands.  ARGS is its
>  argument list and OPTS is the option alist."
> -  (case command
> -    ((list-generations)
> -     ;; List generations.  No need to connect to the daemon, etc.
> -     (let ((pattern (match args
> -                      (() "")
> -                      ((pattern) pattern)
> -                      (x (leave (_ "wrong number of arguments~%"))))))
> -       (list-generations pattern)))
> -    (else
> -     (process-action command args opts))))
> +  (with-store store
> +    (set-build-options-from-command-line store opts)
> +
> +    (run-with-store store
> +      (mbegin %store-monad
> +        (set-guile-for-build (default-guile))
> +        (case command
> +          ((list-generations)
> +           (let ((pattern (match args
> +                            (() "")
> +                            ((pattern) pattern)
> +                            (x (leave (_ "wrong number of arguments~%"))))))
> +             (list-generations pattern)))
> +          (else
> +           (process-action command args opts))))
> +      #:system system)))

Here it's not bound in the function so we take the top-level binding,
which is the `system' library call, a function.

Andy
  

Patch

From 1f3a08b080c75b9be7c74235637cce0f91a249d5 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Wed, 3 Aug 2016 00:39:39 -0700
Subject: [PATCH] Refactor process-action and process-command

---
 guix/scripts/system.scm | 73 ++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index e2c6b2e..8302d57 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -785,47 +785,46 @@  resulting from command-line parsing."
                         (grub-configuration-device
                          (operating-system-bootloader os)))))
 
-    (with-store store
-      (set-build-options-from-command-line store opts)
-
-      (run-with-store store
-        (mbegin %store-monad
-          (set-guile-for-build (default-guile))
-          (case action
-            ((extension-graph)
-             (export-extension-graph os (current-output-port)))
-            ((shepherd-graph)
-             (export-shepherd-graph os (current-output-port)))
-            (else
-             (perform-action action os
-                             #:dry-run? dry?
-                             #:derivations-only? (assoc-ref opts
-                                                            'derivations-only?)
-                             #:use-substitutes? (assoc-ref opts 'substitutes?)
-                             #:image-size (assoc-ref opts 'image-size)
-                             #:full-boot? (assoc-ref opts 'full-boot?)
-                             #:mappings (filter-map (match-lambda
-                                                      (('file-system-mapping . m)
-                                                       m)
-                                                      (_ #f))
-                                                    opts)
-                             #:grub? grub?
-                             #:target target #:device device))))
-        #:system system))))
+    (case action
+      ((extension-graph)
+       (export-extension-graph os (current-output-port)))
+      ((shepherd-graph)
+       (export-shepherd-graph os (current-output-port)))
+      (else
+       (perform-action action os
+                       #:dry-run? dry?
+                       #:derivations-only? (assoc-ref opts
+                                                      'derivations-only?)
+                       #:use-substitutes? (assoc-ref opts 'substitutes?)
+                       #:image-size (assoc-ref opts 'image-size)
+                       #:full-boot? (assoc-ref opts 'full-boot?)
+                       #:mappings (filter-map (match-lambda
+                                                (('file-system-mapping . m)
+                                                 m)
+                                                (_ #f))
+                                              opts)
+                       #:grub? grub?
+                       #:target target #:device device)))))
 
 (define (process-command command args opts)
   "Process COMMAND, one of the 'guix system' sub-commands.  ARGS is its
 argument list and OPTS is the option alist."
-  (case command
-    ((list-generations)
-     ;; List generations.  No need to connect to the daemon, etc.
-     (let ((pattern (match args
-                      (() "")
-                      ((pattern) pattern)
-                      (x (leave (_ "wrong number of arguments~%"))))))
-       (list-generations pattern)))
-    (else
-     (process-action command args opts))))
+  (with-store store
+    (set-build-options-from-command-line store opts)
+
+    (run-with-store store
+      (mbegin %store-monad
+        (set-guile-for-build (default-guile))
+        (case command
+          ((list-generations)
+           (let ((pattern (match args
+                            (() "")
+                            ((pattern) pattern)
+                            (x (leave (_ "wrong number of arguments~%"))))))
+             (list-generations pattern)))
+          (else
+           (process-action command args opts))))
+      #:system system)))
 
 (define (guix-system . args)
   (define (parse-sub-command arg result)
-- 
2.7.3