bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

From emacs -Q, evaluating

  (call-process-shell-command (shell-quote-argument "c:/path with
space/foo-bar.bat") nil t t (shell-quote-argument "x y"))

or the equivalent

  (call-process-shell-command "\"c:/path with space/foo-bar.bat\"" nil
t t "\"x y\"")

gives

  'c:/path' is not recognized as an internal or external command,
  operable program or batch file.

It also fails for paths without a space:

  (call-process-shell-command "\"c:/path-without-space/foo-bar.bat\""
nil t t "\"x y\"")

gives

  'c:/path-without-space/foo-bar.bat" "x' is not recognized as an
internal or external command,
  operable program or batch file.

Doing the same with an .exe file works correctly. Quoting the spaces
with ^ works for the .bat file:

  (call-process-shell-command "c:/path^ with^ space/foo-bar.bat" nil t
t "\"x y\"")

gives

  this is foo-bar, arg1 = "x y", arg2 =

But this doesn't work for .exe files.

foo-bar.bat has contents

  @echo this is foo-bar, arg1 = %1, arg2 = %2

In GNU Emacs 24.3.1 (i386-mingw-nt6.1.7601)
 of 2013-03-17 on MARVIN
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --with-gcc (4.7) --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include
 -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
 -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'

Important settings:
  value of $LANG: ENC
  locale-coding-system: cp1252
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<help-echo> ( c a l l - s h e l C-M-i SPC C-a C-y C-k
C-x C-e C-p C-p C-p C-M-f C-o C-o C-p RET C-y C-x C-e
C-p C-p C-p C-f C-SPC C-M-f M-w C-x C-x M-w M-x r e
p o r t SPC e m <tab> <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set
1
Mark set
1

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp w32-common-fns disp-table w32-win
w32-vars tool-bar dnd fontset image regexp-opt fringe tabulated-list
newcomment lisp-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
w32 multi-tty emacs)

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> Date: Thu, 16 Oct 2014 00:33:55 -0400
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> 
> >From emacs -Q, evaluating
> 
>   (call-process-shell-command (shell-quote-argument "c:/path with
> space/foo-bar.bat") nil t t (shell-quote-argument "x y"))
> 
> or the equivalent
> 
>   (call-process-shell-command "\"c:/path with space/foo-bar.bat\"" nil
> t t "\"x y\"")
> 
> gives
> 
>   'c:/path' is not recognized as an internal or external command,
>   operable program or batch file.
> 
> It also fails for paths without a space:
> 
>   (call-process-shell-command "\"c:/path-without-space/foo-bar.bat\""
> nil t t "\"x y\"")
> 
> gives
> 
>   'c:/path-without-space/foo-bar.bat" "x' is not recognized as an
> internal or external command,
>   operable program or batch file.

This is a Windows misfeature, not an Emacs bug: the CreateProcess
primitive fails like that when passed a command that is a batch file.
To work around, try using cmd.exe as the command, and the batch file
with arguments as its arguments, with "/c" prepended.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Stefan Monnier]

>> (call-process-shell-command "\"c:/path-without-space/foo-bar.bat\""
>> nil t t "\"x y\"")
[...]
> This is a Windows misfeature, not an Emacs bug: the CreateProcess
> primitive fails like that when passed a command that is a batch file.
> To work around, try using cmd.exe as the command, and the batch file
> with arguments as its arguments, with "/c" prepended.

But call-process-shell-command (contrary to call-process) should already
be using cmd.exe, no?


        Stefan

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>,  18745@debbugs.gnu.org
> Date: Thu, 16 Oct 2014 09:47:20 -0400
> 
> >> (call-process-shell-command "\"c:/path-without-space/foo-bar.bat\""
> >> nil t t "\"x y\"")
> [...]
> > This is a Windows misfeature, not an Emacs bug: the CreateProcess
> > primitive fails like that when passed a command that is a batch file.
> > To work around, try using cmd.exe as the command, and the batch file
> > with arguments as its arguments, with "/c" prepended.
> 
> But call-process-shell-command (contrary to call-process) should already
> be using cmd.exe, no?

No, shell-file-name on Windows is set to invoke cmdproxy.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

On Thu, Oct 16, 2014 at 2:50 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> This is a Windows misfeature, not an Emacs bug: the CreateProcess
> primitive fails like that when passed a command that is a batch file.

So there is no way for Emacs to paper over the Windows ugliness?

> To work around, try using cmd.exe as the command, and the batch file
> with arguments as its arguments, with "/c" prepended.

Doesn't work :(  I tried

  (call-process-shell-command
   "cmd.exe" nil t t
   "/c" "\"c:/path with space/foo-bar.bat\"" "\"x y\"")

  (call-process-shell-command
   "cmd.exe" nil t t
   "/c \"c:/path with space/foo-bar.bat\" \"x y\"")

  (call-process-shell-command
   "cmd.exe" nil t t
   "/c" "c:/path with space/foo-bar.bat" "x y")

All of them give

  'c:/path' is not recognized as an internal or external command,
  operable program or batch file.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> Date: Thu, 16 Oct 2014 12:28:58 -0400
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 18745@debbugs.gnu.org
> 
> On Thu, Oct 16, 2014 at 2:50 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > This is a Windows misfeature, not an Emacs bug: the CreateProcess
> > primitive fails like that when passed a command that is a batch file.
> 
> So there is no way for Emacs to paper over the Windows ugliness?

I don't know.  In fact, I don't even know the exact nature of the
"ugliness", as these intimate details of the CreateProcess API are not
documented anywhere, AFAIK, at least not officially.  I just know
about this because I bumped into it in the past.

This is exacerbated in Emacs by the fact that the shell command is
processed twice: once by Emacs, and then again by cmdproxy.

GNU Make overcomes this by detecting these cases, and invoking
CreateProcess in a special way (NULL as the first argument), see the
function process_begin there, around line 710 of sub_proc.c in the GNU
Make sources.  If you can come up with a way to do the same in Emacs,
by some suitable patch to cmdproxy.c, such a patch will be welcome
(assuming either the patch is small, or you will agree to sign legal
papers necessary for submitting substantial patches to FSF projects).

Failing that, I can suggest a workaround: use the short 8+3 alias of
the file name with whitespace.  You can obtain the short alias of any
existing file's name by calling w32-short-filename.  Then you won't
need to quote the batch file name.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Stefan Monnier]

>   (call-process-shell-command
>    "cmd.exe" nil t t
>    "/c \"c:/path with space/foo-bar.bat\" \"x y\"")

A few things to note:

- The "rest" arguments to call-process-shell-command are deprecated.
  I.e. you should use

    (call-process-shell-command
     "cmd.exe /c \"c:/path with space/foo-bar.bat\" \"x y\"" nil t t)

  instead (which is equivalent anyway, and I think it's more honest
  since it doesn't make it seem like those rest args are correctly
  separated).

- Why not use call-process instead (since you don't actually use the
  "shell-command" part, really)?
  Something like either

    (call-process "cmd.exe" nil t t
                  "/c" "\"c:/path with space/foo-bar.bat\" \"x y\"")
  or
    (call-process "c:/path with space/foo-bar.bat" nil t t "x" "y")


        Stefan

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

On Thu, Oct 16, 2014 at 1:06 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> GNU Make overcomes this by detecting these cases, and invoking
> CreateProcess in a special way (NULL as the first argument), see the
> function process_begin there, around line 710 of sub_proc.c in the GNU
> Make sources.  If you can come up with a way to do the same in Emacs,
> by some suitable patch to cmdproxy.c, such a patch will be welcome
> (assuming either the patch is small, or you will agree to sign legal
> papers necessary for submitting substantial patches to FSF projects).

I'll take a look (btw I have already signed for Emacs).

> Failing that, I can suggest a workaround: use the short 8+3 alias of
> the file name with whitespace.  You can obtain the short alias of any
> existing file's name by calling w32-short-filename.  Then you won't
> need to quote the batch file name.

s/w32-short-filename/w32-short-file-name/

That works regardless if the program is a bat or exe which will make
things simpler, thanks.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

On Thu, Oct 16, 2014 at 4:15 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> - The "rest" arguments to call-process-shell-command are deprecated.

I was unaware of that, is it written anywhere?

> - Why not use call-process instead (since you don't actually use the
>   "shell-command" part, really)?

That would probably make more sense, but it turns out not to help anyway:

>     (call-process "cmd.exe" nil t t
>                   "/c" "\"c:/path with space/foo-bar.bat\" \"x y\"")

'\"c:/path with space/foo-bar.bat\"' is not recognized as an internal
or external command,
operable program or batch file.

>     (call-process "c:/path with space/foo-bar.bat" nil t t "x" "y")

'c:\path' is not recognized as an internal or external command,
operable program or batch file.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Stefan Monnier]

>> - The "rest" arguments to call-process-shell-command are deprecated.
> I was unaware of that, is it written anywhere?

It's new in 24.4 (it was actually an oversight: this calling convention
was deprecated for start-process-shell-command in 24.1 IIRC, but for
some reason I didn't notice that it was also used in
call-process-shell-command).

