Support the -Orecurse option properly.

In this mode we still collect all the output from a given target and
dump it at once.  However we don't treat recursive lines any differently
from non-recursive lines.  Also we don't print enter/leave messages
after every dump.  However we do ensure that we always print them once
to stdout, so the parent make will collect it properly.

ChangeLog

@@ -1,3 +1,19 @@
+2013-09-14  Paul Smith  <psmith@gnu.org>
+
+	* misc.c (set_append_mode, open_tmpfd, open_tmpfile): Move to output.c.
+	* misc.h: Ditto.
+	* output.h: Ditto.
+	* main.c (main): Move stdio init into output.c:output_init().
+	Change open_tmpfile() to output_tmpfile().
+	* output.c: Rename open_*() to output_*().  set_append_mode() and
+	open_tmpfd() are static.
+	(_outputs, log_working_directory): Accept a struct output and
+	print to that rather than the global context.
+	(output_dump): In recurse mode print enter/leave once for the
+	whole makefile.
+	(output_init): Initialize this processes stdio as well as child's.
+	* vmsjobs.c: Reformat to be closer to convention.
+
 2013-09-12  Paul Smith  <psmith@gnu.org>
 
 	Rework output to handle synchronization and directory logging more

function.c

@@ -1846,7 +1846,7 @@ func_shell_base (char *o, char **argv, int trim_newlines)
     return o;
 
   /* Note the mktemp() is a security hole, but this only runs on Amiga.
-     Ideally we would use main.c:open_tmpfile(), but this uses a special
+     Ideally we would use output_tmpfile(), but this uses a special
      Open(), not fopen(), and I'm not familiar enough with the code to mess
      with it.  */
   strcpy (tmp_output, "t:MakeshXXXXXXXX");

job.c

@@ -1738,7 +1738,7 @@ new_job (struct file *file)
      'struct child', and add that to the chain.  */
 
   c = xcalloc (sizeof (struct child));
-  output_init (&c->output, output_sync);
+  output_init (&c->output);
 
   c->file = file;
   c->sh_batch_file = NULL;

main.c

@@ -1154,24 +1154,7 @@ main (int argc, char **argv, char **envp)
 # endif
 #endif
 
-  /* Make sure stdout is line-buffered.  */
+  output_init (NULL);
-
-#ifdef HAVE_SETVBUF
-# ifdef SETVBUF_REVERSED
-  setvbuf (stdout, _IOLBF, xmalloc (BUFSIZ), BUFSIZ);
-# else  /* setvbuf not reversed.  */
-  /* Some buggy systems lose if we pass 0 instead of allocating ourselves.  */
-  setvbuf (stdout, 0, _IOLBF, BUFSIZ);
-# endif /* setvbuf reversed.  */
-#elif HAVE_SETLINEBUF
-  setlinebuf (stdout);
-#endif  /* setlinebuf missing.  */
-
-  /* Configure stdout/stderr to be in append mode.
-     This keeps parallel jobs from losing output due to overlapping writes.  */
-
-  set_append_mode (fileno (stdout));
-  set_append_mode (fileno (stderr));
 
   /* Figure out where this program lives.  */
 
@@ -1418,16 +1401,16 @@ main (int argc, char **argv, char **envp)
   /* Set always_make_flag if -B was given and we've not restarted already.  */
   always_make_flag = always_make_set && (restarts == 0);
 
-  /* Print version information.  */
+  /* Print version information, and exit.  */
-  if (print_version_flag || ISDB (DB_BASIC))
+  if (print_version_flag)
     {
       print_version ();
-
-      /* 'make --version' is supposed to just print the version and exit.  */
-      if (print_version_flag)
       die (0);
     }
 
+  if (ISDB (DB_BASIC))
+    print_version ();
+
 #ifndef VMS
   /* Set the "MAKE_COMMAND" variable to the name we were invoked with.
      (If it is a relative pathname with a slash, prepend our directory name
@@ -1670,7 +1653,7 @@ main (int argc, char **argv, char **envp)
 #endif /* !HAVE_DOS_PATHS */
 
             strcat (template, DEFAULT_TMPFILE);
