I've tested the current output-sync implementation in my use case, using "--output-sync=line --trace=dir".
I definitely need "--trace=dir" since I want to refer my compiler errors automatically to their directories. Without this option, this often goes wrong in parallel builds. (Which just means, I don't care about the default for "--output-sync", since I'll need "--trace=dir" anyway which won't be the default I understand.) But unfortunately, even with this option, the enter/leave messages are not quite correct as the attached example shows (test.tar.bz2, though I admit I had to carefully construct it to exhibit the bug): % make -j -Oline --trace=dir make -Cd1 make -Cd2 make[1]: Entering directory 'd1' Makefile:5: d1-1 make[1]: Entering directory 'd2' sleep 1 make[1]: Leaving directory 'd2' make[1]: Entering directory 'd2' Makefile:5: d2-1 make[1]: Entering directory 'd1' sleep 2 make[1]: Leaving directory 'd1' Makefile:2: d1-2 make[1]: Leaving directory 'd1' make[1]: Entering directory 'd2' sleep 2 make[1]: Leaving directory 'd2' Makefile:2: d2-2 make[1]: Leaving directory 'd2' If you count the messages you'll see that the "d1-2" message appears while 'd1' and then 'd2' is "active", so a reader (automatic or manual) of the messages would think we're in d2, but we're actually in d1. Also, the "Leaving directory 'd1'" message right afterwards doesn't pair up correctly (LIFO). The latter problem also exists, and is more clearly visible, without "--trace=dir": % make -j -Oline make -Cd1 make -Cd2 make[1]: Entering directory 'd1' Makefile:5: d1-1 make[1]: Entering directory 'd2' sleep 1 Makefile:5: d2-1 sleep 2 Makefile:2: d1-2 make[1]: Leaving directory 'd1' sleep 2 Makefile:2: d2-2 make[1]: Leaving directory 'd2' That's why I had added the "spurious" enter/leave messages in message(), error() and fatal() which you removed again. If I add them back (misc.c.diff), these problems disappear. I understand you do not like them by default, but perhaps you could add them dependent on "--trace=dir" (or "--trace=dir-verbose" if you want it even more explicit). Some minor things (job.c.diff): - assign_child_tempfiles(): If c->outfd is closed on error, it should probably reset the field to -1. I haven't traced whether this can cause an actual bug, but I think it's safer to not keep an invalid fd. - pump_from_tmp(): Change "to_fd" to "to" in 2 comments. - start_job_command(): "output_sync && sync_cmd" is redundant (but doesn't hurt) since sync_cmd is "output_sync && ..." anyway.
--- job.c.orig 2013-05-26 00:44:38.000000000 +0200
+++ job.c 2013-05-26 01:36:45.000000000 +0200
@@ -687,7 +687,10 @@
error:
if (c->outfd >= 0)
- close (c->outfd);
+ {
+ close (c->outfd);
+ c->outfd = -1;
+ }
output_sync = 0;
}
@@ -701,7 +704,7 @@
int prev_mode;
/* from_fd is opened by open_tmpfd, which does it in binary mode, so
- we need the mode of to_fd to match that. */
+ we need the mode of "to" to match that. */
prev_mode = _setmode (fileno (to), _O_BINARY);
#endif
@@ -721,7 +724,7 @@
}
#ifdef WINDOWS32
- /* Switch to_fd back to its original mode, so that log messages by
+ /* Switch "to" back to its original mode, so that log messages by
Make have the same EOL format as without --output-sync. */
_setmode (fileno (to), prev_mode);
#endif
@@ -1753,7 +1756,7 @@
#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 && sync_cmd)
+ if (sync_cmd)
{
int outfd = fileno (stdout);
int errfd = fileno (stderr);
@@ -1866,7 +1869,7 @@
#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 && sync_cmd)
+ if (sync_cmd)
hPID = process_easy (argv, child->environment,
child->outfd, child->errfd);
else
--- misc.c.orig 2013-05-26 00:44:38.000000000 +0200
+++ misc.c 2013-05-26 04:09:23.000000000 +0200
@@ -266,6 +266,9 @@
if (fmt != 0)
{
+ if (output_sync)
+ log_working_directory (1, 1);
+
if (prefix)
{
if (makelevel == 0)
@@ -277,6 +280,9 @@
vfprintf (stdout, fmt, args);
va_end (args);
putchar ('\n');
+
+ if (output_sync)
+ log_working_directory (0, 1);
}
fflush (stdout);
@@ -289,7 +295,10 @@
{
va_list args;
- log_working_directory (1, 0);
+ if (output_sync)
+ log_working_directory (1, 1);
+ else
+ log_working_directory (1, 0);
if (flocp && flocp->filenm)
fprintf (stderr, "%s:%lu: ", flocp->filenm, flocp->lineno);
@@ -304,6 +313,9 @@
putc ('\n', stderr);
fflush (stderr);
+
+ if (output_sync)
+ log_working_directory (0, 1);
}
/* Print an error message and exit. */
@@ -313,7 +325,10 @@
{
va_list args;
- log_working_directory (1, 0);
+ if (output_sync)
+ log_working_directory (1, 1);
+ else
+ log_working_directory (1, 0);
if (flocp && flocp->filenm)
fprintf (stderr, "%s:%lu: *** ", flocp->filenm, flocp->lineno);
@@ -328,7 +343,8 @@
fputs (_(". Stop.\n"), stderr);
- log_working_directory (0, 1);
+ if (output_sync)
+ log_working_directory (0, 1);
die (2);
}
test.tar.bz2
Description: Binary data
_______________________________________________ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
