mirrors/make-dfsg
1
0
Fork 0
mirror of https://salsa.debian.org/srivasta/make-dfsg.git synced 2024-12-30 16:49:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Improve sync handling for -Ojob/-Otarget and recursion.

Browse source 
If we are not going to sync a command line then dump any collected output
first to preserve ordering.  Do some code cleanup:
  * Move the handle init to a separate function.
  * Move the temp file truncation to the output function.
  * Remember whether we sync in a variable for readability.
  * Handle EINTR and short writes in child_out().
  * Always call sync_output() in case output_sync was changed due to error.
This commit is contained in:
Paul Smith 2013-05-04 13:10:56 -04:00
parent cb2be0db36
commit b8467292c9
5 changed files with 186 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
ChangeLog
View file

@ -1,3 +1,16 @@
2013-05-04 Paul Smith <psmith@gnu.org>
* job.c (child_out): Handle EINTR and incomplete write scenarios.
(sync_init): New function: separate the initialization code.
(assign_child_tempfiles): Remove truncation from this function,
(sync_output): and add it here after output is generated.
(reap_children): Always call sync_output() in case output_sync was
reset after the child started, due to error.
(start_job_command): Create new sync_cmd variable. Use new method
for initializing the handle.
If we're not syncing the output be sure any output we've saved is
dumped immediately before starting the child.
2013-05-04 Eli Zaretskii <eliz@gnu.org> 2013-05-04 Eli Zaretskii <eliz@gnu.org>
* job.c (start_job_command): Make the condition for creating a * job.c (start_job_command): Make the condition for creating a

273
job.c
View file