>> - Why not use call-process instead (since you don't actually use the
>> "shell-command" part, really)?
> That would probably make more sense, but it turns out not to help anyway:

I kind of guessed it.  I remember reports that starting commands with
a space in its name is pretty much impossible.  In the case that the
space is in the directory name, I heard you might work around that by
adding the directory to exec-path:

   (let ((exec-path (cons "c:/path with space/" exec-path)))
     (call-process "foo-bar.bat" nil t t "x" "y")

This is all hearsay, tho, because luckily I never need to use
a Windows system.


        Stefan

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> Date: Thu, 16 Oct 2014 17:50:32 -0400
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, 18745@debbugs.gnu.org
> 
> > - Why not use call-process instead (since you don't actually use the
> >   "shell-command" part, really)?
> 
> That would probably make more sense, but it turns out not to help anyway:

Since call-process-shell-command simply calls call-process, that's
expected.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  18745@debbugs.gnu.org
> Date: Thu, 16 Oct 2014 20:38:38 -0400
> 
> I remember reports that starting commands with a space in its name
> is pretty much impossible.

That's an entirely different problem, the reason being how Windows
normalizes file names.  It has nothing to do with CreateProcess
quirks.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

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

On Thu, Oct 16, 2014 at 5:30 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Thu, Oct 16, 2014 at 1:06 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> GNU Make overcomes this by detecting these cases, and invoking
>> CreateProcess in a special way (NULL as the first argument), see the
>> function process_begin there, around line 710 of sub_proc.c in the GNU
>> Make sources.  If you can come up with a way to do the same in Emacs,
>> by some suitable patch to cmdproxy.c, such a patch will be welcome
>> (assuming either the patch is small, or you will agree to sign legal
>> papers necessary for submitting substantial patches to FSF projects).
>
> I'll take a look (btw I have already signed for Emacs).

Patching cmdproxy.c fixes the call-process-shell-command case:

  (call-process-shell-command
   "\"c:/path with space/foo-bar.bat\" \"x &y\"" nil '(t t) t)

To fix the call-process case:

  (call-process
   "c:/path with space/foo-bar.bat" nil '(t t) t "x &y")

required a patch to w32proc.c. This is my first patch to Emacs proper;
hopefully I got everything in the right format.

[-- Attachment #2: bat-quote.ChangeLog --]
[-- Type: application/octet-stream, Size: 323 bytes --]

2014-10-21  Noam Postavsky  <npostavs@users.sourceforget.net>

	Paper over MS Windows CreateProcess deficiencies (Bug#18745).

	* nt/cmdproxy.c (batch_file_p): new function.
	(spawn): if calling a quoted batch file pass NULL for progname.
	* src/w32proc.c (create_child): if calling a quoted batch file
	pass NULL for exe.

[-- Attachment #3: bat-quote.patch --]
[-- Type: application/octet-stream, Size: 2019 bytes --]

--- org/nt/cmdproxy.c	Tue Oct 21 20:39:39 2014
+++ new/nt/cmdproxy.c	Tue Oct 21 20:27:08 2014
@@ -220,6 +220,28 @@
   return o - buf;
 }
 
+/* Return TRUE if PROGNAME is a batch file */
+BOOL
+batch_file_p (const char *progname)
+{
+  const char *exts[] = {".bat", ".cmd"};
+  int n_exts = sizeof (exts) / sizeof (char *);
+  int i;
+
+  const char *ext = strrchr(progname, '.');
+
+  if (ext)
+    {
+      for (i = 0; i < n_exts; i++)
+        {
+          if (stricmp(ext, exts[i]) == 0)
+            return TRUE;
+        }
+    }
+
+  return FALSE;
+}
+
 /* Search for EXEC file in DIR.  If EXEC does not have an extension,
    DIR is searched for EXEC with the standard extensions appended.  */
 int
@@ -469,6 +491,13 @@
 
   memset (&start, 0, sizeof (start));
   start.cb = sizeof (start);
+
+  /* CreateProcess handles batch files as progname specially. This
+     special handling fails when both the batch file and arguments are
+     quoted. We pass NULL as progname to avoid the special
+     handling. */
+  if (progname != NULL && cmdline[0] == '"' && batch_file_p(progname))
+      progname = NULL;
 
   if (CreateProcess (progname, cmdline, &sec_attrs, NULL, TRUE,
 		     0, envblock, dir, &start, &child))
--- org/src/w32proc.c	Tue Oct 21 20:39:43 2014
+++ new/src/w32proc.c	Tue Oct 21 20:27:08 2014
@@ -1078,6 +1078,7 @@
   DWORD flags;
   char dir[ MAX_PATH ];
   char *p;
+  const char *ext;
 
   if (cp == NULL) emacs_abort ();
 
@@ -1115,6 +1116,15 @@
   for (p = dir; *p; p = CharNextA (p))
     if (*p == '/')
       *p = '\\';
+
+  /* CreateProcess handles batch files as exe specially. This special
+     handling fails when both the batch file and arguments are
+     quoted. We pass NULL as exe to avoid the special handling. */
+  if (exe && cmdline[0] == '"' &&
+      (ext = strrchr(exe, '.')) &&
+      (xstrcasecmp (ext, ".bat") == 0
+       || xstrcasecmp (ext, ".cmd") == 0))
+      exe = NULL;
 
   flags = (!NILP (Vw32_start_process_share_console)
 	   ? CREATE_NEW_PROCESS_GROUP

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> Date: Tue, 21 Oct 2014 21:12:27 -0400
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 18745@debbugs.gnu.org
> 
> Patching cmdproxy.c fixes the call-process-shell-command case:
> 
>   (call-process-shell-command
>    "\"c:/path with space/foo-bar.bat\" \"x &y\"" nil '(t t) t)
> 
> To fix the call-process case:
> 
>   (call-process
>    "c:/path with space/foo-bar.bat" nil '(t t) t "x &y")
> 
> required a patch to w32proc.c. This is my first patch to Emacs proper;
> hopefully I got everything in the right format.

Thanks, the changes look reasonable.  If you didn't already, please
run the test suite and make sure there are no regressions due to these
changes.  Bonus points for adding specialized tests to test this
specific issue.

> 	* nt/cmdproxy.c (batch_file_p): new function.
> 	(spawn): if calling a quoted batch file pass NULL for progname.
> 	* src/w32proc.c (create_child): if calling a quoted batch file
> 	pass NULL for exe.

In the future, please begin each ChangeLog entry with a capital
letter.

> +  /* CreateProcess handles batch files as progname specially. This
> +     special handling fails when both the batch file and arguments are
> +     quoted. We pass NULL as progname to avoid the special
> +     handling. */

Please in the future leave 2 spaces between sentences, per the US
English convention we use in Emacs.

> +  if (progname != NULL && cmdline[0] == '"' && batch_file_p(progname))
> +      progname = NULL;

Our coding style is to leave one space between the function name and
the opening parenthesis that follows it.

If there are no more comments or objections, I will commit in a few
days.

Thanks again for working on this.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

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

On Wed, Oct 22, 2014 at 1:12 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Thanks, the changes look reasonable.  If you didn't already, please
> run the test suite and make sure there are no regressions due to these
> changes.  Bonus points for adding specialized tests to test this
> specific issue.

The tests in test/automated? No regressions.

Here is a new patch with added test and formatting issues fixed.

[-- Attachment #2: bat-quote-v2.ChangeLog --]
[-- Type: application/octet-stream, Size: 401 bytes --]

2014-10-22  Noam Postavsky  <npostavs@users.sourceforget.net>

	Paper over MS Windows CreateProcess deficiencies (Bug#18745).

	* nt/cmdproxy.c (batch_file_p): New function.
	(spawn): If calling a quoted batch file pass NULL for progname.
	* src/w32proc.c (create_child): If calling a quoted batch file
	pass NULL for exe.
	* test/automated/process-tests.el (process-test-quoted-batfile):
	New test.


[-- Attachment #3: bat-quote-v2.patch --]
[-- Type: application/octet-stream, Size: 3180 bytes --]

--- org/nt/cmdproxy.c
+++ new/nt/cmdproxy.c
@@ -220,6 +220,28 @@
   return o - buf;
 }
 
+/* Return TRUE if PROGNAME is a batch file. */
+BOOL
+batch_file_p (const char *progname)
+{
+  const char *exts[] = {".bat", ".cmd"};
+  int n_exts = sizeof (exts) / sizeof (char *);
+  int i;
+
+  const char *ext = strrchr (progname, '.');
+
+  if (ext)
+    {
+      for (i = 0; i < n_exts; i++)
+        {
+          if (stricmp (ext, exts[i]) == 0)
+            return TRUE;
+        }
+    }
+
+  return FALSE;
+}
+
 /* Search for EXEC file in DIR.  If EXEC does not have an extension,
    DIR is searched for EXEC with the standard extensions appended.  */
 int
@@ -469,6 +491,13 @@
 
   memset (&start, 0, sizeof (start));
   start.cb = sizeof (start);
+
+  /* CreateProcess handles batch files as progname specially. This
+     special handling fails when both the batch file and arguments are
+     quoted.  We pass NULL as progname to avoid the special
+     handling. */
+  if (progname != NULL && cmdline[0] == '"' && batch_file_p (progname))
+      progname = NULL;
 
   if (CreateProcess (progname, cmdline, &sec_attrs, NULL, TRUE,
 		     0, envblock, dir, &start, &child))
--- org/src/w32proc.c
+++ new/src/w32proc.c
@@ -1078,6 +1078,7 @@
   DWORD flags;
   char dir[ MAX_PATH ];
   char *p;
+  const char *ext;
 
   if (cp == NULL) emacs_abort ();
 
@@ -1115,6 +1116,15 @@
   for (p = dir; *p; p = CharNextA (p))
     if (*p == '/')
       *p = '\\';
+
+  /* CreateProcess handles batch files as exe specially.  This special
+     handling fails when both the batch file and arguments are quoted.
+     We pass NULL as exe to avoid the special handling. */
+  if (exe && cmdline[0] == '"' &&
+      (ext = strrchr (exe, '.')) &&
+      (xstrcasecmp (ext, ".bat") == 0
+       || xstrcasecmp (ext, ".cmd") == 0))
+      exe = NULL;
 
   flags = (!NILP (Vw32_start_process_share_console)
 	   ? CREATE_NEW_PROCESS_GROUP
--- org/test/automated/process-tests.el
+++ new/test/automated/process-tests.el
@@ -50,4 +50,26 @@
   (should
    (process-test-sentinel-wait-function-working-p (lambda () (sit-for 0.01 t)))))
 
+(when (eq system-type 'windows-nt)
+  (ert-deftest process-test-quoted-batfile ()
+    "Check that Emacs hides CreateProcess deficiency (bug#18745)."
+    (let (batfile)
+      (unwind-protect
+          (progn
+            ;; CreateProcess will fail when both the bat file and 1st
+            ;; argument are quoted, so include spaces in both of those
+            ;; to force quoting.
+            (setq batfile (make-temp-file "echo args" nil ".bat"))
+            (with-temp-file batfile
+              (insert "@echo arg1 = %1, arg2 = %2\n"))
+            (with-temp-buffer
+              (call-process batfile nil '(t t) t "x &y")
+              (should (string= (buffer-string) "arg1 = \"x &y\", arg2 = \n")))
+            (with-temp-buffer
+              (call-process-shell-command
+               (mapconcat #'shell-quote-argument (list batfile "x &y") " ")
+               nil '(t t) t)
+              (should (string= (buffer-string) "arg1 = \"x &y\", arg2 = \n"))))
+        (when batfile (delete-file batfile))))))
+
 (provide 'process-tests)

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> Date: Wed, 22 Oct 2014 21:33:59 -0400
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 18745@debbugs.gnu.org
> 
> The tests in test/automated? No regressions.

Great, thanks.

> Here is a new patch with added test and formatting issues fixed.

Looks good, thanks.  And you do get the bonus points.

As I said, will commit in a couple of days.

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Eli Zaretskii]

> Date: Thu, 23 Oct 2014 18:52:58 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 18745@debbugs.gnu.org
> 
> > Date: Wed, 22 Oct 2014 21:33:59 -0400
> > From: Noam Postavsky <npostavs@users.sourceforge.net>
> > Cc: 18745@debbugs.gnu.org
> > 
> > The tests in test/automated? No regressions.
> 
> Great, thanks.
> 
> > Here is a new patch with added test and formatting issues fixed.
> 
> Looks good, thanks.  And you do get the bonus points.
> 
> As I said, will commit in a couple of days.

Done, as trunk revision 118196.  I'm therefore closing this bug.
Thanks a lot for working on this.

Btw, the email address mentioned in your copyright assignment is
different from the one you used in this thread.  Is that address still
valid, or should we always use the one you used here?

bug#18745: 24.3; MS Windows, `call-process-shell-command' fails on `shell-quote-argument'ed bat file with quoted args

[Noam Postavsky]

On Sat, Oct 25, 2014 at 5:16 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Btw, the email address mentioned in your copyright assignment is
> different from the one you used in this thread.  Is that address still
> valid, or should we always use the one you used here?

Oh, this one is just an alias for the other. I generally try to use
this one for public mailing lists.

bug#18745:

[Juanma Barranquero]

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

process-test-quoted-batfile fails on my Windows 10 Home

F process-test-quoted-batfile
    Check that Emacs hides CreateProcess deficiency (bug#18745).
    (ert-test-failed
     ((should
       (string=
(buffer-string)
"arg1 = \"x &y\", arg2 =
"))
      :form
      (string= "arg1 = \"x &y\", arg2 =
" "arg1 = \"x &y\", arg2 =
")
      :value nil))

The result lacks the trailing space before the \n.

"arg1 = \"x &y\", arg2 =\n"

instead of

"arg1 = \"x &y\", arg2 = \n"

[-- Attachment #2: Type: text/html, Size: 1006 bytes --]

bug#18745:

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Tue, 27 Oct 2015 12:49:01 +0100
> 
> process-test-quoted-batfile fails on my Windows 10 Home
> 
> F process-test-quoted-batfile
> Check that Emacs hides CreateProcess deficiency (bug#18745).
> (ert-test-failed
> ((should
> (string=
> (buffer-string)
> "arg1 = \"x &y\", arg2 = 
> "))
> :form
> (string= "arg1 = \"x &y\", arg2 =
> " "arg1 = \"x &y\", arg2 = 
> ")
> :value nil))
> 
> The result lacks the trailing space before the \n.
> 
> "arg1 = \"x &y\", arg2 =\n"
> 
> instead of
> 
> "arg1 = \"x &y\", arg2 = \n"

Do you see the same result if you invoke 'echo' from the cmd prompt?
IOW, does this mean Windows 10 changed the behavior of the built-in
'echo', and it now removes trailing whitespace from its argument?  If
that's the reason, how about modifying the test so that it expects the
correct result depending on the value returned by x-server-version?

bug#18745:

[Juanma Barranquero]

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

On Tue, Oct 27, 2015 at 8:14 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> Do you see the same result if you invoke 'echo' from the cmd prompt?
> IOW, does this mean Windows 10 changed the behavior of the built-in
> 'echo', and it now removes trailing whitespace from its argument?

IIUC your question, no

C:\Users\Juanma>echo A>A.txt

C:\Users\Juanma>echo B    >B.txt

C:\Users\Juanma>dir *.txt

28/10/2015  00:23                 3 A.txt
28/10/2015  00:23                 7 B.txt
               2 archivos             10 bytes

[-- Attachment #2: Type: text/html, Size: 851 bytes --]

bug#18745:

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Wed, 28 Oct 2015 00:24:29 +0100
> Cc: 18745@debbugs.gnu.org
> 
> > Do you see the same result if you invoke 'echo' from the cmd prompt?
> > IOW, does this mean Windows 10 changed the behavior of the built-in
> > 'echo', and it now removes trailing whitespace from its argument?
> 
> IIUC your question, no
> 
> C:\Users\Juanma>echo A>A.txt
> 
> C:\Users\Juanma>echo B >B.txt
> 
> C:\Users\Juanma>dir *.txt
> 
> 28/10/2015 00:23 3 A.txt
> 28/10/2015 00:23 7 B.txt
> 2 archivos 10 bytes

The test actually uses a batch file, so I think it's worthwhile to try
that, and see if the behavior of 'echo' when called from a batch file
changed.

If not, then please try digging deeper, because the unmodified test
work OK for me (not on Windows 10).

bug#18745:

[Juanma Barranquero]

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

On Wed, Oct 28, 2015 at 5:01 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> If not, then please try digging deeper, because the unmodified test
> work OK for me (not on Windows 10).

Well, mystery solved. It's not a bug or change in Windows 10, it's an
incompatibility between CMD.EXE and TCC.EXE (the TakeCommand shell). I
usually run everything from inside TakeCommand, so COMSPEC points to
TCC.EXE, not CMD.EXE.

Not sure how to fix it. I suppose "nothing to do, just run the tests with
the right COMSPEC" is an option. OTOH, the test is supposed to check for a
CreateProcess "deficiency". And certainly 4DOS/4NT/TCC is a quite popular
shell.

[-- Attachment #2: Type: text/html, Size: 806 bytes --]

bug#18745:

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Thu, 29 Oct 2015 04:38:47 +0100
> Cc: 18745@debbugs.gnu.org
> 
> Well, mystery solved. It's not a bug or change in Windows 10, it's an incompatibility between CMD.EXE and TCC.EXE (the TakeCommand shell). I usually run everything from inside TakeCommand, so COMSPEC points to TCC.EXE, not CMD.EXE.
> 
> Not sure how to fix it. I suppose "nothing to do, just run the tests with the right COMSPEC" is an option. OTOH, the test is supposed to check for a CreateProcess "deficiency". And certainly 4DOS/4NT/TCC is a quite popular shell.

The CreateProcess deficiency we test there has nothing to do with
trailing whitespace, btw.

Anyway, does it work to put "ComSpec=%windir%\system32\cmd.exe" into
process-environment before running that code?

bug#18745:

[Juanma Barranquero]

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

On Thu, Oct 29, 2015 at 5:17 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> The CreateProcess deficiency we test there has nothing to do with
> trailing whitespace, btw.

That's what I meant: that changing the test to accommodate for a TCC
incompatibility still would make sense because the trailing space is not
what the test is about. Only I said it with less words and zero clarity.

> Anyway, does it work to put "ComSpec=%windir%\system32\cmd.exe" into
> process-environment before running that code?

No. I had already tried that, and now I've done some more checking. (In all
cases, I run the test from the shell with emacs -batch -l ert -l
process-test [etc]).

- Testing from CMD.EXE works (as expected)
- Setting COMSPEC to point to CMD and testing from TCC.EXE also works.
- let-binding process-environment to (cons
"ComSpec=C:\\windows\\system32\\cmd.exe" process-environment) around the
test does not work.
- Using (setenv "ComSpec" "C:\\Windows\\system32\\cmd.exe") or (setenv
"ComSpec" "%windir%\\system32\\cmd.exe" t) in the test function before
calling call-process does not work
- Same for variants with forward slashes, %windir% vs. explicit path, etc.

I would be very surprised that the process-environment does not affect
call-process, so I *must* be missing something obvious. I'll take a closer
look as soon as I have a little more time.

    J

[-- Attachment #2: Type: text/html, Size: 1713 bytes --]

bug#18745:

[Juanma Barranquero]

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

> I would be very surprised that the process-environment does not affect
call-process, so I *must* be missing something obvious. I'll take a closer
look as soon as I have a little more time.

OK, now this is freak.

REM ---- test.bat ---
@echo off
echo TEST=%TEST>test.log
echo COMSPEC=%COMSPEC>>test.log
REM -- end of test.bat ---


C:\...\test> emacs -Q --batch --eval "(call-process
\"c:/devel/emacs/repo/trunk/test.bat\")"

C:\...\test> type test.log
TEST=AAA
COMSPEC=C:\bin64\JPSoft\TCMD\TCC.EXE

C:\...\test> emacs -Q --batch --eval "(let ((process-environment
'(\"TEST=BBB\" \"COMSPEC=C:\\Windows\\system32\\cmd.exe\"))) (call-process
\"c:/devel/emacs/repo/trunk/test.bat\"))"

C:\...\test> type test.log
TEST=BBB
COMSPEC=C:\bin64\JPSoft\TCMD\TCC.EXE

Is that expected?

[-- Attachment #2: Type: text/html, Size: 1183 bytes --]

bug#18745:

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 30 Oct 2015 15:25:37 +0100
> Cc: 18745@debbugs.gnu.org
> 
> C:\...\test> emacs -Q --batch --eval "(let ((process-environment '(\"TEST=BBB\"
> \"COMSPEC=C:\\Windows\\system32\\cmd.exe\"))) (call-process
> \"c:/devel/emacs/repo/trunk/test.bat\"))"
> 
> C:\...\test> type test.log
> TEST=BBB
> COMSPEC=C:\bin64\JPSoft\TCMD\TCC.EXE
> 
> Is that expected?

No, but it could be some TCC magic, aimed at intercepting any explicit
references to cmd.exe.

What happens if you replace test.bat with a compiled C program that
calls getenv to look at these two variables -- do you see the same
there?  (You could use Emacs with a suitable --eval argument to
emulate such a "C program".)

bug#18745:

[Juanma Barranquero]

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

On Fri, Oct 30, 2015 at 3:55 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> No, but it could be some TCC magic, aimed at intercepting any explicit
> references to cmd.exe.

I think you're right. Running the let-binding variant under CMD.EXE works
as expected (the called process receives the right COMSPEC value).

> What happens if you replace test.bat with a compiled C program that
> calls getenv to look at these two variables -- do you see the same
> there?

Both in TCC.EXE and CMD.EXE the let-binding works correctly.

I don't think there's anything more to do here, other than perhaps adding a
note somewhere that the test doesn't work under TCC.

[-- Attachment #2: Type: text/html, Size: 891 bytes --]

bug#18745:

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 30 Oct 2015 18:09:38 +0100
> Cc: 18745@debbugs.gnu.org
> 
> I don't think there's anything more to do here, other than perhaps adding a
> note somewhere that the test doesn't work under TCC.

If the batch says

  cmd /c echo arg1 = %1, arg2 = %2

i.e. invokes cmd.exe from the batch file, does the test still fail?

If it still fails, I think making the test more tolerant of the
trailing whitespace is IMO a better alternative than letting it fail.
After all, that trailing space is not what we are testing there.

bug#18745:

[Juanma Barranquero]

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

On Fri, Oct 30, 2015 at 9:10 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> If the batch says
>
>   cmd /c echo arg1 = %1, arg2 = %2
>
> i.e. invokes cmd.exe from the batch file, does the test still fail?

With

            (with-temp-file batfile
              (insert "@cmd /c echo arg1 = %1, arg2 = %2\n"))

it still works on cmd.exe, fails on tcc.exe

> If it still fails, I think making the test more tolerant of the
> trailing whitespace is IMO a better alternative than letting it fail.
> After all, that trailing space is not what we are testing there.

Something like this, you mean?

diff --git a/test/automated/process-tests.el
b/test/automated/process-tests.el
index 1dab615..58a2de7 100644
--- a/test/automated/process-tests.el
+++ b/test/automated/process-tests.el
@@ -61,15 +61,15 @@ process-test-sentinel-wait-function-working-p
             ;; to force quoting.
             (setq batfile (make-temp-file "echo args" nil ".bat"))
             (with-temp-file batfile
-              (insert "@echo arg1 = %1, arg2 = %2\n"))
+              (insert "@echo arg1=%1, arg2=%2\n"))
             (with-temp-buffer
               (call-process batfile nil '(t t) t "x &y")
-              (should (string= (buffer-string) "arg1 = \"x &y\", arg2 =
\n")))
+              (should (string= (buffer-string) "arg1=\"x &y\", arg2=\n")))
             (with-temp-buffer
               (call-process-shell-command
                (mapconcat #'shell-quote-argument (list batfile "x &y") " ")
                nil '(t t) t)
-              (should (string= (buffer-string) "arg1 = \"x &y\", arg2 =
\n"))))
+              (should (string= (buffer-string) "arg1=\"x &y\", arg2=\n"))))
         (when batfile (delete-file batfile))))))

 (ert-deftest process-test-stderr-buffer ()
warning: LF will be replaced by CRLF in test/automated/process-tests.el.
The file will have its original line endings in your working directory.



BTW, curiously, the stderr-related tests in the same test file
(process-test-stderr-buffer and process-test-stderr-filter) fail sometimes.
Much more frequently on TCC than CMD, but they also fail in CMD, as in
these examples:

C:\...\automated> ..\..\src\emacs.exe -Q -batch -l ert -l process-tests.elc
-f ert-run-tests-batch-and-exit
Running 5 tests (2015-10-31 23:35:46+0100)
   passed  1/5  process-test-quoted-batfile
   passed  2/5  process-test-sentinel-accept-process-output
   passed  3/5  process-test-sentinel-sit-for
Test process-test-stderr-buffer backtrace:
  #[nil "\306\307C \310 \311 \3121 \313\216\314      \"\211 )0\202 \210
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test process-test-stderr-buffer nil #[ni
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test p
  ert-run-tests(t #[385 "\306 \307\"\203G \211\211G\310U\203 \211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit()
  command-line-1(("-l" "ert" "-l" "process-tests.elc" "-f" "ert-run-te
  command-line()
  normal-top-level()
Test process-test-stderr-buffer condition:
    (ert-test-failed
     ((should
       (with-current-buffer stderr-buffer
         (goto-char ...)
         (looking-at "hello stderr!")))
      :form
      (save-current-buffer
        (set-buffer stderr-buffer)
        (goto-char
         (point-min))
        (looking-at "hello stderr!"))
      :value nil))
   FAILED  4/5  process-test-stderr-buffer
   passed  5/5  process-test-stderr-filter

Ran 5 tests, 4 results as expected, 1 unexpected (2015-10-31 23:35:47+0100)

1 unexpected results:
   FAILED  process-test-stderr-buffer


C:\...\automated> ..\..\src\emacs.exe -Q -batch -l ert -l process-tests.elc
-f ert-run-tests-batch-and-exit
Running 5 tests (2015-10-31 23:34:42+0100)
   passed  1/5  process-test-quoted-batfile
   passed  2/5  process-test-sentinel-accept-process-output
   passed  3/5  process-test-sentinel-sit-for
   passed  4/5  process-test-stderr-buffer
Test process-test-stderr-filter backtrace:
  #[nil "\306\307C \310 \311 \3121 \313\216\314      \"\211 )0\202 \210
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test process-test-stderr-filter nil #[ni
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test p
  ert-run-tests(t #[385 "\306 \307\"\203G \211\211G\310U\203 \211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit()
  command-line-1(("-l" "ert" "-l" "process-tests.elc" "-f" "ert-run-te
  command-line()
  normal-top-level()
Test process-test-stderr-filter condition:
    (ert-test-failed
     ((should stderr-sentinel-called)
      :form stderr-sentinel-called :value nil))
   FAILED  5/5  process-test-stderr-filter

Ran 5 tests, 4 results as expected, 1 unexpected (2015-10-31 23:34:43+0100)

1 unexpected results:
   FAILED  process-test-stderr-filter

[-- Attachment #2: Type: text/html, Size: 6809 bytes --]

bug#18745:

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Sun, 1 Nov 2015 00:20:34 +0100
> Cc: 18745@debbugs.gnu.org
> 
> > If it still fails, I think making the test more tolerant of the
> > trailing whitespace is IMO a better alternative than letting it fail.
> > After all, that trailing space is not what we are testing there.
> 
> Something like this, you mean?

Yes, thanks.

> BTW, curiously, the stderr-related tests in the same test file
> (process-test-stderr-buffer and process-test-stderr-filter) fail sometimes.
> Much more frequently on TCC than CMD, but they also fail in CMD, as in these
> examples:

It doesn't fail for me, even if I run the test many times.

bug#18745:

[Juanma Barranquero]

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

On Sun, Nov 1, 2015 at 6:49 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> Yes, thanks.

OK, thanks. Installed. I'm re-closing this thread.

> It doesn't fail for me, even if I run the test many times.

That's interesting. I'll look into it.

[-- Attachment #2: Type: text/html, Size: 419 bytes --]