Mysterious error while refactoring guix/scripts/system.scm

Message ID 8737md3x5a.fsf@gmail.com
State New
Headers

Commit Message

Chris Marusich Aug. 10, 2016, 7:23 a.m. UTC
  Hi Andy,

Thank you for the help!  Sorry for the late reply; I wasn't able to
respond until just now.

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

You're right; that was the problem.  I've updated the patch (see
attached) and applied it again to
7972d8a2e98af6592050a37036c2c80a01358fcf, but when I built the new
version and ran './pre-inst-env guix system list-generations', the
following new error occurred (after successfully printing the list of
generations):

--8<---------------cut here---------------start------------->8---
Generation 1	Jul 29 2016 08:26:37
  file name: /var/guix/profiles/system-1-link
  canonical file name: /gnu/store/90xaq1d04g39c1wmxplngh3hhc1h5z7g-system
  label: GNU with Linux-Libre 4.7 (beta)
  root device: /dev/sda1
  kernel: /gnu/store/p7wbfpbs8p8ykdrn1440f95ap9iq1czh-linux-libre-4.7
Generation 2	Aug 03 2016 07:35:00
  file name: /var/guix/profiles/system-2-link
  canonical file name: /gnu/store/6il3znhh2cyp3xw0800x6m3yjq8jq5z9-system
  label: GNU with Linux-Libre 4.7 (beta)
  root device: my-root
  kernel: /gnu/store/p7wbfpbs8p8ykdrn1440f95ap9iq1czh-linux-libre-4.7
Generation 3	Aug 03 2016 07:38:12
  file name: /var/guix/profiles/system-3-link
  canonical file name: /gnu/store/74d698pk4dwv4k4bvf2n4b7j4bg0gbq0-system
  label: GNU with Linux-Libre 4.7 (beta)
  root device: #vu8(18 55 238 217 114 62 64 185 130 140 206 84 109 78 188 217)
  kernel: /gnu/store/p7wbfpbs8p8ykdrn1440f95ap9iq1czh-linux-libre-4.7
Generation 4	Aug 03 2016 07:55:20	(current)
  file name: /var/guix/profiles/system-4-link
  canonical file name: /gnu/store/01fd40gf19syn493y2f6h35cb58kqmg4-system
  label: GNU with Linux-Libre 4.7 (beta)
  root device: #vu8(18 55 238 217 114 62 64 185 130 140 206 84 109 78 188 217)
  kernel: /gnu/store/p7wbfpbs8p8ykdrn1440f95ap9iq1czh-linux-libre-4.7
