Re: [bug #33138] .PARLLELSYNC enhancement with patch

[Frank Heckenbach]

Paul Smith wrote:

> I went a different way: I modified the child_error() function so that if
> there was an output-sync file, the error message would be written to the
> end of that file instead of printed directly.  This way when the output
> is shown to the user she sees the entire thing, including error messages
> from make, in order.
> 
> This also allowed me to drop the layer of enter/leave notices around
> error messages.

I see, this seems better.

I've tested your changes and so far "-O job" works for my use cases.

> PS. Some may be tsking to themselves noting this change would have been
> a lot simpler if we'd kept a FILE* handle instead of a file descriptor.
> Unfortunately it's not clear that would work.  I created a test program
> to verify that this method of having the parent append messages to the
> file after the child exits, and with a FILE* handle I couldn't make it
> work right.  My fseek() after the child exited didn't skip past the
> child's output and my message printed by the parent ended up overwriting
> that output.  I'm sure it would have worked if I'd closed the file in
> the parent and re-opened it, but since I'm using tmpfile() that is not
> possible.  I might have done something wrong, but anyway it wasn't a
> slam-dunk.

I suppose a FILE* wants to own the actual file. I haven't checked
this particular case, but it rhymes with my experience in similar
situations. FILE* seems not suited for this kind of usage.

BTW:

:   /* This is needed to avoid the "label at end of compound statement"
:      diagnostics on Posix platforms.  */

I don't think that's a POSIX thing (which is a standard about
operating systems, not compiler diagnostics), is it?

:           if (output_sync == OUTPUT_SYNC_MAKE
:               || (output_sync && !(flags & COMMANDS_RECURSE)))

:           if (output_sync && (output_sync == OUTPUT_SYNC_MAKE
:                               || !(flags & COMMANDS_RECURSE)))

These two conditions are logically equivalent, but written
differently which may be confusing later.

Also, I think in the Windows branch, the former condition also
should say just "output_sync" instead of
"output_sync == OUTPUT_SYNC_TARGET", a recent change during the
introduction of "-O job".

The following patch makes these changes and fixes a precedence bug
that had crept in during the introduction of child_out (although I
can't actually test it on VMS :).

--- job.c.orig  2013-05-04 03:19:12.000000000 +0200
+++ job.c       2013-05-04 04:08:09.000000000 +0200
@@ -536,7 +536,7 @@
   l = strlen (pre) + strlen (f->name) + strlen (post);
 
 #ifdef VMS
-  if (exit_code & 1 != 0)
+  if ((exit_code & 1) != 0)
     return;
 
   msg = error_s (l + INTEGER_LENGTH, NILF, _("%s[%s] Error 0x%x%s"),
@@ -1709,8 +1709,8 @@
 #ifdef OUTPUT_SYNC
           /* Divert child output if output_sync in use.  Don't capture
              recursive make output unless we are synchronizing "make" mode.  */
-          if (output_sync && (output_sync == OUTPUT_SYNC_MAKE
-                              || !(flags & COMMANDS_RECURSE)))
+          if (output_sync == OUTPUT_SYNC_MAKE
+              || (output_sync && !(flags & COMMANDS_RECURSE)))
             {
               int outfd = fileno (stdout);
               int errfd = fileno (stderr);
@@ -1834,8 +1834,7 @@
               child's stdout, and another one for its stderr, if they
               are separate. */
           if (output_sync == OUTPUT_SYNC_MAKE
-              || (output_sync == OUTPUT_SYNC_TARGET
-                 && !(flags & COMMANDS_RECURSE)))
+              || (output_sync && !(flags & COMMANDS_RECURSE)))
             {
               if (!assign_child_tempfiles (child, combined_output))
                {
@@ -1859,8 +1858,8 @@
 #ifdef OUTPUT_SYNC
           /* Divert child output if output_sync in use.  Don't capture
              recursive make output unless we are synchronizing "make" mode.  */
-          if (output_sync && (output_sync == OUTPUT_SYNC_MAKE
-                              || !(flags & COMMANDS_RECURSE)))
+          if (output_sync == OUTPUT_SYNC_MAKE
+              || (output_sync && !(flags & COMMANDS_RECURSE)))
            hPID = process_easy(argv, child->environment,
                                child->outfd, child->errfd);
          else