Re: Bug: make fails to execute .BAT files with space in the path, with SHELL=cmd.exe

[Eli Zaretskii]

> Date: Mon, 29 Apr 2013 05:40:25 +0300
> From: Eli Zaretskii <address@hidden>
> Cc: address@hidden
> 
> When invoking commands through cmd.exe, the problem disappears because
> cmd.exe's name does not include whitespace.  As I wrote, the problem
> only happens when _both_ the command and its arguments are quited and
> include whitespace.  I'm not yet sure why this triggers the problem.

Further investigation reveals that the problem happens only with .bat
and .cmd files; if you replace "a b.bat" with some "a b.exe" (I used a
simple program that just echoes its argv[] array), all of your
examples will suddenly work.

Based on some extensive testing and the corresponding reactions, I
think that what happens is that CreateProcess invokes cmd.exe to run
the batch file, and while doing so, it quotes the command line.  When
the original command already has quotes, the additional quoting by
CreateProcess is buggy (or maybe it's trying to solve a problem that
is unsolvable within its constraints), and the result sometimes
utterly confuses cmd.exe.  Moreover, at least in some cases, the 1st
argument to CreateProcess (which is the full absolute file name of the
batch file) is completely ignored, and instead CreateProcess lets
cmd.exe to intuit the batch name from the command line: the 'a' which
it is complaining about is the beginning of the command line, not the
first letter of the batch file name.

Anyway, can you please try the patch below?  It detects the situation
which triggers this bug, and passes NULL as the first argument to
CreateProcess.  If this gives good results, I will install it in the
repository.  (For best results, I suggest to use the latest
development sources, since that's where I tested these changes.)


--- w32/subproc/sub_proc.c~0    2013-04-28 06:42:01.000000000 +0300
+++ w32/subproc/sub_proc.c      2013-04-29 11:47:37.944075900 +0300
@@ -23,6 +23,7 @@ this program.  If not, see <http://www.g
 #else
 # include <stdint.h>
 #endif
+#include <string.h>
 #include <process.h>  /* for msvc _beginthreadex, _endthreadex */
 #include <signal.h>
 #include <windows.h>
@@ -540,6 +541,26 @@ find_file(const char *exec_path, const c
        return INVALID_HANDLE_VALUE;
 }
 
+/*
+ * Return non-zero of FNAME specifies a batch file and its name
+ * includes embedded whitespace.
+ */
+
+static int
+batch_file_with_spaces(const char *fname)
+{
+       size_t fnlen = strlen(fname);
+
+       return (fnlen > 4
+               && (_strnicmp(fname + fnlen - 4, ".bat", 4) == 0
+                   || _strnicmp(fname + fnlen - 4, ".cmd", 4) == 0)
+               /* The set of characters in the 2nd arg to strpbrk
+                  should be the same one used by make_command_line
+                  below to decide whether an argv[] element needs
+                  quoting.  */
+               && strpbrk(fname, " \t") != NULL);
+}
+
 
 /*
  * Description:   Create the child process to be helped
@@ -570,6 +591,7 @@ process_begin(
        STARTUPINFO startInfo;
        PROCESS_INFORMATION procInfo;
        char *envblk=NULL;
+       int pass_null_exec_path = 0;
 
        /*
         *  Shell script detection...  if the exec_path starts with #! then
@@ -651,8 +673,27 @@ process_begin(
 
        if (file_not_found)
                command_line = make_command_line( shell_name, exec_path, argv);
-       else
+       else {
+               /* If exec_fname includes whitespace, CreateProcess
+                  behaves erratically and unreliably, and often fails
+                  if argv[0] also includes whitespace (and thus will
+                  be quoted by make_command_line below).  So in that
+                  case, we don't pass exec_fname as the 1st arg to
+                  CreateProcess, but instead replace argv[0] with
+                  exec_fname (to keep its leading directories and
+                  extension as found by find_file), and pass NULL to
+                  CreateProcess as its 1st arg.  This works around
+                  the bugs in CreateProcess, which are probably
+                  caused by its passing the command to cmd.exe with
+                  some incorrect quoting.  */
+               if (!shell_name
+                   && batch_file_with_spaces(exec_fname)
+                   && _stricmp(exec_path, argv[0]) == 0) {
+                       pass_null_exec_path = 1;
+                       argv[0] = exec_fname;
+               }
                command_line = make_command_line( shell_name, exec_fname, argv);
+       }
 
        if ( command_line == NULL ) {
                pproc->last_err = 0;
@@ -669,7 +710,7 @@ process_begin(
                }
        }
 
-       if ((shell_name) || (file_not_found)) {
+       if (shell_name || file_not_found || pass_null_exec_path) {
                exec_path = 0;  /* Search for the program in %Path% */
        } else {
                exec_path = exec_fname;