@ -244,9 +244,12 @@ unsigned long job_counter = 0;
unsigned int jobserver_tokens = 0; unsigned int jobserver_tokens = 0;
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* Semaphore for use in -j mode with output_sync. */
sync_handle_t sync_handle = -1; /* Semaphore for use in -j mode with output_sync. */
static sync_handle_t sync_handle = -1;
/* Is make's stdout going to the same place as stderr? */
static int combined_output = 0;
#define STREAM_OK(_s) ((fcntl (fileno (_s), F_GETFD) != -1) || (errno != EBADF)) #define STREAM_OK(_s) ((fcntl (fileno (_s), F_GETFD) != -1) || (errno != EBADF))
@ -467,7 +470,7 @@ is_bourne_compatible_shell (const char *path)
} }
/* Write a message relating to a child. Write it to the child's output /* Write a message in the child's context. Write it to the child's output
sync file if present, otherwise to the terminal. */ sync file if present, otherwise to the terminal. */
static void static void
@ -477,16 +480,27 @@ child_out (const struct child *child, const char *msg, int out)
if (outfd >= 0) if (outfd >= 0)
{ {
int t = strlen (msg);
int l;
lseek (outfd, 0, SEEK_END); lseek (outfd, 0, SEEK_END);
write (outfd, msg, strlen (msg)); while (t)
write (outfd, "\n", 1); {
EINTRLOOP (l, write (outfd, msg, t));
if (l == t)
break;
if (l < 0)
{
perror ("write()");
break;
}
t -= l;
msg += l;
}
} }
else else
{ {
FILE *outf = out ? stdout : stderr; FILE *outf = out ? stdout : stderr;
fputs (msg, outf); fputs (msg, outf);
putc ('\n', outf);
fflush (outf); fflush (outf);
} }
} }
@ -503,7 +517,7 @@ child_error (const struct child *child,
const char *post = ""; const char *post = "";
const char *dump = ""; const char *dump = "";
const struct file *f = child->file; const struct file *f = child->file;
gmk_floc *flocp = &f->cmds->fileinfo; const gmk_floc *flocp = &f->cmds->fileinfo;
const char *msg; const char *msg;
const char *nm; const char *nm;
unsigned int l; unsigned int l;
@ -529,8 +543,8 @@ child_error (const struct child *child,
nm = a; nm = a;
} }
msg = message_s (strlen (nm) + strlen (f->name), msg = message_s (strlen (nm) + strlen (f->name), 0,
0, _("%s: recipe for target '%s' failed"), nm, f->name); _("%s: recipe for target '%s' failed"), nm, f->name);
child_out (child, msg, 1); child_out (child, msg, 1);
l = strlen (pre) + strlen (f->name) + strlen (post); l = strlen (pre) + strlen (f->name) + strlen (post);
@ -539,17 +553,17 @@ child_error (const struct child *child,
if (exit_code & 1 != 0) if (exit_code & 1 != 0)
return; return;
msg = error_s (l + INTEGER_LENGTH, NILF, _("%s[%s] Error 0x%x%s"), msg = error_s (l + INTEGER_LENGTH, NILF,
pre, f->name, exit_code, post); _("%s[%s] Error 0x%x%s"), pre, f->name, exit_code, post);
#else #else
if (exit_sig == 0) if (exit_sig == 0)
msg = error_s (l + INTEGER_LENGTH, NILF, _("%s[%s] Error %d%s"), msg = error_s (l + INTEGER_LENGTH, NILF,
pre, f->name, exit_code, post); _("%s[%s] Error %d%s"), pre, f->name, exit_code, post);
else else
{ {
const char *s = strsignal (exit_sig); const char *s = strsignal (exit_sig);
msg = error_s (l + strlen (s) + strlen (dump), msg = error_s (l + strlen (s) + strlen (dump), NILF,
NILF, _("%s[%s] %s%s%s"), pre, f->name, s, dump, post); _("%s[%s] %s%s%s"), pre, f->name, s, dump, post);
} }
#endif /* VMS */ #endif /* VMS */
@ -591,52 +605,85 @@ child_handler (int sig UNUSED)
} }
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* Adds file descriptors to the child structure to support output_sync;
one for stdout and one for stderr as long as they are (a) open and /* Set up the sync handle and configure combined_output.
(b) separate. If stdout and stderr share a device they can share a Disables output_sync on error. */
temp file too. */ static void
static int sync_init ()
assign_child_tempfiles (struct child *c, int combined)
{ {
/* If we don't have a temp file, get one. */ #ifdef WINDOWS32
if (c->outfd < 0 && c->errfd < 0) if ((!STREAM_OK (stdout) && !STREAM_OK (stderr))
|| (sync_handle = create_mutex ()) == -1)
{ {
if (STREAM_OK (stdout)) perror_with_name ("output-sync suppressed: ", "stderr");
{ output_sync = 0;
c->outfd = open_tmpfd (); }
CLOSE_ON_EXEC (c->outfd); else
} {
combined_output = same_stream (stdout, stderr);
if (STREAM_OK (stderr)) prepare_mutex_handle_string (sync_handle);
{
if (c->outfd >= 0 && combined)
c->errfd = c->outfd;
else
{
c->errfd = open_tmpfd ();
CLOSE_ON_EXEC (c->errfd);
}
}
return 1;
} }
/* We already have a temp file. On per-job output, truncate and reset. */ #else
if (output_sync == OUTPUT_SYNC_JOB) if (STREAM_OK (stdout))
{ {
if (c->outfd >= 0) struct stat stbuf_o, stbuf_e;
sync_handle = fileno (stdout);
combined_output =
fstat (fileno (stdout), &stbuf_o) == 0 &&
fstat (fileno (stderr), &stbuf_e) == 0 &&
stbuf_o.st_dev == stbuf_e.st_dev &&
stbuf_o.st_ino == stbuf_e.st_ino;
}
else if (STREAM_OK (stderr))
sync_handle = fileno (stderr);
else
{
perror_with_name ("output-sync suppressed: ", "stderr");
output_sync = 0;
}
#endif
}
/* 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
assign_child_tempfiles (struct child *c)
{
/* If we have a temp file, we're done. */
if (c->outfd >= 0 || c->errfd >= 0)
return;
if (STREAM_OK (stdout))
{
c->outfd = open_tmpfd ();
if (c->outfd < 0)
goto error;
CLOSE_ON_EXEC (c->outfd);
}
if (STREAM_OK (stderr))
{
if (c->outfd >= 0 && combined_output)
c->errfd = c->outfd;
else
{ {
lseek(c->outfd, 0, SEEK_SET); c->errfd = open_tmpfd ();
ftruncate(c->outfd, 0); if (c->errfd < 0)
} goto error;
if (c->errfd >= 0 && c->errfd != c->outfd) CLOSE_ON_EXEC (c->errfd);
{
lseek(c->errfd, 0, SEEK_SET);
ftruncate(c->outfd, 0);
} }
} }
return 1; return;
error:
if (c->outfd >= 0)
close (c->outfd);
output_sync = 0;
} }
/* Support routine for sync_output() */ /* Support routine for sync_output() */
@ -750,6 +797,20 @@ sync_output (struct child *c)
/* Exit the critical section. */ /* Exit the critical section. */
if (sem) if (sem)
release_semaphore (sem); release_semaphore (sem);
/* Truncate and reset the output, in case we use it again. */
if (c->outfd >= 0)
{
int e;
lseek(c->outfd, 0, SEEK_SET);
EINTRLOOP (e, ftruncate(c->outfd, 0));
}
if (c->errfd >= 0 && c->errfd != c->outfd)
{
int e;
lseek(c->errfd, 0, SEEK_SET);
EINTRLOOP (e, ftruncate(c->errfd, 0));
}
} }
} }
#endif /* OUTPUT_SYNC */ #endif /* OUTPUT_SYNC */
@ -1095,7 +1156,8 @@ reap_children (int block, int err)
else else
{ {
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* If we're sync'ing per job, write it now. */ /* If we're sync'ing per line, write the previous line's
output before starting the next one. */
if (output_sync == OUTPUT_SYNC_JOB) if (output_sync == OUTPUT_SYNC_JOB)
sync_output (c); sync_output (c);
#endif #endif
@ -1130,9 +1192,8 @@ reap_children (int block, int err)
/* When we get here, all the commands for c->file are finished. */ /* When we get here, all the commands for c->file are finished. */
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* Synchronize parallel output if requested */ /* Synchronize any remaining parallel output. */
if (output_sync) sync_output (c);
sync_output (c);
#endif /* OUTPUT_SYNC */ #endif /* OUTPUT_SYNC */
/* At this point c->file->update_status contains 0 or 2. But /* At this point c->file->update_status contains 0 or 2. But
@ -1334,6 +1395,7 @@ start_job_command (struct child *child)
#if !defined(_AMIGA) && !defined(WINDOWS32) #if !defined(_AMIGA) && !defined(WINDOWS32)
static int bad_stdin = -1; static int bad_stdin = -1;
#endif #endif
int sync_cmd = 0;
char *p; char *p;
/* Must be volatile to silence bogus GCC warning about longjmp/vfork. */ /* Must be volatile to silence bogus GCC warning about longjmp/vfork. */
volatile int flags; volatile int flags;
@ -1521,6 +1583,11 @@ start_job_command (struct child *child)
fflush (stdout); fflush (stdout);
fflush (stderr); fflush (stderr);
/* Are we going to synchronize this command's output? Do so if either we're
in SYNC_MAKE mode or this command is not recursive. We'll also check
output_sync separately below in case it changes due to error. */
sync_cmd = output_sync == OUTPUT_SYNC_MAKE || !(flags & COMMANDS_RECURSE);
#ifndef VMS #ifndef VMS
#if !defined(WINDOWS32) && !defined(_AMIGA) && !defined(__MSDOS__) #if !defined(WINDOWS32) && !defined(_AMIGA) && !defined(__MSDOS__)
@ -1646,42 +1713,16 @@ start_job_command (struct child *child)
#else /* !__EMX__ */ #else /* !__EMX__ */
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
if (output_sync) if (output_sync && sync_handle == -1)
{ sync_init();
static int combined_output;
/* If output_sync is turned on, find a resource to
synchronize on. This block is traversed only once. */
if (sync_handle == -1)
{
if (STREAM_OK (stdout))
{
struct stat stbuf_o, stbuf_e;
sync_handle = fileno (stdout); if (output_sync && sync_cmd)
combined_output = /* If we still want to sync, make sure we have temp files. */
fstat (fileno (stdout), &stbuf_o) == 0 && assign_child_tempfiles (child);
fstat (fileno (stderr), &stbuf_e) == 0 && else
stbuf_o.st_dev == stbuf_e.st_dev && /* We don't want to sync this command: to avoid misordered
stbuf_o.st_ino == stbuf_e.st_ino; output ensure any already-synched content is written. */
} sync_output (child);
else if (STREAM_OK (stderr))
sync_handle = fileno (stderr);
else
{
perror_with_name ("output-sync suppressed: ", "stderr");
output_sync = 0;
}
}
/* If it still looks like we can synchronize, create a temp
file to hold stdout (and one for stderr if separate). */
if (output_sync == OUTPUT_SYNC_MAKE
|| (output_sync && !(flags & COMMANDS_RECURSE)))
{
if (!assign_child_tempfiles (child, combined_output))
output_sync = 0;
}
}
#endif /* OUTPUT_SYNC */ #endif /* OUTPUT_SYNC */
child->pid = vfork (); child->pid = vfork ();
@ -1710,8 +1751,7 @@ start_job_command (struct child *child)
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* Divert child output if output_sync in use. Don't capture /* Divert child output if output_sync in use. Don't capture
recursive make output unless we are synchronizing "make" mode. */ recursive make output unless we are synchronizing "make" mode. */
if (output_sync && (output_sync == OUTPUT_SYNC_MAKE if (output_sync && sync_cmd)
|| !(flags & COMMANDS_RECURSE)))
{ {
int outfd = fileno (stdout); int outfd = fileno (stdout);
int errfd = fileno (stderr); int errfd = fileno (stderr);
@ -1812,38 +1852,16 @@ start_job_command (struct child *child)
char* arg0; char* arg0;
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
if (output_sync) if (output_sync && sync_handle == -1)
{ sync_init();
static int combined_output;
/* If output_sync is turned on, create a mutex to if (output_sync && sync_cmd)
synchronize on. This is done only once. */ /* If we still want to sync, make sure we have temp files. */
if (sync_handle == -1) assign_child_tempfiles (child);
{ else
if ((!STREAM_OK (stdout) && !STREAM_OK (stderr)) /* We don't want to sync this command: to avoid misordered output
|| (sync_handle = create_mutex ()) == -1) ensure any already-synched content is written. */
{ sync_output (child);
perror_with_name ("output-sync suppressed: ", "stderr");
output_sync = 0;
}
else
{
combined_output = same_stream (stdout, stderr);
prepare_mutex_handle_string (sync_handle);
}
}
/* If we can synchronize, create a temporary file to hold
child's stdout, and another one for its stderr, if they
are separate. */
if (output_sync == OUTPUT_SYNC_MAKE
|| (output_sync && !(flags & COMMANDS_RECURSE)))
{
if (!assign_child_tempfiles (child, combined_output))
{
perror_with_name ("output-sync suppressed: ", "stderr");
output_sync = 0;
}
}
}
#endif /* OUTPUT_SYNC */ #endif /* OUTPUT_SYNC */
/* make UNC paths safe for CreateProcess -- backslash format */ /* make UNC paths safe for CreateProcess -- backslash format */
@ -1859,8 +1877,7 @@ start_job_command (struct child *child)
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* Divert child output if output_sync in use. Don't capture /* Divert child output if output_sync in use. Don't capture
recursive make output unless we are synchronizing "make" mode. */ recursive make output unless we are synchronizing "make" mode. */
if (output_sync && (output_sync == OUTPUT_SYNC_MAKE if (output_sync && sync_cmd)
|| !(flags & COMMANDS_RECURSE)))
hPID = process_easy(argv, child->environment, hPID = process_easy(argv, child->environment,
child->outfd, child->errfd); child->outfd, child->errfd);
else else

19
misc.c
View file

@ -182,7 +182,8 @@ concat (unsigned int num, ...)
/* If we had a standard-compliant vsnprintf() this would be a lot simpler. /* If we had a standard-compliant vsnprintf() this would be a lot simpler.
Maybe in the future we'll include gnulib's version. */ Maybe in the future we'll include gnulib's version. */
/* Return a formatted string buffer. */ /* Return a formatted string buffer.
LENGTH must be the maximum length of all format arguments, stringified. */
const char * const char *
message_s (unsigned int length, int prefix, const char *fmt, ...) message_s (unsigned int length, int prefix, const char *fmt, ...)
@ -193,7 +194,7 @@ message_s (unsigned int length, int prefix, const char *fmt, ...)
va_list args; va_list args;
/* Compute the maximum buffer size we'll need, and make sure we have it. */ /* Compute the maximum buffer size we'll need, and make sure we have it. */
length += strlen (fmt) + strlen (program) + 4 + INTEGER_LENGTH + 1; length += strlen (fmt) + strlen (program) + 4 + INTEGER_LENGTH + 2;
if (length > bsize) if (length > bsize)
{ {
bsize = length * 2; bsize = length * 2;
@ -214,10 +215,13 @@ message_s (unsigned int length, int prefix, const char *fmt, ...)
vsprintf (bp, fmt, args); vsprintf (bp, fmt, args);
va_end (args); va_end (args);
strcat (bp, "\n");
return buffer; return buffer;
} }
/* Return a formatted error message in a buffer. */ /* Return a formatted error message in a buffer.
LENGTH must be the maximum length of all format arguments, stringified. */
const char * const char *
error_s (unsigned int length, const gmk_floc *flocp, const char *fmt, ...) error_s (unsigned int length, const gmk_floc *flocp, const char *fmt, ...)
@ -228,7 +232,7 @@ error_s (unsigned int length, const gmk_floc *flocp, const char *fmt, ...)
va_list args; va_list args;
/* Compute the maximum buffer size we'll need, and make sure we have it. */ /* Compute the maximum buffer size we'll need, and make sure we have it. */
length += (strlen (fmt) + strlen (program) + 4 + INTEGER_LENGTH + 1 length += (strlen (fmt) + strlen (program) + 4 + INTEGER_LENGTH + 2
+ (flocp && flocp->filenm ? strlen (flocp->filenm) : 0)); + (flocp && flocp->filenm ? strlen (flocp->filenm) : 0));
if (length > bsize) if (length > bsize)
{ {
@ -249,10 +253,13 @@ error_s (unsigned int length, const gmk_floc *flocp, const char *fmt, ...)
vsprintf (bp, fmt, args); vsprintf (bp, fmt, args);
va_end (args); va_end (args);
strcat (bp, "\n");
return buffer; return buffer;
} }
/* Print a message on stdout. */ /* Print a message on stdout. We could use message_s() to format it but then
we'd need a va_list version... */
void void
message (int prefix, const char *fmt, ...) message (int prefix, const char *fmt, ...)
@ -950,7 +957,7 @@ char *mktemp (char *template);
# endif # endif
#endif #endif
/* This is only used by output-sync, and it may not be portable to Windows. */ /* This is only used by output-sync, and it may not be portable. */
#ifdef OUTPUT_SYNC #ifdef OUTPUT_SYNC
/* Returns a file descriptor to a temporary file. The file is automatically /* Returns a file descriptor to a temporary file. The file is automatically

5
tests/ChangeLog
View file

@ -1,3 +1,8 @@
2013-05-04 Paul Smith <psmith@gnu.org>
* scripts/features/output-sync (output_sync_set): New test for
ordered recursive output for -Ojob / -Otarget.
2013-05-03 Eli Zaretskii <eliz@gnu.org> 2013-05-03 Eli Zaretskii <eliz@gnu.org>
* scripts/features/load: Fix signatures of testload_gmk_setup and * scripts/features/load: Fix signatures of testload_gmk_setup and

10
tests/scripts/features/output-sync
View file

@ -244,5 +244,15 @@ foo: end
# Remove temporary directories and contents. # Remove temporary directories and contents.
output_sync_clean(); output_sync_clean();
# Ensure recursion doesn't mis-order or double-print output
run_make_test(qq!
all:
\t\@echo foo
\t\@+echo bar
!,
'-j -Ojob', "foo\nbar\n");
run_make_test(undef, '-j -Otarget', "foo\nbar\n");
# This tells the test driver that the perl test script executed properly. # This tells the test driver that the perl test script executed properly.
1; 1;