Backtrace:
In ice-9/boot-9.scm:
 157: 15 [catch #t #<catch-closure 26304a0> ...]
In unknown file:
   ?: 14 [apply-smob/1 #<catch-closure 26304a0>]
In ice-9/boot-9.scm:
  63: 13 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 432: 12 [eval # #]
In ice-9/boot-9.scm:
2401: 11 [save-module-excursion #<procedure 264f940 at ice-9/boot-9.scm:4045:3 ()>]
4050: 10 [#<procedure 264f940 at ice-9/boot-9.scm:4045:3 ()>]
1724: 9 [%start-stack load-stack ...]
1729: 8 [#<procedure 2668ea0 ()>]
In unknown file:
   ?: 7 [primitive-load "/root/guix/scripts/guix"]
In guix/ui.scm:
1209: 6 [run-guix-command system "list-generations"]
In ice-9/boot-9.scm:
 157: 5 [catch srfi-34 #<procedure 52af180 at guix/ui.scm:425:2 ()> ...]
 157: 4 [catch system-error ...]
In guix/scripts/system.scm:
 884: 3 [#<procedure 52ae750 at guix/scripts/system.scm:876:2 ()>]
 818: 2 [process-command list-generations () ...]
In guix/store.scm:
1182: 1 [run-with-store # ...]
In unknown file:
   ?: 0 [#<unspecified> #<build-daemon 256.15 52b8e80>]

ERROR: In procedure #<unspecified>:
ERROR: Wrong type to apply: #<unspecified>
--8<---------------cut here---------------end--------------->8---

I'm hoping you can help me (1) understand this particular problem, and
(2) help me learn to become more self-sufficient at troubleshooting
these kinds of issues going forward.  I don't want to have to bother you
or the mailing list every time I encounter a stack trace.

Regarding (1), I'm having trouble understanding this particular stack
trace.  Where did the error occur?  What caused it?  I'm used to stack
traces in Java and Python, where often the exact line which threw an
exception is usually obvious in the stack trace.  I'm finding Guile's
stack traces more difficult to understand.  Maybe I'm just not used to
them yet.

Regarding (2), I've read some of the manual at '(guile) Debugging'
(mainly '(guile) Evaluation Model' and '(guile) Programmatic Error
Handling'), and although it's great and detailed, it isn't obvious to me
how to apply that knowledge in practice.  For example, I'm not even sure
how to find out what the parts and symbols in the above stack trace
mean.  How can I get better at this?  Should I just read the rest of
'(guile) Debugging' in detail, and experiment with my own toy programs
until it makes more sense?  Or is there a common tool people use for
debugging that I'm not aware of, like how pdb is a common debugging tool
for Python programs?

Again, thank you for your help.  I really appreciate it!
  

Comments

Ludovic Courtès Aug. 29, 2016, 3:53 p.m. UTC | #1
Hi Chris, and sorry for the delay!

Chris Marusich <cmmarusich@gmail.com> skribis:

> Backtrace:
> In ice-9/boot-9.scm:
>  157: 15 [catch #t #<catch-closure 26304a0> ...]
> In unknown file:
>    ?: 14 [apply-smob/1 #<catch-closure 26304a0>]
> In ice-9/boot-9.scm:
>   63: 13 [call-with-prompt prompt0 ...]
> In ice-9/eval.scm:
>  432: 12 [eval # #]
> In ice-9/boot-9.scm:
> 2401: 11 [save-module-excursion #<procedure 264f940 at ice-9/boot-9.scm:4045:3 ()>]
> 4050: 10 [#<procedure 264f940 at ice-9/boot-9.scm:4045:3 ()>]
> 1724: 9 [%start-stack load-stack ...]
> 1729: 8 [#<procedure 2668ea0 ()>]
> In unknown file:
>    ?: 7 [primitive-load "/root/guix/scripts/guix"]
> In guix/ui.scm:
> 1209: 6 [run-guix-command system "list-generations"]
> In ice-9/boot-9.scm:
>  157: 5 [catch srfi-34 #<procedure 52af180 at guix/ui.scm:425:2 ()> ...]
>  157: 4 [catch system-error ...]
> In guix/scripts/system.scm:
>  884: 3 [#<procedure 52ae750 at guix/scripts/system.scm:876:2 ()>]
>  818: 2 [process-command list-generations () ...]
> In guix/store.scm:
> 1182: 1 [run-with-store # ...]
> In unknown file:
>    ?: 0 [#<unspecified> #<build-daemon 256.15 52b8e80>]
>
> ERROR: In procedure #<unspecified>:
> ERROR: Wrong type to apply: #<unspecified>

This means that we’re trying to invoke #<unspecified>, but
#<unspecified> is not a procedure.

Note that this is from within ‘run-to-store’, which is the procedure to
“run” a monadic value.  So in effect, what happened is equivalent to:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix monads)
scheme@(guile-user)> ,use(guix store)
scheme@(guile-user)> (with-store s
		       (run-with-store s *unspecified*))
ERROR: In procedure #<unspecified>:
ERROR: Wrong type to apply: #<unspecified>
--8<---------------cut here---------------end--------------->8---

The bug here is in fact a type error: ‘run-with-store’ expect a monadic
value, but what we have here is a regular value.

To “fix” it, we need:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (with-store s
		       (run-with-store s
			 (with-monad %store-monad (return *unspecified*))))
--8<---------------cut here---------------end--------------->8---

… where ‘return’ procedure a monadic value from a normal value.

In your case, ‘list-generations’ is not a monadic procedure (a procedure
that returns a monadic value), so in this patch:

  (run-with-store store
    …
    (list-generations))

… triggers this very type error.

I imagine this may be more than you wanted to learn.  ;-)
Monads in a dynamically typed setting are kinda annoying because of
this.

>  (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 (assoc-ref opts 'system))))

As the comment above suggests, the idea here was to avoid connecting to
the daemon for operations that do not need it, such as
‘list-generations’.  I think we should preserve this property.

Thanks for your work!  I’m sorry this is more painful than I thought.
:-/

Ludo’.
  
Chris Marusich Sept. 24, 2016, 7:54 p.m. UTC | #2
Hi Ludo’,

Thank you for getting back to me!

ludo@gnu.org (Ludovic Courtès) writes:

> Hi Chris, and sorry for the delay!
>
> Chris Marusich <cmmarusich@gmail.com> skribis:
>
>> Backtrace:
>> In ice-9/boot-9.scm:
>>  157: 15 [catch #t #<catch-closure 26304a0> ...]
>> In unknown file:
>>    ?: 14 [apply-smob/1 #<catch-closure 26304a0>]
>> In ice-9/boot-9.scm:
>>   63: 13 [call-with-prompt prompt0 ...]
>> In ice-9/eval.scm:
>>  432: 12 [eval # #]
>> In ice-9/boot-9.scm:
>> 2401: 11 [save-module-excursion #<procedure 264f940 at ice-9/boot-9.scm:4045:3 ()>]
>> 4050: 10 [#<procedure 264f940 at ice-9/boot-9.scm:4045:3 ()>]
>> 1724: 9 [%start-stack load-stack ...]
>> 1729: 8 [#<procedure 2668ea0 ()>]
>> In unknown file:
>>    ?: 7 [primitive-load "/root/guix/scripts/guix"]
>> In guix/ui.scm:
>> 1209: 6 [run-guix-command system "list-generations"]
>> In ice-9/boot-9.scm:
>>  157: 5 [catch srfi-34 #<procedure 52af180 at guix/ui.scm:425:2 ()> ...]
>>  157: 4 [catch system-error ...]
>> In guix/scripts/system.scm:
>>  884: 3 [#<procedure 52ae750 at guix/scripts/system.scm:876:2 ()>]
>>  818: 2 [process-command list-generations () ...]
>> In guix/store.scm:
>> 1182: 1 [run-with-store # ...]
>> In unknown file:
>>    ?: 0 [#<unspecified> #<build-daemon 256.15 52b8e80>]
>>
>> ERROR: In procedure #<unspecified>:
>> ERROR: Wrong type to apply: #<unspecified>
>
> This means that we’re trying to invoke #<unspecified>, but
> #<unspecified> is not a procedure.
>
> Note that this is from within ‘run-to-store’, which is the procedure to
> “run” a monadic value.  So in effect, what happened is equivalent to:
>
>
> scheme@(guile-user)> ,use(guix monads)
> scheme@(guile-user)> ,use(guix store)
> scheme@(guile-user)> (with-store s
> 		       (run-with-store s *unspecified*))
> ERROR: In procedure #<unspecified>:
> ERROR: Wrong type to apply: #<unspecified>
>
>
> The bug here is in fact a type error: ‘run-with-store’ expect a monadic
> value, but what we have here is a regular value.
>
> To “fix” it, we need:
>
>
> scheme@(guile-user)> (with-store s
> 		       (run-with-store s
> 			 (with-monad %store-monad (return *unspecified*))))
>
>
> … where ‘return’ procedure a monadic value from a normal value.
>
> In your case, ‘list-generations’ is not a monadic procedure (a procedure
> that returns a monadic value), so in this patch:
>
>   (run-with-store store
>     …
>     (list-generations))
>
> … triggers this very type error.

I see.  I've read the sections in the manual about gexps, and I've
peeked at the gexp code.  I thought that basically I could use
run-with-store to run any Guile code I want, but I seem to be missing
something.  I will try experimenting with monads and the store to learn
more and hopefully avoid surprises like this going forward.

> I imagine this may be more than you wanted to learn.  ;-)
> Monads in a dynamically typed setting are kinda annoying because of
> this.

I appreciate the info.  Hopefully I will fully understand it soon.

>>  (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 (assoc-ref opts 'system))))
>
> As the comment above suggests, the idea here was to avoid connecting to
> the daemon for operations that do not need it, such as
> ‘list-generations’.  I think we should preserve this property.

The reason I wanted to perform this refactoring in the first place is
because I'd like to add a new procedure to guix/scripts/system.scm
called 'switch-to-system-generation'.  Because this new procedure calls
other procedures which seem to require access to the store, I thought I
would need to call 'switch-to-system-generation' via 'run-with-store'.
Am I just confused?

In particular, 'switch-to-system-generation' will eventually call the
existing procedure 'grub-configuration-file' (defined in
gnu/system/grub.scm).  As I understand it, 'grub-configuration-file'
returns a derivation that builds a GRUB configuration file.  This
existing 'grub-configuration-file' procedure does a lot with the store
and gexps.  I thought that if I didn't use 'run-with-store' to run
'switch-to-system-generation', it wouldn't work because
'grub-configuration-file' wouldn't work.

> Thanks for your work!  I’m sorry this is more painful than I thought.
> :-/
>
> Ludo’.

I'm happy to help.  I just wish I understood these monads better.  And I
wish I had more time to work on it!  Anyway, I'll keep at it.
  
Ludovic Courtès Sept. 30, 2016, 8:36 p.m. UTC | #3
Hello,

Chris Marusich <cmmarusich@gmail.com> skribis:

> The reason I wanted to perform this refactoring in the first place is
> because I'd like to add a new procedure to guix/scripts/system.scm
> called 'switch-to-system-generation'.  Because this new procedure calls
> other procedures which seem to require access to the store, I thought I
> would need to call 'switch-to-system-generation' via 'run-with-store'.
> Am I just confused?

Essentially, to write a procedure that needs to access the store, you
need to write a “monadic procedure”—i.e., a procedure that returns a
value in the “store monad” instead of a “normal value.”

To do that, you would write:

  (define (switch-to-system-generation …)
    (with-monad %store-monad
      (return 'some-value)))    ;return this symbol as a monadic value

or:

  (define (switch-to-system-generation …)
     (some-monadic-procedure x y z))  ;tail-call a monadic procedure

The caller of a monadic procedure must be prepared to “unpack” its
value, using the monadic “bind” operator.  The ‘mlet’ form does exactly
that:

  (define (the-caller …)
    (mlet %store-monad ((result (switch-to-system-generation …)))
      …))

At the bottom, there must be somewhere a call to ‘run-with-store’ to
“run” the monadic value in the monad (info "(guix) The Store Monad").
This call is already in ‘process-action’ in (guix scripts system).

> In particular, 'switch-to-system-generation' will eventually call the
> existing procedure 'grub-configuration-file' (defined in
> gnu/system/grub.scm).  As I understand it, 'grub-configuration-file'
> returns a derivation that builds a GRUB configuration file.  This
> existing 'grub-configuration-file' procedure does a lot with the store
> and gexps.  I thought that if I didn't use 'run-with-store' to run
> 'switch-to-system-generation', it wouldn't work because
> 'grub-configuration-file' wouldn't work.

‘grub-configuration-file’ is a monadic procedure.  Thus, to “unpack” its
return value, you need to bind it, for instance with ‘mlet’:

  (mlet %store-monad ((file (grub-configuration-file …)))
    …)

or:

  (with-monad %store-monad
    (>>= (grub-configuration-file …)
         (lambda (file)
           …)))

HTH!

Ludo’.
  

Patch

From a3e094e5393ac7289075a0b63696bd9e5024b202 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 | 74 ++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 38 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 209ebf9..731e3cd 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -774,7 +774,6 @@  resulting from command-line parsing."
   (let* ((file     (match args
                      (() #f)
                      ((x . _) x)))
-         (system   (assoc-ref opts 'system))
          (os       (if file
                        (load* file %user-module
                               #:on-error (assoc-ref opts 'on-error))
@@ -789,47 +788,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 (assoc-ref opts 'system))))
 
 (define (guix-system . args)
   (define (parse-sub-command arg result)
-- 
2.7.3