Re: running with -jN on Windows

[Eli Zaretskii]

> Date: Wed, 4 Jan 2006 14:51:45 -0500
> Cc: address@hidden, address@hidden
> From: "Paul D. Smith" <address@hidden>
> 
> The shell (at least in UNIX) is only used in the slow path.  The fast
> path does a direct exec() (or CreateProcess() in Windows?)  I'm assuming
> that direct invocation preserves pipes.
> 
> I just mean that there are other requirements on the operating system
> itself, such as proper support for pipes, signal handling, etc.
> 
> But I think the only requirements on the shell are that it not interfere
> with the pipes that the parent make passes to its child makes.
> 
>   ez> Anyway, if file handle inheritance is the only requirement on
>   ez> Posix systems that allows -jN, then the proposed patch does
>   ez> nothing in this area on Windows.  I guess it is dealing with some
>   ez> Windows-specific aspect of Make, perhaps the invocation via batch
>   ez> files.
> 
> Yes, that might be an issue.

I looked at this, and I see no problem to enable -jN on Windows even
if a Unixy shell is not available.  The suggested patches are attached
below; besides removing the forceful limitation to -j1, they also
modify job.c to avoid a small window of possible race condition
between creating a temporary batch file and using it to write the
batch script into it.

I tested the (MinGW build of) ported Make thus patched using the tests
in the `parallelism' script from the test suite.  To test them with
cmd.exe, I needed to replace ";" with "&&" in multi-line commands of
the Makefiles, because that is how cmd.exe supports several commands
on the same line.

With this change, all the tests but one passed on Windows with cmd.exe
and produced the expected results.  To be on the safe side, I also ran
Make with --debug=j and looked at the output: I didn't notice anything
that would indicate some kind of trouble.

The only test that didn't work as the test suite expected was the one
that first recurses.  It didn't work for both flavors of the shell, so
the problem is not related to the shell.  Here's the Makefile it
creates for the Unixy shell:

    recurse: ; @$(MAKE) --no-print-directory -f tmp.mkf INC=yes all
    all: 1 2; @echo success

    INC = no
    ifeq ($(INC),yes)
    -include 1.inc 2.inc
    endif

    1.inc: ; @echo ONE.inc; sleep 2; echo TWO.inc; echo '1: ; @echo ONE; sleep 
2; echo TWO' > $@
    2.inc: ; @sleep 1; echo THREE.inc; echo '2: ; @sleep 1; echo THREE' > $@

The required output, when Make is invoked with -j4, is:

    ONE.inc
    THREE.inc
    TWO.inc
    ONE
    THREE
    TWO
    success

What I get instead (no matter what shell I use) is this:

    THREE.inc 
    ONE.inc 
    TWO.inc 
    ONE 
    TWO 
    THREE 
    success

I think this might be related to the fact that the Windows port does
not support MAKE_JOBSERVER.  Paul, could that be the reason? is this
test supposed to work without the job server?

Something else possibly related is this: I added to the Makefile a
command to print the value of MAKEFLAGS.  On a GNU/Linux machine, the
value I get is:

  --no-print-directory - --jobserver-fds=3,4 -j - -- INC=yes

However, on Windows, I get this instead:

  --no-print-directory -- INC=yes

Note the absence of -j.  If I add "-j" to the Make invocation command
in the `recurse' target, the test succeeds, i.e. it produces the
expected results, both with sh.exe and with cmd.exe!  In addition, if
I add "-j4" instead of "-j", the sub-Make receives "-j 1", which
surprised me.

Is all this expected when MAKE_JOBSERVER is not supported, or is this
due to bug(s) that I need to look for?

Finally, here are the patches to allow -jN without a Unixy shell; I
hope it's not too late to include them in the coming release:


2006-01-14  Eli Zaretskii  <address@hidden>

        * main.c (main) [WINDOWS32]: Don't refuse to run with -jN, even if
        the shell is not sh.exe.

        * job.c (create_batch_file): Renamed from create_batch_filename;
        all callers changed.  Don't close the temporary file; return its
        file descriptor instead.  New arg FD allows to return the file
        descriptor.
        (construct_command_argv_internal): Use _fdopen instead of fopen to
        open the batch file.