-            outfile = open_tmpfile (&stdin_nm, template);
+            outfile = output_tmpfile (&stdin_nm, template);
             if (outfile == 0)
               pfatal_with_name (_("fopen (temporary file)"));
             while (!feof (stdin) && ! ferror (stdin))

makeint.h

@@ -449,9 +449,6 @@ int alpha_compare (const void *, const void *);
 void print_spaces (unsigned int);
 char *find_percent (char *);
 const char *find_percent_cached (const char **);
-void set_append_mode (int);
-int open_tmpfd (void);
-FILE *open_tmpfile (char **, const char *);
 
 #ifndef NO_ARCHIVES
 int ar_name (const char *);

misc.c

@@ -704,7 +704,7 @@ child_access (void)
 
 #endif  /* GETLOADAVG_PRIVILEGED */
 }
-
+
 #ifdef NEED_GET_PATH_MAX
 unsigned int
 get_path_max (void)
@@ -725,100 +725,6 @@ get_path_max (void)
 #endif
 
 
-/* Set a file descriptor to be in O_APPEND mode.
-   If it fails, just ignore it.  */
-
-void
-set_append_mode (int fd)
-{
-#if defined(F_GETFL) && defined(F_SETFL) && defined(O_APPEND)
-  int flags = fcntl (fd, F_GETFL, 0);
-  if (flags >= 0)
-    fcntl (fd, F_SETFL, flags | O_APPEND);
-#endif
-}
-
-/* Provide support for temporary files.  */
-
-#ifndef HAVE_STDLIB_H
-# ifdef HAVE_MKSTEMP
-int mkstemp (char *template);
-# else
-char *mktemp (char *template);
-# endif
-#endif
-
-/* This is only used by output-sync, and it may not be portable.  */
-#ifdef OUTPUT_SYNC
-
-/* Returns a file descriptor to a temporary file.  The file is automatically
-   closed/deleted on exit.  Don't use a FILE* stream.  */
-int
-open_tmpfd ()
-{
-  int fd = -1;
-  FILE *tfile = tmpfile ();
-
-  if (! tfile)
-    pfatal_with_name ("tmpfile");
-
-  /* Create a duplicate so we can close the stream.  */
-  fd = dup (fileno (tfile));
-  if (fd < 0)
-    pfatal_with_name ("dup");
-
-  fclose (tfile);
-
-  set_append_mode (fd);
-
-  return fd;
-}
-
-#endif
-
-FILE *
-open_tmpfile (char **name, const char *template)
-{
-#ifdef HAVE_FDOPEN
-  int fd;
-#endif
-
-#if defined HAVE_MKSTEMP || defined HAVE_MKTEMP
-# define TEMPLATE_LEN   strlen (template)
-#else
-# define TEMPLATE_LEN   L_tmpnam
-#endif
-  *name = xmalloc (TEMPLATE_LEN + 1);
-  strcpy (*name, template);
-
-#if defined HAVE_MKSTEMP && defined HAVE_FDOPEN
-  /* It's safest to use mkstemp(), if we can.  */
-  fd = mkstemp (*name);
-  if (fd == -1)
-    return 0;
-  return fdopen (fd, "w");
-#else
-# ifdef HAVE_MKTEMP
-  (void) mktemp (*name);
-# else
-  (void) tmpnam (*name);
-# endif
-
-# ifdef HAVE_FDOPEN
-  /* Can't use mkstemp(), but guard against a race condition.  */
-  fd = open (*name, O_CREAT|O_EXCL|O_WRONLY, 0600);
-  if (fd == -1)
-    return 0;
-  return fdopen (fd, "w");
-# else
-  /* Not secure, but what can we do?  */
-  return fopen (*name, "w");
-# endif
-#endif
-}
-
-
-
 /* This code is stolen from gnulib.
    If/when we abandon the requirement to work with K&R compilers, we can
    remove this (and perhaps other parts of GNU make!) and migrate to using

output.c

@@ -60,9 +60,9 @@ msc_vsnprintf(char *str, size_t size, const char *format, va_list ap)
 
 /* Write a string to the current STDOUT or STDERR.  */
 static void
