Message ID | 20220514230508.27885-1-plattfot@posteo.net |
---|---|
State | Accepted |
Headers | show |
Series | Add a function to parse emacs elisp's package header | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
Hi Fredrik, The patches LGTM modulo cosmetic issues: Fredrik Salomonsson <plattfot@posteo.net> skribis: > * guix/build/emacs-utils.scm (emacs-batch-script): New procedure. [...] > +(define (emacs-batch-script expr) > + "Execute the Elisp code EXPR in Emacs batch mode and return output." > + (call-with-port > + (open-pipe* > + OPEN_READ > + (%emacs) "--quick" "--batch" > + (string-append "--eval=" (expr->string expr))) > + read-string)) I suggest something like: (let* ((pipe (open-pipe* …)) (output (read-string pipe)) (status (close-pipe pipe))) (unless (zero? status) ;; Use SRFI-34 + either a &message condition or (better) ;; a dedicate SRFI-35 condition type for the error. (raise (condition …))) output) That way, execution failures would be caught and reported. Ludo’.
Hi Ludo, Ludovic Courtès <ludo@gnu.org> writes: > Hi Fredrik, > > The patches LGTM modulo cosmetic issues: > > Fredrik Salomonsson <plattfot@posteo.net> skribis: > >> * guix/build/emacs-utils.scm (emacs-batch-script): New procedure. > > [...] > >> +(define (emacs-batch-script expr) >> + "Execute the Elisp code EXPR in Emacs batch mode and return output." >> + (call-with-port >> + (open-pipe* >> + OPEN_READ >> + (%emacs) "--quick" "--batch" >> + (string-append "--eval=" (expr->string expr))) >> + read-string)) > > I suggest something like: > > (let* ((pipe (open-pipe* …)) > (output (read-string pipe)) > (status (close-pipe pipe))) > (unless (zero? status) > ;; Use SRFI-34 + either a &message condition or (better) > ;; a dedicate SRFI-35 condition type for the error. > (raise (condition …))) > output) > > That way, execution failures would be caught and reported. Thank you for the feedback. I update the procedure to use a dedicated SRFI-35 condition type. But I cannot figure out how to capture the error message from emacs, as I want the condition type to contain both the expression as well as the error you get from emacs. Here is what I have so far: (define-condition-type &emacs-batch-error &error emacs-batch-error? (expression emacs-batch-error-expression) (message emacs-batch-error-message)) (define (emacs-batch-script expr) "Execute the Elisp code EXPR in Emacs batch mode and return output." (let* ((error-pipe (open-output-string)) (pipe (with-error-to-port error-pipe (lambda () (open-pipe* OPEN_READ (%emacs) "--quick" "--batch" (string-append "--eval=" (expr->string expr)))))) (output (read-string pipe)) (error (get-output-string error-pipe)) (status (close-pipe pipe))) (unless (zero? status) (raise (condition (&emacs-batch-error (expression expr) (message error))))) output)) Here is the output when I test it out in the guix repl: -------------------------------------------------------------------------------- scheme@(guix-user)> (use-modules (guix build emacs-utils)) (use-modules (guix build emacs-utils)) scheme@(guix-user)> (emacs-batch-script '(prog (princ "hello"))) (emacs-batch-script '(prog (princ "hello"))) ice-9/boot-9.scm:1685:16: In procedure raise-exception: Wrong type (expecting exact integer): #<&emacs-batch-error expression: (prog (princ "hello")) message: ""> Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue. scheme@(guix-user) [1]> -------------------------------------------------------------------------------- Note the message is empty in #<&emacs-batch-error…>. It should contain: Debugger entered--Lisp error: (void-function prog) (prog (princ "hello")) command-line-1(("--eval=(prog (princ \"hello\"))")) command-line() normal-top-level() Any idea what I'm doing wrong? Thanks
Hi, Fredrik Salomonsson <plattfot@posteo.net> skribis: > Here is what I have so far: > > (define-condition-type &emacs-batch-error &error > emacs-batch-error? > (expression emacs-batch-error-expression) > (message emacs-batch-error-message)) > > (define (emacs-batch-script expr) > "Execute the Elisp code EXPR in Emacs batch mode and return output." > (let* ((error-pipe (open-output-string)) > (pipe (with-error-to-port error-pipe > (lambda () > (open-pipe* > OPEN_READ > (%emacs) "--quick" "--batch" > (string-append "--eval=" (expr->string expr)))))) > (output (read-string pipe)) > (error (get-output-string error-pipe)) > (status (close-pipe pipe))) > (unless (zero? status) > (raise (condition (&emacs-batch-error > (expression expr) > (message error))))) > output)) Unfortunately ‘open-pipe*’ is not smart enough to redirect stderr to a non-file port (a string port in this case). One way around it would be to merge stdout and stderr, like so: (parameterize ((current-error-port (current-output-port))) (open-pipe* …)) but then you get both on the same stream, which could be a problem for instance if Emacs emits warnings and such. You could work around it by establishing a second pipe: (match (pipe) ((stderr-input . stderr-output) (parameterize ((current-error-port stderr-output)) (open-pipe* …)))) Here you should be able to read, in the parent process, from ‘stderr-input’. Another option is to not try to capture stderr: after all, that’ll get printed on the screen anyway. HTH, Ludo’.
Hi Ludo, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > Fredrik Salomonsson <plattfot@posteo.net> skribis: > >> Here is what I have so far: >> >> (define-condition-type &emacs-batch-error &error >> emacs-batch-error? >> (expression emacs-batch-error-expression) >> (message emacs-batch-error-message)) >> >> (define (emacs-batch-script expr) >> "Execute the Elisp code EXPR in Emacs batch mode and return output." >> (let* ((error-pipe (open-output-string)) >> (pipe (with-error-to-port error-pipe >> (lambda () >> (open-pipe* >> OPEN_READ >> (%emacs) "--quick" "--batch" >> (string-append "--eval=" (expr->string expr)))))) >> (output (read-string pipe)) >> (error (get-output-string error-pipe)) >> (status (close-pipe pipe))) >> (unless (zero? status) >> (raise (condition (&emacs-batch-error >> (expression expr) >> (message error))))) >> output)) > > Unfortunately ‘open-pipe*’ is not smart enough to redirect stderr to a > non-file port (a string port in this case). > > One way around it would be to merge stdout and stderr, like so: > > (parameterize ((current-error-port (current-output-port))) > (open-pipe* …)) > > but then you get both on the same stream, which could be a problem for > instance if Emacs emits warnings and such. > > You could work around it by establishing a second pipe: > > (match (pipe) > ((stderr-input . stderr-output) > (parameterize ((current-error-port stderr-output)) > (open-pipe* …)))) > > Here you should be able to read, in the parent process, from > ‘stderr-input’. > > Another option is to not try to capture stderr: after all, that’ll get > printed on the screen anyway. I just sent in v2 of the patches. Changes are: - emacs-batch-script will now raise an &emacs-batch-error if emacs batch script fails. I opted to capture the stderr and package that up in the condition. Thank you for the pointers! I removed the expression slot I had in my prototype. It felt redundant, emacs error message should be enough. - tests/build-emacs-utils.scm: I added tests for the two new procedures. It was good that I added the tests as I had missed to use (srfi srfi-34) in build/emacs-utils.scm during my prototyping and that generated the cryptic error: Wrong type (expecting exact integer): #<&emacs-batch-error…> Took me a while to figure that one out. But now all is good. The tests are passing. Thanks again.
diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm index 60a754b9e9..8d40b9e139 100644 --- a/guix/build/emacs-utils.scm +++ b/guix/build/emacs-utils.scm @@ -3,6 +3,7 @@ ;;; Copyright © 2014 Alex Kost <alezost@gmail.com> ;;; Copyright © 2018, 2020, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; Copyright © 2019 Liliana Marie Prikler <liliana.prikler@gmail.com> +;;; Copyright © 2022 Fredrik Salomonsson <plattfot@posteo.net> ;;; ;;; This file is part of GNU Guix. ;;; @@ -22,10 +23,13 @@ (define-module (guix build emacs-utils) #:use-module (guix build utils) #:use-module (ice-9 format) + #:use-module (ice-9 popen) + #:use-module (ice-9 rdelim) #:export (%emacs emacs-batch-eval emacs-batch-edit-file emacs-batch-disable-compilation + emacs-batch-script emacs-generate-autoloads emacs-byte-compile-directory @@ -69,6 +73,15 @@ (define (emacs-batch-disable-compilation file) (add-file-local-variable 'no-byte-compile t) (basic-save-buffer)))) +(define (emacs-batch-script expr) + "Execute the Elisp code EXPR in Emacs batch mode and return output." + (call-with-port + (open-pipe* + OPEN_READ + (%emacs) "--quick" "--batch" + (string-append "--eval=" (expr->string expr))) + read-string)) + (define (emacs-generate-autoloads name directory) "Generate autoloads for Emacs package NAME placed in DIRECTORY." (let* ((file (string-append directory "/" name "-autoloads.el"))