Final fixes for obscure output-sync errors.

4 files changed, 72 insertions, 62 deletions

diff --git a/ChangeLog b/ChangeLog
index e21b2c6..c70cd45 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2013-09-29  Paul Smith  <psmith@gnu.org>
 
+	* output.c (output_dump): Always write Enter/Leave messages to stdio.
+	(log_working_directory): This now always writes to stdio, so we
+	don't need the struct output parameter anymore.
+	(output_start): Show the working directory when output_sync is not
+	set or is recursive.
+	* main.c (main): Ensure the special "already shown Enter message"
+	token is removed from MAKE_RESTARTS before the user can see it.
+	* function.c (func_shell_base): If the output_context stderr
+	exists but is invalid, write to the real stderr.
+	Fixes suggested by Frank Heckenbach <f.heckenbach@fh-soft.de>.
+
 	* output.c: Guard unistd.h inclusion, add io.h.
 	* gnumake.h: Move GMK_EXPORT before the declarations.
 	* make_msvc_net2003.vcproj: Add missing files.
diff --git a/function.c b/function.c
index b8b9cf3..ce60ef5 100644
--- a/function.c
+++ b/function.c
@@ -1620,7 +1620,7 @@ char *
 func_shell_base (char *o, char **argv, int trim_newlines)
 {
   char *batch_filename = NULL;
-
+  int errfd;
 #ifdef __MSDOS__
   FILE *fpipe;
 #endif
@@ -1636,9 +1636,9 @@ func_shell_base (char *o, char **argv, int trim_newlines)
      are used to run the commands, because we normally refrain from
      creating batch files under -n.  */
   int j_p_f = just_print_flag;
-
   just_print_flag = 0;
 #endif
+
   /* Construct the argument list.  */
   command_argv = construct_command_argv (argv[0], NULL, NULL, 0,
                                          &batch_filename);
@@ -1655,7 +1655,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
        export var = $(shell echo foobie)
        bad := $(var)
      because target_environment hits a loop trying to expand $(var) to put it
-     in the environment.  This is even more confusing when var was not
+     in the environment.  This is even more confusing when 'var' was not
      explicitly exported, but just appeared in the calling environment.
 
      See Savannah bug #10593.
@@ -1678,6 +1678,9 @@ func_shell_base (char *o, char **argv, int trim_newlines)
   /* Set up the output in case the shell writes something.  */
   output_start ();
 
+  errfd = (output_context && output_context->err >= 0
+           ? output_context->err : FD_STDERR);
+
 #if defined(__MSDOS__)
   fpipe = msdos_openpipe (pipedes, &pid, argv[0]);
   if (pipedes[0] < 0)
@@ -1710,7 +1713,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
   CLOSE_ON_EXEC(pipedes[1]);
   CLOSE_ON_EXEC(pipedes[0]);
   /* Never use fork()/exec() here! Use spawn() instead in exec_command() */
-  pid = child_execute_job (FD_STDIN, pipedes[1], FD_STDERR, command_argv, envp);
+  pid = child_execute_job (FD_STDIN, pipedes[1], errfd, command_argv, envp);
   if (pid < 0)
     perror_with_name (error_prefix, "spawn");
 # else /* ! __EMX__ */
@@ -1724,9 +1727,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
       if (stack_limit.rlim_cur)
        setrlimit (RLIMIT_STACK, &stack_limit);
 #  endif
-      child_execute_job (FD_STDIN, pipedes[1],
-                         output_context ? output_context->err : FD_STDERR,
-                         command_argv, envp);
+      child_execute_job (FD_STDIN, pipedes[1], errfd, command_argv, envp);
     }
   else
 # endif
