diff mbox series

[bug#55420,1/2] guix: emacs-utils: Add emacs-batch-script.

Message ID 20220514230508.27885-1-plattfot@posteo.net
State Accepted
Headers show
Series Add a function to parse emacs elisp's package header | expand

Checks

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

Commit Message

Fredrik Salomonsson May 14, 2022, 11:05 p.m. UTC
* guix/build/emacs-utils.scm (emacs-batch-script): New procedure.
---
 guix/build/emacs-utils.scm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Ludovic Courtès June 1, 2022, 8:38 p.m. UTC | #1
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’.
Fredrik Salomonsson June 2, 2022, 2:53 a.m. UTC | #2
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
Ludovic Courtès June 2, 2022, 1:44 p.m. UTC | #3
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’.
Fredrik Salomonsson June 5, 2022, 12:42 a.m. UTC | #4
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 mbox series

Patch

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"))