-_outputs (int is_err, const char *msg)
+_outputs (struct output *out, int is_err, const char *msg)
 {
-  if (! output_context || ! output_context->syncout)
+  if (! out || ! out->syncout)
     {
       FILE *f = is_err ? stderr : stdout;
       fputs (msg, f);
@@ -70,7 +70,7 @@ _outputs (int is_err, const char *msg)
     }
   else
     {
-      int fd = is_err ? output_context->err : output_context->out;
+      int fd = is_err ? out->err : out->out;
       int len = strlen (msg);
       int r;
 
@@ -92,7 +92,7 @@ _outputs (int is_err, const char *msg)
    left (according to ENTERING) the current directory.  */
 
 static int
-log_working_directory (int entering)
+log_working_directory (struct output *out, int entering)
 {
   static char *buf = NULL;
   static unsigned int len = 0;
@@ -100,10 +100,6 @@ log_working_directory (int entering)
   const char *fmt;
   char *p;
 
-  /* Only print if directory logging is enabled.  */
-  if (entering && ! print_directory_flag)
-    return 0;
-
   /* Get enough space for the longest possible output.  */
   need = strlen (program) + INTEGER_LENGTH + 2 + 1;
   if (starting_directory)
@@ -158,10 +154,23 @@ log_working_directory (int entering)
   else
     sprintf (p, fmt, program, makelevel, starting_directory);
 
-  _outputs (0, buf);
+  _outputs (out, 0, buf);
 
   return 1;
 }
+
+/* Set a file descriptor to be in O_APPEND mode.
+   If it fails, just ignore it.  */
+
+static void
+set_append_mode (int fd)
+{
+#if defined(F_GETFL) && defined(F_SETFL) && defined(O_APPEND)
+  int flags = fcntl (fd, F_GETFL, 0);
+  if (flags >= 0)
+    fcntl (fd, F_SETFL, flags | O_APPEND);
+#endif
+}
 
 
 #ifdef OUTPUT_SYNC
@@ -277,6 +286,73 @@ release_semaphore (void *sem)
     perror ("fcntl()");
 }
 
+/* Returns a file descriptor to a temporary file.  The file is automatically
+   closed/deleted on exit.  Don't use a FILE* stream.  */
+int
+output_tmpfd ()
+{
+  int fd = -1;
+  FILE *tfile = tmpfile ();
+
+  if (! tfile)
+    pfatal_with_name ("tmpfile");
+
+  /* Create a duplicate so we can close the stream.  */
+  fd = dup (fileno (tfile));
+  if (fd < 0)
+    pfatal_with_name ("dup");
+
+  fclose (tfile);
+
+  set_append_mode (fd);
+
+  return fd;
+}
+
+/* Adds file descriptors to the child structure to support output_sync; one
+   for stdout and one for stderr as long as they are open.  If stdout and
+   stderr share a device they can share a temp file too.
+   Will reset output_sync on error.  */
+static void
+setup_tmpfile (struct output *out)
+{
+  /* Is make's stdout going to the same place as stderr?  */
+  static int combined_output = -1;
+
+  if (combined_output < 0)
+    combined_output = sync_init ();
+
+  if (STREAM_OK (stdout))
+    {
+      int fd = output_tmpfd ();
+      if (fd < 0)
+        goto error;
+      CLOSE_ON_EXEC (fd);
+      out->out = fd;
+    }
+
+  if (STREAM_OK (stderr))
+    {
+      if (out->out != OUTPUT_NONE && combined_output)
+        out->err = out->out;
+      else
+        {
+          int fd = output_tmpfd ();
+          if (fd < 0)
+            goto error;
+          CLOSE_ON_EXEC (fd);
+          out->err = fd;
+        }
+    }
+
+  return;
+
+  /* If we failed to create a temp file, disable output sync going forward.  */
+ error:
+  output_close (out);
+  output_sync = 0;
+}
+
 /* Synchronize the output of jobs in -j mode to keep the results of
    each job together. This is done by holding the results in temp files,
    one for stdout and potentially another for stderr, and only releasing
@@ -290,15 +366,15 @@ output_dump (struct output *out)
 
   if (outfd_not_empty || errfd_not_empty)
     {
-      int logged = 0;
+      int traced = 0;
 
       /* Try to acquire the semaphore.  If it fails, dump the output
          unsynchronized; still better than silently discarding it.  */
       void *sem = acquire_semaphore ();
 
-      /* Log the working directory, if we need to.  */
+      /* Log the working directory for this dump.  */
-      if (out->syncout)
+      if (print_directory_flag && output_sync != OUTPUT_SYNC_RECURSE)
-        logged = log_working_directory (1);
+        traced = log_working_directory (output_context, 1);
 
       /* We've entered the "critical section" during which a lock is held.  We
          want to keep it as short as possible.  */
@@ -307,8 +383,8 @@ output_dump (struct output *out)
       if (errfd_not_empty && out->err != out->out)
         pump_from_tmp (out->err, stderr);
 
-      if (logged)
+      if (traced)
-        log_working_directory (0);
+        log_working_directory (output_context, 0);
 
       /* Exit the critical section.  */
       if (sem)
@@ -329,58 +405,88 @@ output_dump (struct output *out)
         }
     }
 }
