From 9cd01958da86a68d0e47defcb9745ab373ef3d79 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 21 Sep 2013 15:23:05 -0400 Subject: Ensure that stderr from shell functions in recipes is synced. --- ChangeLog | 16 ++++ function.c | 10 ++- job.c | 153 ++++++++++++++++++++++--------------- job.h | 13 +++- main.c | 3 +- tests/ChangeLog | 3 + tests/scripts/features/output-sync | 6 ++ 7 files changed, 133 insertions(+), 71 deletions(-) diff --git a/ChangeLog b/ChangeLog index 861d841..f4a897e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2013-09-21 Paul Smith + + Stderr generated from shell functions in recipes should be synced. + + * job.h (FD_STDIN, FD_STDOUT, FD_STDERR): Create new macros to + avoid magic numbers. + (child_execute_job): Take a FD for stderr. + * job.c (child_execute_job): Handle STDERR FD's in addition to + stdin and stdout. + (start_job_command): Call child_execute_job() with the new STDERR + parameter. Instead of performing the dup() here, send it to + child_execute_job() where it's already being done. + * function.c (func_shell_base): Pass the OUTPUT_CONTEXT stderr to + child_execute_job() if it's set, otherwise FD_STDERR. + * main.c (main): Pass FD_STDERR to child_execute_job(). + 2013-09-19 Paul Smith * main.c (main): Set MAKE_RESTARTS to negative before re-exec if diff --git a/function.c b/function.c index 4ea2446..866ac48 100644 --- a/function.c +++ b/function.c @@ -1710,7 +1710,7 @@ func_shell_base (char *o, char **argv, int trim_newlines) CLOSE_ON_EXEC(pipedes[1]); CLOSE_ON_EXEC(pipedes[0]); /* Never use fork()/exec() here! Use spawn() instead in exec_command() */ - pid = child_execute_job (0, pipedes[1], command_argv, envp); + pid = child_execute_job (FD_STDIN, pipedes[1], FD_STDOUT, command_argv, envp); if (pid < 0) perror_with_name (error_prefix, "spawn"); # else /* ! __EMX__ */ @@ -1719,12 +1719,14 @@ func_shell_base (char *o, char **argv, int trim_newlines) perror_with_name (error_prefix, "fork"); else if (pid == 0) { -#ifdef SET_STACK_SIZE +# ifdef SET_STACK_SIZE /* Reset limits, if necessary. */ if (stack_limit.rlim_cur) setrlimit (RLIMIT_STACK, &stack_limit); -#endif - child_execute_job (0, pipedes[1], command_argv, envp); +# endif + child_execute_job (FD_STDIN, pipedes[1], + output_context ? output_context->err : FD_STDERR, + command_argv, envp); } else # endif diff --git a/job.c b/job.c index 6480d8d..79d25ee 100644 --- a/job.c +++ b/job.c @@ -1438,7 +1438,8 @@ start_job_command (struct child *child) CLOSE_ON_EXEC (job_rfd); /* Never use fork()/exec() here! Use spawn() instead in exec_command() */ - child->pid = child_execute_job (child->good_stdin ? 0 : bad_stdin, 1, + child->pid = child_execute_job (child->good_stdin ? FD_STDIN : bad_stdin, + FD_STDOUT, FD_STDERR, argv, child->environment); if (child->pid < 0) { @@ -1463,6 +1464,9 @@ start_job_command (struct child *child) environ = parent_environ; /* Restore value child may have clobbered. */ if (child->pid == 0) { + int outfd = FD_STDOUT; + int errfd = FD_STDERR; + /* We are the child side. */ unblock_sigs (); @@ -1482,26 +1486,17 @@ start_job_command (struct child *child) setrlimit (RLIMIT_STACK, &stack_limit); #endif -#ifdef OUTPUT_SYNC - /* Divert child output if output_sync in use. Don't capture - recursive make output unless we are synchronizing "make" mode. */ + /* Divert child output if output_sync in use. */ if (child->output.syncout) { - int outfd = fileno (stdout); - int errfd = fileno (stderr); - - if ((child->output.out >= 0 - && (close (outfd) == -1 - || dup2 (child->output.out, outfd) == -1)) - || (child->output.err >= 0 - && (close (errfd) == -1 - || dup2 (child->output.err, errfd) == -1))) - perror_with_name ("output-sync: ", "dup2()"); + if (child->output.out >= 0) + outfd = child->output.out; + if (child->output.err >= 0) + errfd = child->output.err; } -#endif /* OUTPUT_SYNC */ - child_execute_job (child->good_stdin ? 0 : bad_stdin, 1, - argv, child->environment); + child_execute_job (child->good_stdin ? FD_STDIN : bad_stdin, + outfd, errfd, argv, child->environment); } else if (child->pid < 0) { @@ -2185,59 +2180,82 @@ start_waiting_jobs (void) /* EMX: Start a child process. This function returns the new pid. */ # if defined __EMX__ int -child_execute_job (int stdin_fd, int stdout_fd, char **argv, char **envp) +child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd, + char **argv, char **envp) { int pid; - /* stdin_fd == 0 means: nothing to do for stdin; - stdout_fd == 1 means: nothing to do for stdout */ - int save_stdin = (stdin_fd != 0) ? dup (0) : 0; - int save_stdout = (stdout_fd != 1) ? dup (1): 1; - - /* < 0 only if dup() failed */ - if (save_stdin < 0) - fatal (NILF, _("no more file handles: could not duplicate stdin\n")); - if (save_stdout < 0) - fatal (NILF, _("no more file handles: could not duplicate stdout\n")); - - /* Close unnecessary file handles for the child. */ - if (save_stdin != 0) - CLOSE_ON_EXEC (save_stdin); - if (save_stdout != 1) - CLOSE_ON_EXEC (save_stdout); - - /* Connect the pipes to the child process. */ - if (stdin_fd != 0) - (void) dup2 (stdin_fd, 0); - if (stdout_fd != 1) - (void) dup2 (stdout_fd, 1); - - /* stdin_fd and stdout_fd must be closed on exit because we are - still in the parent process */ - if (stdin_fd != 0) - CLOSE_ON_EXEC (stdin_fd); - if (stdout_fd != 1) - CLOSE_ON_EXEC (stdout_fd); + int save_stdin = -1; + int save_stdout = -1; + int save_stderr = -1; + + /* For each FD which needs to be redirected first make a dup of the standard + FD to save and mark it close on exec so our child won't see it. Then + dup2() the standard FD to the redirect FD, and also mark the redirect FD + as close on exec. */ + if (stdin_fd != FD_STDIN) + { + save_stdin = dup (FD_STDIN); + if (save_stdin < 0) + fatal (NILF, _("no more file handles: could not duplicate stdin\n")); + CLOSE_ON_EXEC (save_stdin); + + dup2 (stdin_fd, FD_STDIN); + CLOSE_ON_EXEC (stdin_fd); + } + + if (stdout_fd != FD_STDOUT) + { + save_stdout = dup (FD_STDOUT); + if (save_stdout < 0) + fatal (NILF, _("no more file handles: could not duplicate stdout\n")); + CLOSE_ON_EXEC (save_stdout); + + dup2 (stdout_fd, FD_STDOUT); + CLOSE_ON_EXEC (stdout_fd); + } + + if (stderr_fd != FD_STDERR) + { + if (stderr_fd != stdout_fd) + { + save_stderr = dup (FD_STDERR); + if (save_stderr < 0) + fatal (NILF, _("no more file handles: could not duplicate stderr\n")); + CLOSE_ON_EXEC (save_stderr); + } + + dup2 (stderr_fd, FD_STDERR); + CLOSE_ON_EXEC (stderr_fd); + } /* Run the command. */ pid = exec_command (argv, envp); - /* Restore stdout/stdin of the parent and close temporary FDs. */ - if (stdin_fd != 0) + /* Restore stdout/stdin/stderr of the parent and close temporary FDs. */ + if (save_stdin >= 0) { - if (dup2 (save_stdin, 0) != 0) + if (dup2 (save_stdin, FD_STDIN) != 0) fatal (NILF, _("Could not restore stdin\n")); else close (save_stdin); } - if (stdout_fd != 1) + if (save_stdout >= 0) { - if (dup2 (save_stdout, 1) != 1) + if (dup2 (save_stdout, FD_STDOUT) != 0) fatal (NILF, _("Could not restore stdout\n")); else close (save_stdout); } + if (save_stderr >= 0) + { + if (dup2 (save_stderr, FD_STDERR) != 0) + fatal (NILF, _("Could not restore stderr\n")); + else + close (save_stderr); + } + return pid; } @@ -2245,19 +2263,28 @@ child_execute_job (int stdin_fd, int stdout_fd, char **argv, char **envp) /* UNIX: Replace the current process with one executing the command in ARGV. - STDIN_FD and STDOUT_FD are used as the process's stdin and stdout; ENVP is - the environment of the new program. This function does not return. */ + STDIN_FD/STDOUT_FD/STDERR_FD are used as the process's stdin/stdout/stderr; + ENVP is the environment of the new program. This function does not return. */ void -child_execute_job (int stdin_fd, int stdout_fd, char **argv, char **envp) +child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd, + char **argv, char **envp) { - if (stdin_fd != 0) - (void) dup2 (stdin_fd, 0); - if (stdout_fd != 1) - (void) dup2 (stdout_fd, 1); - if (stdin_fd != 0) - (void) close (stdin_fd); - if (stdout_fd != 1) - (void) close (stdout_fd); + /* For any redirected FD, dup2() it to the standard FD then close it. */ + if (stdin_fd != FD_STDIN) + { + dup2 (stdin_fd, FD_STDIN); + close (stdin_fd); + } + + if (stdout_fd != FD_STDOUT) + dup2 (stdout_fd, FD_STDOUT); + if (stderr_fd != FD_STDERR) + dup2 (stderr_fd, FD_STDERR); + + if (stdout_fd != FD_STDOUT) + close (stdout_fd); + if (stderr_fd != FD_STDERR && stderr_fd != stdout_fd) + close (stderr_fd); /* Run the command. */ exec_command (argv, envp); diff --git a/job.h b/job.h index 967fa2b..c9cb7fa 100644 --- a/job.h +++ b/job.h @@ -123,10 +123,17 @@ char **construct_command_argv (char *line, char **restp, struct file *file, int cmd_flags, char** batch_file); #ifdef VMS int child_execute_job (char *argv, struct child *child); -#elif defined(__EMX__) -int child_execute_job (int stdin_fd, int stdout_fd, char **argv, char **envp); #else -void child_execute_job (int stdin_fd, int stdout_fd, char **argv, char **envp); +# define FD_STDIN (fileno (stdin)) +# define FD_STDOUT (fileno (stdout)) +# define FD_STDERR (fileno (stderr)) +# if defined(__EMX__) +int child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd, + char **argv, char **envp) +# else +void child_execute_job (int stdin_fd, int stdout_fd, int stderr_fd, + char **argv, char **envp) __attribute__ ((noreturn)); +# endif #endif #ifdef _AMIGA void exec_command (char **argv) __attribute__ ((noreturn)); diff --git a/main.c b/main.c index 1a6c193..bd8b478 100644 --- a/main.c +++ b/main.c @@ -2363,7 +2363,8 @@ main (int argc, char **argv, char **envp) termination. */ int pid; int r; - pid = child_execute_job (0, 1, nargv, environ); + pid = child_execute_job (FD_STDIN, FD_STDOUT, FD_STDERR, + nargv, environ); /* is this loop really necessary? */ do { diff --git a/tests/ChangeLog b/tests/ChangeLog index 4ee7265..e0e9475 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,5 +1,8 @@ 2013-09-21 Paul Smith + * scripts/features/output-sync: Test shell functions writing to + stderr in recipes: ensure it's captured via output-sync. + * scripts/options/dash-w: Add a test for -w flag. 2013-09-15 Paul Smith diff --git a/tests/scripts/features/output-sync b/tests/scripts/features/output-sync index a6b2456..e09505f 100644 --- a/tests/scripts/features/output-sync +++ b/tests/scripts/features/output-sync @@ -261,5 +261,11 @@ all: run_make_test(undef, '-j -Otarget', "foobar\ntrue\n"); +# Ensure that shell functions inside recipes write stderr to the sync file +run_make_test(q! +all: ; @: $(shell echo foo 1>&2) +!, + '-w -Oline', "#MAKE#: Entering directory '#PWD#'\nfoo\n#MAKE#: Leaving directory '#PWD#'\n"); + # This tells the test driver that the perl test script executed properly. 1; -- cgit v1.2.3