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

[Paul Smith]

As always thanks for your thorough testing Frank.

On Tue, 2013-09-24 at 20:41 +0200, Frank Heckenbach wrote:
> Paul Smith wrote:
> 
> > On Thu, 2013-09-19 at 14:47 +0200, Frank Heckenbach wrote:
> > > Paul Smith wrote:
> > > 
> > > > I didn't fix it this way.  Instead I used the existing MAKE_RESTART
> > > > environment variable to communicate from the current make to the
> > > > restarted make that the enter message was already printed (if it was) so
> > > > it wouldn't be printed again.  This ensures we don't get a stream of
> > > > enter/leave statements as we re-exec.
> > > > 
> > > > This works now (in my repo).
> 
> It works for me too. However, since MAKE_RESTARTS is a documented
> variable, couldn't this change confuse user Makefiles?

That's a good point.  I rearranged this to ensure we set the make
variable to the integer value, so the user can't see the special token.

> > > If so, the use of output_context might be slightly irritating
> > > (though not wrong) -- at first I wondered where the
> > > log_working_directory() output after the pump_from_tmp() calls was
> > > going to and whether it didn't need pumping too if it was going to
> > > the temp file, but apparently this never happens.
> 
> ... which apparently does lead to a problem here (non-deterministic
> like many "-j" problems):

> AIUI, it dumps out to stdout/stderr, but prints "Enter/Leave" to
> output_context (which might be dumped much later), so out's contents
> are not properly enclosed. Since we dump out to stdout/stderr (and
> we got the semaphore for writing to those), it seems logical to me
> to print "Enter/Leave" there as well, so this seems to fix it for me
> (and again would obviate the need for the first parameter to
> log_working_directory()):

I agree with this change.

> 8.
> 
> Like job.c, I think function.c should check "output_context->err >= 0",
> to avoid redirecting to -1 when no temp file for stderr was set up.

Yes, sounds right.

> 9.

> -  if (! output_context || output_sync == OUTPUT_SYNC_RECURSE)
> +  if (! output_sync || output_sync == OUTPUT_SYNC_RECURSE)

This looks right to me as well.