-
-/* Adds file descriptors to the child structure to support output_sync; one
-   for stdout and one for stderr as long as they are open.  If stdout and
-   stderr share a device they can share a temp file too.
-   Will reset output_sync on error.  */
-static void
-setup_tmpfile (struct output *out)
-{
-  /* Is make's stdout going to the same place as stderr?  */
-  static int combined_output = -1;
-
-  if (combined_output < 0)
-    combined_output = sync_init ();
-
-  if (STREAM_OK (stdout))
-    {
-      int fd = open_tmpfd ();
-      if (fd < 0)
-        goto error;
-      CLOSE_ON_EXEC (fd);
-      out->out = fd;
-    }
-
-  if (STREAM_OK (stderr))
-    {
-      if (out->out != OUTPUT_NONE && combined_output)
-        out->err = out->out;
-      else
-        {
-          int fd = open_tmpfd ();
-          if (fd < 0)
-            goto error;
-          CLOSE_ON_EXEC (fd);
-          out->err = fd;
-        }
-    }
-
-  return;
-
-  /* If we failed to create a temp file, disable output sync going forward.  */
- error:
-  output_close (out);
-  output_sync = 0;
-}
 #endif /* OUTPUT_SYNC */
 
 
+/* Provide support for temporary files.  */
+
+#ifndef HAVE_STDLIB_H
+# ifdef HAVE_MKSTEMP
+int mkstemp (char *template);
+# else
+char *mktemp (char *template);
+# endif
+#endif
+
+FILE *
+output_tmpfile (char **name, const char *template)
+{
+#ifdef HAVE_FDOPEN
+  int fd;
+#endif
+
+#if defined HAVE_MKSTEMP || defined HAVE_MKTEMP
+# define TEMPLATE_LEN   strlen (template)
+#else
+# define TEMPLATE_LEN   L_tmpnam
+#endif
+  *name = xmalloc (TEMPLATE_LEN + 1);
+  strcpy (*name, template);
+
+#if defined HAVE_MKSTEMP && defined HAVE_FDOPEN
+  /* It's safest to use mkstemp(), if we can.  */
+  fd = mkstemp (*name);
+  if (fd == -1)
+    return 0;
+  return fdopen (fd, "w");
+#else
+# ifdef HAVE_MKTEMP
+  (void) mktemp (*name);
+# else
+  (void) tmpnam (*name);
+# endif
+
+# ifdef HAVE_FDOPEN
+  /* Can't use mkstemp(), but guard against a race condition.  */
+  fd = open (*name, O_CREAT|O_EXCL|O_WRONLY, 0600);
+  if (fd == -1)
+    return 0;
+  return fdopen (fd, "w");
+# else
+  /* Not secure, but what can we do?  */
+  return fopen (*name, "w");
+# endif
+#endif
+}
+
+
 void
