From 9cd01958da86a68d0e47defcb9745ab373ef3d79 Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
Date: Sat, 21 Sep 2013 15:23:05 -0400
Subject: [PATCH] Ensure that stderr from shell functions in recipes is synced.
---
ChangeLog | 16 ++++
function.c | 10 +-
job.c | 145 +++++++++++++++++------------
job.h | 13 ++-
main.c | 3 +-
tests/ChangeLog | 3 +
tests/scripts/features/output-sync | 6 ++
7 files changed, 129 insertions(+), 67 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 861d841d..f4a897e8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2013-09-21 Paul Smith <psmith@gnu.org>
+
+ 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 <psmith@gnu.org>
* main.c (main): Set MAKE_RESTARTS to negative before re-exec if
diff --git a/function.c b/function.c
index 4ea24465..866ac48b 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 6480d8de..79d25ee3 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;
+ int save_stdin = -1;
+ int save_stdout = -1;
+ int save_stderr = -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"));
+ /* 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);
- /* 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);
+ dup2 (stdin_fd, FD_STDIN);
+ CLOSE_ON_EXEC (stdin_fd);
+ }
- /* 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);
+ 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);
- /* 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);
+ 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 967fa2bf..c9cb7fa3 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 1a6c193c..bd8b478a 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 4ee7265b..e0e9475e 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,8 @@
2013-09-21 Paul Smith <psmith@gnu.org>
+ * 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 <psmith@gnu.org>
diff --git a/tests/scripts/features/output-sync b/tests/scripts/features/output-sync
index a6b24563..e09505fd 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;