bug#33016: 26.1; (make-process ...) doesn't signal an error, when executable given as absolute Windows path does not exist

[Noam Postavsky <npostavs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

On Thu, 11 Apr 2019 at 10:05, Eli Zaretskii <eliz@gnu.org> wrote:

> > static intmax_t when_entered_debugger;
> >
> > So I guess we'd need some way of resetting it from Lisp?
>
> Doesn't it work to simply set its value before the second test?

Yes, or in each test, since the tests don't necessarily have knowledge
of what order they're called in (I think it's currently alphabetical
order of test name). See attached diff (against the state of my v3
patch), but it seems a bit silly to make the variable Lisp accessible
just for this obscure test case. I don't see any other way though.

> > As far as I can tell, the normal debugger resets it by calling
> > recursive-edit, but there's no way to return from that without human
> > intervention (I think?).
>
> Doesn't abort-recursive-edit work noninteractively?

Yes, but how can I arrange for it to be called without stopping to
read commands from the user first? E.g., in the following
abort-recursive-edit is too late to do any good:

(progn
  (recursive-edit)
  (abort-recursive-edit))

Using pre-command-hook is also too late, the user has to type
something to trigger the beginning of a certain command first.

(let ((pre-command-hook #'abort-recursive-edit))
  (recursive-edit))

> > +                    ;; On Windows, "nul.FOO" is the empty file for any
> > +                    ;; FOO, in any directory.  So this passes Emacs'
>
> Instead of "is the empty file", I'd say something like "resolves to
> the null device, reading from which sets the EOF condition".

Hmm, while technically more accurate, it seems like a little too much
detail to be useful; I think saying "acts like an always-empty file"
should be enough.

[-- Attachment #2: reset-when-entered.debugger.diff --]
[-- Type: application/octet-stream, Size: 3234 bytes --]

diff --git i/src/.gdbinit w/src/.gdbinit
index b8b3031..32b2680 100644
--- i/src/.gdbinit
+++ w/src/.gdbinit
@@ -1227,6 +1227,7 @@ if defined_HAVE_X_WINDOWS
   break x_error_quitter
 end
 
+break Fredraw_display
 
 # Put the Python code at the end of .gdbinit so that if GDB does not
 # support Python, GDB will do all the above initializations before
diff --git i/src/eval.c w/src/eval.c
index e9f118c..6c1a2bb 100644
--- i/src/eval.c
+++ w/src/eval.c
@@ -59,7 +59,7 @@ Lisp_Object Vrun_hooks;
    signal the error instead of entering an infinite loop of debugger
    invocations.  */
 
-static intmax_t when_entered_debugger;
+//static intmax_t when_entered_debugger;
 
 /* The function from which the last `signal' was called.  Set in
    Fsignal.  */
@@ -4170,6 +4170,12 @@ Note that `debug-on-error', `debug-on-quit' and friends
 still determine whether to handle the particular condition.  */);
   Vdebug_on_signal = Qnil;
 
+  DEFSYM (Qwhen_entered_debugger, "when-entered-debugger");
+  DEFVAR_INT ("when-entered-debugger", when_entered_debugger,
+              doc: /* The number of keyboard events as of last time `debugger' was called.
+Used to avoid infinite loops if the debugger itself has an error.
+Don't set this unless you're sure that can't happen.  */);
+
   /* When lexical binding is being used,
    Vinternal_interpreter_environment is non-nil, and contains an alist
    of lexically-bound variable, or (t), indicating an empty
diff --git i/test/src/callproc-tests.el w/test/src/callproc-tests.el
index a2728a3..28f7975 100644
--- i/test/src/callproc-tests.el
+++ w/test/src/callproc-tests.el
@@ -43,7 +43,12 @@
   (skip-unless (eq system-type 'windows-nt))
   (let* ((debug-on-error t)
          (have-called-debugger nil)
-         (debugger (lambda (&rest _) (setq have-called-debugger t))))
+         (debugger (lambda (&rest _)
+                     (setq have-called-debugger t)
+                     ;; Allow entering the debugger later in the same
+                     ;; test run, before going back to the command
+                     ;; loop.
+                     (setq when-entered-debugger -1))))
     (should (eq :got-error ;; NOTE: `should-error' would inhibit debugger.
                 (condition-case-unless-debug ()
                     ;; On Windows, "nul.FOO" is the empty file for any
diff --git i/test/src/process-tests.el w/test/src/process-tests.el
index 25a52b7..640f292 100644
--- i/test/src/process-tests.el
+++ w/test/src/process-tests.el
@@ -220,7 +220,12 @@ process-tests--mixable
   (skip-unless (eq system-type 'windows-nt))
   (let* ((debug-on-error t)
          (have-called-debugger nil)
-         (debugger (lambda (&rest _) (setq have-called-debugger t))))
+         (debugger (lambda (&rest _)
+                     (setq have-called-debugger t)
+                     ;; Allow entering the debugger later in the same
+                     ;; test run, before going back to the command
+                     ;; loop.
+                     (setq when-entered-debugger -1))))
     (should (eq :got-error ;; NOTE: `should-error' would inhibit debugger.
                 (condition-case-unless-debug ()
                     ;; Emacs doesn't search for absolute filenames, so