-output_init (struct output *out, unsigned int syncout)
+output_init (struct output *out)
+{
+  if (out)
     {
       out->out = out->err = OUTPUT_NONE;
-  out->syncout = !!syncout;
+      out->syncout = !!output_sync;
+      return;
+    }
+
+  /* Configure this instance of make.  Be sure stdout is line-buffered.  */
+
+#ifdef HAVE_SETVBUF
+# ifdef SETVBUF_REVERSED
+  setvbuf (stdout, _IOLBF, xmalloc (BUFSIZ), BUFSIZ);
+# else  /* setvbuf not reversed.  */
+  /* Some buggy systems lose if we pass 0 instead of allocating ourselves.  */
+  setvbuf (stdout, 0, _IOLBF, BUFSIZ);
+# endif /* setvbuf reversed.  */
+#elif HAVE_SETLINEBUF
+  setlinebuf (stdout);
+#endif  /* setlinebuf missing.  */
+
+  /* Force stdout/stderr into append mode.  This ensures parallel jobs won't
+     lose output due to overlapping writes.  */
+  set_append_mode (fileno (stdout));
+  set_append_mode (fileno (stderr));
 }
 
 void
@@ -389,7 +495,7 @@ output_close (struct output *out)
   if (! out)
     {
       if (stdio_traced)
-        log_working_directory (0);
+        log_working_directory (NULL, 0);
       return;
     }
 
@@ -402,46 +508,34 @@ output_close (struct output *out)
   if (out->err >= 0 && out->err != out->out)
     close (out->err);
 
-  output_init (out, 0);
+  output_init (out);
 }
 
-/* We're about to run a sub-process so ensure we've got our output set up.  */
+/* We're about to generate output: be sure it's set up.  */
 void
 output_start ()
 {
-  if (! output_context)
-    {
-      if (! stdio_traced)
-        stdio_traced = log_working_directory (1);
-    }
 #ifdef OUTPUT_SYNC
-  else if (output_context->syncout && ! OUTPUT_ISSET(output_context))
+  if (output_context && output_context->syncout && ! 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);
+    }
 }
 
 void
 outputs (int is_err, const char *msg)
 {
-  /* For stdio, an empty msg means we're about to invoke a shell command,
-     which may or may not generate output, so log the directory.  */
-  if (! output_context && ! stdio_traced)
-    stdio_traced = log_working_directory (1);
-
-  /* Don't bother to do anything with empty strings.  */
   if (! msg || *msg == '\0')
     return;
 
-#ifdef OUTPUT_SYNC
+  output_start ();
-  if (output_context)
-    {
-      /* Create a temporary file to write to, if necessary.  */
-      if (output_context->syncout && ! OUTPUT_ISSET(output_context))
-        setup_tmpfile (output_context);
-    }
-#endif
 
-  _outputs (is_err, msg);
+  _outputs (output_context, is_err, msg);
 }
 
 

output.h

@@ -26,12 +26,22 @@ extern struct output *output_context;
 #define OUTPUT_SET(_new)    do{ if ((_new)->syncout) output_context = (_new); }while(0)
 #define OUTPUT_UNSET()      do{ output_context = NULL; }while(0)
 
-void output_init (struct output *out, unsigned int syncout);
+FILE *output_tmpfile (char **, const char *);
+
+/* Initialize and close a child output structure: if NULL do this program's
+   output (this should only be done once).  */
+void output_init (struct output *out);
 void output_close (struct output *out);
 
+/* In situations where output may be about to be displayed but we're not
+   sure if we've set it up yet, call this.  */
 void output_start (void);
+
+/* Show a message on stdout or stderr.  Will start the output if needed.  */
 void outputs (int is_err, const char *msg);
 
 #ifdef OUTPUT_SYNC
+int output_tmpfd (void);
+/* Dump any child output content to stdout, and reset it.  */
 void output_dump (struct output *out);
 #endif

tests/ChangeLog