--- main.c~1    2005-12-17 14:31:39.375000000 +0200
+++ main.c      2006-01-14 13:44:27.843750000 +0200
@@ -1595,12 +1595,6 @@ main (int argc, char **argv, char **envp
   /* look one last time after reading all Makefiles */
   if (no_default_sh_exe)
     no_default_sh_exe = !find_and_set_default_shell(NULL);
-
-  if (no_default_sh_exe && job_slots != 1) {
-    error (NILF, _("Do not specify -j or --jobs if sh.exe is not available."));
-    error (NILF, _("Resetting make for single job mode."));
-    job_slots = 1;
-  }
 #endif /* WINDOWS32 */
 
 #if defined (__MSDOS__) || defined (__EMX__)

--- job.c~1     2005-12-17 14:34:20.796875000 +0200
+++ job.c       2006-01-14 14:00:00.921875000 +0200
@@ -242,11 +242,12 @@ w32_kill(int pid, int sig)
   return ((process_kill((HANDLE)pid, sig) == TRUE) ? 0 : -1);
 }
 
-/* This function creates a temporary file name with the given extension
- * the unixy param controls both the extension and the path separator
- * return an xmalloc'ed string of a newly created temp file or die.  */
+/* This function creates a temporary file name with an extension specified
+ * by the unixy arg.
+ * Return an xmalloc'ed string of a newly created temp file and its
+ * file descriptor, or die.  */
 static char *
-create_batch_filename(char const *base, int unixy)
+create_batch_file (char const *base, int unixy, int *fd)
 {
   const char *const ext = unixy ? "sh" : "bat";
   const char *error = NULL;
@@ -304,7 +305,7 @@ create_batch_filename(char const *base, 
           const unsigned final_size = path_size + size + 1;
           char *const path = (char *) xmalloc (final_size);
           memcpy (path, temp_path, final_size);
-          CloseHandle (h);
+          *fd = _open_osfhandle ((long)h, 0);
           if (unixy)
             {
               char *p;
@@ -317,6 +318,7 @@ create_batch_filename(char const *base, 
         }
     }
 
+  *fd = -1;
   if (error == NULL)
     error = _("Cannot create a temporary file\n");
   fatal (NILF, error);
@@ -2755,19 +2757,21 @@ construct_command_argv_internal (char *l
        command line (e.g. Cygnus GNUWIN32 sh.exe on WIN32 systems).  In these
        cases, run commands via a script file.  */
     if ((no_default_sh_exe || batch_mode_shell) && batch_filename_ptr) {
+      int temp_fd;
       FILE* batch = NULL;
       int id = GetCurrentProcessId();
       PATH_VAR(fbuf);
 
       /* create a file name */
       sprintf(fbuf, "make%d", id);
-      *batch_filename_ptr = create_batch_filename (fbuf, unixy_shell);
+      *batch_filename_ptr = create_batch_file (fbuf, unixy_shell, &temp_fd);
 
       DB (DB_JOBS, (_("Creating temporary batch file %s\n"),
                     *batch_filename_ptr));
 
-      /* create batch file to execute command */
-      batch = fopen (*batch_filename_ptr, "w");
+      /* Create a FILE object for the batch file, and write to it the
+        commands to be executed.  */
+      batch = _fdopen (temp_fd, "w");
       if (!unixy_shell)
         fputs ("@echo off\n", batch);
       fputs (command_ptr, batch);

--- README.W32~ 2005-12-13 18:47:25.000000000 +0200
+++ README.W32  2006-01-14 18:08:44.875000000 +0200
@@ -90,14 +90,11 @@
        To work around this difficulty, this version of make supports
        a batch mode.  When BATCH_MODE_ONLY_SHELL is defined at compile
        time, make forces all command lines to be executed via script
-       files instead of by command line.  In this mode you must have a
-       working sh.exe in order to use parallel builds (-j).
+       files instead of by command line.
 
        A native Windows32 system with no Bourne shell will also run
        in batch mode.  All command lines will be put into batch files
-       and executed via $(COMSPEC) (%COMSPEC%).  Note that parallel
-        builds (-j) require a working Bourne shell; they will not work
-        with COM.
+       and executed via $(COMSPEC) (%COMSPEC%).
 
 GNU make and Cygnus GNU Windows32 tools:
 

--- NEWS~       2005-12-17 19:36:03.234375000 +0200
+++ NEWS        2006-01-14 18:10:40.609375000 +0200
@@ -106,6 +106,10 @@
   the DOS command interpreter in batch mode even if a UNIX-like shell
   could be found on the system.
 
+* Parallel builds (-jN) no longer require a working Bourne shell on
+  MS-Windows.  They work even with the stock Windows shells, such as
+  cmd.exe and command.com.
+
 * Updated to autoconf 2.59, automake 1.9.5, and gettext 0.14.1.  Users
   should not be impacted.