diff --git a/main.c b/main.c
index 13ded20..7069669 100644
--- a/main.c
+++ b/main.c
@@ -1295,64 +1295,62 @@ main (int argc, char **argv, char **envp)
 
     for (i = 0; envp[i] != 0; ++i)
       {
-        int do_not_define = 0;
+        struct variable *v;
         char *ep = envp[i];
+        /* By default, export all variables culled from the environment.  */
+        enum variable_export export = v_export;
+        unsigned int len;
 
         while (! STOP_SET (*ep, MAP_EQUALS))
           ++ep;
+
+        /* If there's no equals sign it's a malformed environment.  Ignore.  */
+        if (*ep == '\0')
+          continue;
+
 #ifdef WINDOWS32
         if (!unix_path && strneq (envp[i], "PATH=", 5))
           unix_path = ep+1;
         else if (!strnicmp (envp[i], "Path=", 5))
           {
-            do_not_define = 1; /* it gets defined after loop exits */
             if (!windows32_path)
               windows32_path = ep+1;
+            /* PATH gets defined after the loop exits.  */
+            continue;
           }
 #endif
-        /* The result of pointer arithmetic is cast to unsigned int for
-           machines where ptrdiff_t is a different size that doesn't widen
-           the same.  */
-        if (!do_not_define)
+
+        /* Length of the variable name, and skip the '='.  */
+        len = ep++ - envp[i];
+
+        /* If this is MAKE_RESTARTS, check to see if the "already printed
+           the enter statement" flag is set.  */
+        if (len == 13 && strneq (envp[i], "MAKE_RESTARTS", 13))
           {
-            struct variable *v;
-
-            v = define_variable (envp[i], (unsigned int) (ep - envp[i]),
-                                 ep + 1, o_env, 1);
-            /* Force exportation of every variable culled from the
-               environment.  We used to rely on target_environment's
-               v_default code to do this.  But that does not work for the
-               case where an environment variable is redefined in a makefile
-               with 'override'; it should then still be exported, because it
-               was originally in the environment.  */
-            v->export = v_export;
-
-            /* Another wrinkle is that POSIX says the value of SHELL set in
-               the makefile won't change the value of SHELL given to
-               subprocesses.  */
-            if (streq (v->name, "SHELL"))
+            if (*ep == '-')
               {
-#ifndef __MSDOS__
-                v->export = v_noexport;
-#endif
-                shell_var.name = "SHELL";
-                shell_var.length = 5;
-                shell_var.value = xstrdup (ep + 1);
+                OUTPUT_TRACED ();
+                ++ep;
               }
+            restarts = (unsigned int) atoi (ep);
+            export = v_noexport;
+          }
 
-            /* If MAKE_RESTARTS is set, remember it but don't export it.
-               If it's negative, it means the "enter" message was printed.  */
-            else if (streq (v->name, "MAKE_RESTARTS"))
-              {
-                v->export = v_noexport;
-                if (*(++ep) == '-')
-                  {
-                    OUTPUT_TRACED ();
-                    ++ep;
-                  }
-                restarts = (unsigned int) atoi (ep);
-              }
+        v = define_variable (envp[i], len, ep, o_env, 1);
+
+        /* POSIX says the value of SHELL set in the makefile won't change the
+           value of SHELL given to subprocesses.  */
+        if (streq (v->name, "SHELL"))
+          {
+#ifndef __MSDOS__
+            export = v_noexport;
+#endif
+            shell_var.name = "SHELL";
+            shell_var.length = 5;
+            shell_var.value = xstrdup (ep);
           }
+
+        v->export = export;
       }
   }
 #ifdef WINDOWS32
diff --git a/output.c b/output.c
index bb351d3..fa757b0 100644
--- a/output.c
+++ b/output.c
@@ -96,10 +96,8 @@ _outputs (struct output *out, int is_err, const char *msg)
       while (1)
         {
           EINTRLOOP (r, write (fd, msg, len));
-          if (r == len)
+          if (r == len || r <= 0)
             break;
-          if (r <= 0)
-            return;
           len -= r;
           msg += r;
         }
@@ -110,7 +108,7 @@ _outputs (struct output *out, int is_err, const char *msg)
    left (according to ENTERING) the current directory.  */
 
 static int
-log_working_directory (struct output *out, int entering)
+log_working_directory (int entering)
 {
   static char *buf = NULL;
   static unsigned int len = 0;
@@ -172,7 +170,7 @@ log_working_directory (struct output *out, int entering)
   else
     sprintf (p, fmt, program, makelevel, starting_directory);
 
-  _outputs (out, 0, buf);
+  _outputs (NULL, 0, buf);
 
   return 1;
 }
@@ -391,7 +389,7 @@ output_dump (struct output *out)
 
       /* Log the working directory for this dump.  */
       if (print_directory_flag && output_sync != OUTPUT_SYNC_RECURSE)
-        traced = log_working_directory (output_context, 1);
+        traced = log_working_directory (1);
 
       if (outfd_not_empty)
         pump_from_tmp (out->out, stdout);
@@ -399,7 +397,7 @@ output_dump (struct output *out)
         pump_from_tmp (out->err, stderr);
 
       if (traced)
-        log_working_directory (output_context, 0);
+        log_working_directory (0);
 
       /* Exit the critical section.  */
       if (sem)
@@ -562,7 +560,7 @@ output_close (struct output *out)
   if (! out)
     {
       if (stdio_traced)
-        log_working_directory (NULL, 0);
+        log_working_directory (0);
       return;
     }
 
@@ -583,15 +581,17 @@ void
 output_start ()
 {
 #ifndef NO_OUTPUT_SYNC
-  if (output_context && output_context->syncout && ! OUTPUT_ISSET(output_context))
-    setup_tmpfile (output_context);
+  /* If we're syncing output make sure the temporary file is set up.  */
+  if (output_context && output_context->syncout)
+    if (! OUTPUT_ISSET(output_context))
+      setup_tmpfile (output_context);
 #endif
 
-  if (! output_context || output_sync == OUTPUT_SYNC_RECURSE)
-    {
-      if (! stdio_traced && print_directory_flag)
-        stdio_traced = log_working_directory (NULL, 1);
-    }
+  /* If we're not syncing this output per-line or per-target, make sure we emit
+     the "Entering..." message where appropriate.  */
+  if (output_sync == OUTPUT_SYNC_NONE || output_sync == OUTPUT_SYNC_RECURSE)
+    if (! stdio_traced && print_directory_flag)
+      stdio_traced = log_working_directory (1);
 }
 
 void