@@ -1,3 +1,7 @@
+2013-09-14  Paul Smith  <psmith@gnu.org>
+
+	* scripts/features/output-sync: Verify -Orecurse properly.
+
 2013-09-12  Paul Smith  <psmith@gnu.org>
 
 	* scripts/features/output-sync: Modify for output sync behavior.

tests/scripts/features/output-sync

@@ -135,8 +135,6 @@ foo: end
 #MAKE#[1]: Entering directory '#PWD#/bar'
 bar: start
 bar: end
-#MAKE#[1]: Leaving directory '#PWD#/bar'
-#MAKE#[1]: Entering directory '#PWD#/bar'
 baz: start
 baz: end
 #MAKE#[1]: Leaving directory '#PWD#/bar'\n", 0, 6);

vmsjobs.c

@@ -86,9 +86,7 @@ vms_handle_apos (char *p)
   alast = 0;
 
   while (*p != 0)
-    {
     if (*p == '"')
-	{
       if (alast)
         {
           alast = 0;
@@ -101,10 +99,8 @@ vms_handle_apos (char *p)
             break;
           alast = 1;
         }
-	}
     else
       p++;
-    }
 
   return p;
 }
@@ -173,10 +169,9 @@ vmsHandleChildTerm(struct child *child)
           break;
 
         case cs_finished:
-	    if (c->file->update_status != us_success) {
+          if (c->file->update_status != us_success)
             /* We failed to start the commands.  */
             delete_child_targets (c);
-	    }
           break;
 
         default:
@@ -261,9 +256,11 @@ tryToSetupYAst(void)
 
   if (chan)
     loc_chan= chan;
-	else {
+  else
+    {
       status= sys$assign(&inputDsc,&loc_chan,0,0);
-		if (!(status&SS$_NORMAL)) {
+      if (!(status&SS$_NORMAL))
+        {
           lib$signal(status);
           return;
         }
@@ -272,7 +269,8 @@ tryToSetupYAst(void)
                     astYHandler,0,0,0,0,0);
   if (status==SS$_NORMAL)
     status= iosb.status;
-	if (status!=SS$_NORMAL) {
+  if (status!=SS$_NORMAL)
+    {
       if (!chan)
         sys$dassgn(loc_chan);
       if (status!=SS$_ILLIOFUNC && status!=SS$_NOPRIV)
@@ -287,7 +285,8 @@ tryToSetupYAst(void)
     fprintf (stderr,
              _("-warning, you may have to re-enable CTRL-Y handling from DCL.\n"));
   status= lib$disable_ctrl (&ctrlMask, &oldCtrlMask);
-	if (!(status&SS$_NORMAL)) {
+  if (!(status&SS$_NORMAL))
+    {
       lib$signal(status);
       return;
     }
@@ -385,10 +384,8 @@ child_execute_job (char *argv, struct child *child)
                   efiledsc.dsc$b_class = DSC$K_CLASS_S;
                 }
               else
-		  {
                   p = vms_redirect (&efiledsc, efile, p);
             }
-	      }
           else
             {
               if (*(p+1) == '>')
@@ -494,7 +491,7 @@ child_execute_job (char *argv, struct child *child)
           return 0;
         }
 
-      outfile = open_tmpfile (&child->comname, "sys$scratch:CMDXXXXXX.COM");
+      outfile = output_tmpfile (&child->comname, "sys$scratch:CMDXXXXXX.COM");
       if (outfile == 0)
         pfatal_with_name (_("fopen (temporary file)"));
       comnamelen = strlen (child->comname);
@@ -514,7 +511,6 @@ child_execute_job (char *argv, struct child *child)
         }
 
       if (ofile[0])
-	{
         if (have_append)
           {
             fprintf (outfile, "$ set noon\n");
@@ -528,7 +524,6 @@ child_execute_job (char *argv, struct child *child)
             DB (DB_JOBS, (_("Redirected output to %s\n"), ofile));
             ofiledsc.dsc$w_length = 0;
           }
-	}
 
       p = sep = q = cmd;
       for (c = '\n'; c; c = *q++)