| Message ID | 20200310160305.15865-1-ludo@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#40016] inferior: Distinguish inferior exceptions. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
Hello, Ludovic Courtès <ludo@gnu.org> skribis: > This avoids ambiguities when looking at a backtrace where the exception > was actually thrown by an inferior in a very different context. The idea was suggested at the Guix Days by someone who had had a hard time looking at one of these weird backtraces, until you realize the exception was actually thrown by another process. There are a few places in the Data Service that do things like: (catch 'misc-error (lambda () (inferior-eval …)) …) This will have to be adjusted. Does that work for you, Chris? If you want to go fancy, you can implement a compatibility later, though I’m not sure it’s worth it. Eventually I’d like ‘&inferior-exception’ to include inferior stack frames, though that’ll require changes to the REPL protocol. Ludo’.
Ludovic Courtès <ludo@gnu.org> writes: > Ludovic Courtès <ludo@gnu.org> skribis: > >> This avoids ambiguities when looking at a backtrace where the exception >> was actually thrown by an inferior in a very different context. > > The idea was suggested at the Guix Days by someone who had had a hard > time looking at one of these weird backtraces, until you realize the > exception was actually thrown by another process. > > There are a few places in the Data Service that do things like: > > (catch 'misc-error > (lambda () > (inferior-eval …)) > …) > > This will have to be adjusted. Does that work for you, Chris? Yeah that sounds fine. Generally I think it's good to keep backwards compatibility with the Guix Data Service, so I'd be looking to support both ways exceptions can be raised, but that doesn't seem to difficult. Thanks, Chris
Hi! Christopher Baines <mail@cbaines.net> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >> Ludovic Courtès <ludo@gnu.org> skribis: >> >>> This avoids ambiguities when looking at a backtrace where the exception >>> was actually thrown by an inferior in a very different context. >> >> The idea was suggested at the Guix Days by someone who had had a hard >> time looking at one of these weird backtraces, until you realize the >> exception was actually thrown by another process. >> >> There are a few places in the Data Service that do things like: >> >> (catch 'misc-error >> (lambda () >> (inferior-eval …)) >> …) >> >> This will have to be adjusted. Does that work for you, Chris? > > Yeah that sounds fine. Good. > Generally I think it's good to keep backwards compatibility with the > Guix Data Service, so I'd be looking to support both ways exceptions > can be raised, but that doesn't seem to difficult. Yes. Though you’ll also have to handle Guile 3.0 vs. 2.2 exception types, in particular wrt. SRFI-34 exceptions (see Guix commit 7f3bbfaf8ec3b96e02e0cf74e7515ac33c002107.) Thanks for your feedback, Ludo’.
Ludovic Courtès <ludo@gnu.org> skribis: > This avoids ambiguities when looking at a backtrace where the exception > was actually thrown by an inferior in a very different context. > > * guix/inferior.scm (&inferior-exception): New condition type. > (read-repl-response): Add optional 'inferior' parameter. Raise > '&inferior-exception' instead of rethrowing to KEY when receiving an > 'exception' message. > (read-inferior-response): Pass INFERIOR to 'read-repl-response'. > * tests/inferior.scm ("&inferior-exception"): New test. Pushed as f7537e30b892cef09d91902547c00e5fa9b66f3b. Ludo’.
diff --git a/guix/inferior.scm b/guix/inferior.scm index 0236fb61ad..6b685ece30 100644 --- a/guix/inferior.scm +++ b/guix/inferior.scm @@ -63,6 +63,9 @@ inferior-eval inferior-eval-with-store inferior-object? + inferior-exception? + inferior-exception-arguments + inferior-exception-inferior read-repl-response inferior-packages @@ -195,8 +198,15 @@ equivalent. Return #f if the inferior could not be launched." (set-record-type-printer! <inferior-object> write-inferior-object) -(define (read-repl-response port) - "Read a (guix repl) response from PORT and return it as a Scheme object." +;; Reified exception thrown by an inferior. +(define-condition-type &inferior-exception &error + inferior-exception? + (arguments inferior-exception-arguments) ;key + arguments + (inferior inferior-exception-inferior)) ;<inferior> | #f + +(define* (read-repl-response port #:optional inferior) + "Read a (guix repl) response from PORT and return it as a Scheme object. +Raise '&inferior-exception' when an exception is read from PORT." (define sexp->object (match-lambda (('value value) @@ -208,10 +218,13 @@ equivalent. Return #f if the inferior could not be launched." (('values objects ...) (apply values (map sexp->object objects))) (('exception key objects ...) - (apply throw key (map sexp->object objects))))) + (raise (condition (&inferior-exception + (arguments (cons key (map sexp->object objects))) + (inferior inferior))))))) (define (read-inferior-response inferior) - (read-repl-response (inferior-socket inferior))) + (read-repl-response (inferior-socket inferior) + inferior)) (define (send-inferior-request exp inferior) (write exp (inferior-socket inferior)) diff --git a/tests/inferior.scm b/tests/inferior.scm index f54b6d6037..b4417d8629 100644 --- a/tests/inferior.scm +++ b/tests/inferior.scm @@ -1,5 +1,5 @@ ;;; GNU Guix --- Functional package management for GNU -;;; Copyright © 2018, 2019 Ludovic Courtès <ludo@gnu.org> +;;; Copyright © 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -61,6 +61,17 @@ (close-inferior inferior) (list a (inferior-object? b)))))) +(test-equal "&inferior-exception" + '(a b c d) + (let ((inferior (open-inferior %top-builddir + #:command "scripts/guix"))) + (guard (c ((inferior-exception? c) + (close-inferior inferior) + (and (eq? inferior (inferior-exception-inferior c)) + (inferior-exception-arguments c)))) + (inferior-eval '(throw 'a 'b 'c 'd) inferior) + 'badness))) + (test-equal "inferior-packages" (take (sort (fold-packages (lambda (package lst) (cons (list (package-name package)
This avoids ambiguities when looking at a backtrace where the exception was actually thrown by an inferior in a very different context. * guix/inferior.scm (&inferior-exception): New condition type. (read-repl-response): Add optional 'inferior' parameter. Raise '&inferior-exception' instead of rethrowing to KEY when receiving an 'exception' message. (read-inferior-response): Pass INFERIOR to 'read-repl-response'. * tests/inferior.scm ("&inferior-exception"): New test. --- guix/inferior.scm | 21 +++++++++++++++++---- tests/inferior.scm | 13 ++++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-)