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?
> > 5.
> >
> > ISTM when output_dump() is called, output_context always ought to
> > be NULL (the call is either outside of any OUTPUT_SET/OUTPUT_UNSET
> > section, or (job.c:1274) child->output.syncout is NULL), is that
> > right?
I now see that's not the case ...
> > 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):
% cat Makefile
all: a c
a: b
$(shell true)
b:
true
c:
echo c
% make -j -Oline -sw
c
make: Entering directory 'foo'
make: Entering directory 'foo'
make: Leaving directory 'foo'
make: Leaving directory 'foo'
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()):
--- output.c.orig 2013-09-24 19:45:35.000000000 +0200
+++ output.c 2013-09-24 19:45:50.000000000 +0200
@@ -387,7 +387,7 @@
/* 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 (NULL, 1);
if (outfd_not_empty)
pump_from_tmp (out->out, stdout);
@@ -395,7 +395,7 @@
pump_from_tmp (out->err, stderr);
if (traced)
- log_working_directory (output_context, 0);
+ log_working_directory (NULL, 0);
/* Exit the critical section. */
if (sem)
I'm afraid I found two new issues (continuing my numbering):
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.
However, I wasn't able to really test it, because when starting make
with stderr closed, the first open() call (here, reading the
Makefile) would open it as fd 2 so "stderr" wasn't closed anymore by
the time setup_tmpfile() was called. This is, of course, perfectly
expected POSIX behaviour, and I don't think there's anything make
can or should do about it. So the whole STREAM_OK checking is
probably overkill since starting make (or just about any non-daemon)
with stdfoo closed is just a stupid thing to do. But now that we
have it, I think we should just keep it.
--- function.c.orig 2013-09-24 16:24:54.000000000 +0200
+++ function.c 2013-09-24 17:55:54.000000000 +0200
@@ -1725,7 +1725,7 @@
setrlimit (RLIMIT_STACK, &stack_limit);
# endif
child_execute_job (FD_STDIN, pipedes[1],
- output_context ? output_context->err : FD_STDERR,
+ output_context && output_context->err >= 0 ?
output_context->err : FD_STDERR,
command_argv, envp);
}
else
9.
In a complex case, I still get mismatched "Enter/Leave" messages.
I haven't been able to produce a small test case so far, but I did
find that the following statement (and the corresponding "Leave"
one) was executed in "-Oline" mode:
stdio_traced = log_working_directory (NULL, 1);
That seems wrong per se, since it writes to stdout unsynchronized.
This must be due to the first part of this condition:
if (! output_context || output_sync == OUTPUT_SYNC_RECURSE)
So I'd suggest to change this as follows. This would make sure no
unsynchronized "Enter/Leave" messages could be produced in
-Oline/-Otarget. Hopefully, after your last changes, there should be
no other unsynchronized output in theses modes anymore at all, but
even there is (which would be a bug), it might be better to avoid
those messages. Otherwise, in a parallel, recursive,
target/line-synched build, those unsynchronized messages may appear
in far away places, intermixed with messages from other directories
which is what seemed to happen in my case. After applying this
patch, I haven't seen this problem anymore.
--- output.c.orig 2013-09-24 20:25:14.000000000 +0200
+++ output.c 2013-09-24 20:25:21.000000000 +0200
@@ -583,7 +583,7 @@
setup_tmpfile (output_context);
#endif
- if (! output_context || output_sync == OUTPUT_SYNC_RECURSE)
+ if (! output_sync || output_sync == OUTPUT_SYNC_RECURSE)
{
if (! stdio_traced && print_directory_flag)
stdio_traced = log_working_directory (NULL, 1);
_______________________________________________
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
