Subject: patches to fix make -j
To: None <tech-toolchain@netbsd.org>
From: James Chacon <jmc@netbsd.org>
List: tech-toolchain
Date: 12/17/2003 12:31:53
See the previous posts for the main details but make -j fails currently on
anything that has target commands like this:
( ... )
i.e. anything in a subshell. This is due to depending on sh -ev to stop on
errors and as discussed (at length) on tech-userlevel, SUSE3 states that -e
doesn't apply to compound commands. Various ideas for fixing make were
suggested and the common consensus seems to be adding || exit $? to commands
as needed (i.e. not in -<command> cases) would work.
The following patches do this and make releases worked fine on x86 and
cesfic with -N0 -> -N2 (I use cesfic since it'll build to sets in 30m for me)
These patches stay with the current model of job.c in spec'ing a Shell
structure to define how to call commands. The default is "sh" and to make
this work I added 2 fields and reworked some code in JobPrintCommand()
to deal with shells that have no error control but can provide a way otherwise
to check for errors. Turns out adding '|| exit $?' isn't quite that simple
as commands ending in ; will break it, etc. So the whole command needs to
get wrapped in {...\n} || exit $? (and yes the newline is needed).
The main reason for new fields and doing it this way was to avoid gutting
the code entirely as far as spec'ing a shell and instead hard coding /bin/sh.
The main changes have to deal with printing out the command being run vs the
actual command (i.e. so the user doesn't have to see the {} || exit stuff).
At this point '|| exit $?' vs '&&' comes out the same except that the exit's
are easier to follow in most cases when debugging the shell input script.
Concerns/comments welcome. Otherwise I'll probably commit these in the next
few days barring objections.
James
Index: job.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/job.c,v
retrieving revision 1.82
diff -u -r1.82 job.c
--- job.c 7 Aug 2003 11:14:51 -0000 1.82
+++ job.c 17 Dec 2003 17:48:29 -0000
@@ -218,7 +218,7 @@
{
"csh",
TRUE, "unset verbose", "set verbose", "unset verbose", 10,
- FALSE, "echo \"%s\"\n", "csh -c \"%s || exit 0\"",
+ FALSE, "echo \"%s\"\n", "csh -c \"%s || exit 0\"", "", '#',
"v", "e",
},
/*
@@ -227,17 +227,14 @@
*/
{
"sh",
- TRUE, "set -", "set -v", "set -", 5,
- TRUE, "set -e", "set +e",
-#ifdef OLDBOURNESHELL
- FALSE, "echo \"%s\"\n", "sh -c '%s || exit 0'\n",
-#endif
+ FALSE, "", "", "", 0,
+ FALSE, "echo \"%s\"\n", "%s\n", "{ %s \n} || exit $?\n", '#',
#ifdef __NetBSD__
- "vq",
+ "q",
#else
- "v",
+ "",
#endif
- "e",
+ "",
},
/*
* KSH description.
@@ -245,7 +242,7 @@
{
"ksh",
TRUE, "set +v", "set -v", "set +v", 6,
- TRUE, "set -e", "set +e",
+ TRUE, "set -e", "set +e", "", '#',
"v",
"e",
},
@@ -255,7 +252,7 @@
{
(char *) 0,
FALSE, (char *) 0, (char *) 0, (char *) 0, 0,
- FALSE, (char *) 0, (char *) 0,
+ FALSE, (char *) 0, (char *) 0, (char *) 0, 0,
(char *) 0, (char *) 0,
}
};
@@ -673,10 +670,12 @@
const char *cmdTemplate; /* Template to use when printing the
* command */
char *cmdStart; /* Start of expanded command */
+ char *escCmd = NULL; /* Command with quotes/backticks escaped */
char *cmd = (char *) cmdp;
Job *job = (Job *) jobp;
- char *cp;
-
+ char *cp;
+ int i, j;
+
noSpecials = NoExecute(job->node);
if (strcmp(cmd, "...") == 0) {
@@ -717,12 +716,31 @@
while (isspace((unsigned char) *cmd))
cmd++;
+ /*
+ * If the shell doesn't have error control the alternate echo'ing will
+ * be done (to avoid showing additional error checking code)
+ * and this will need the characters '$ ` \ "' escaped
+ */
+
+ if (!commandShell->hasErrCtl) {
+ /* Worst that could happen is every char needs escaping. */
+ escCmd = (char *) emalloc((strlen(cmd) * 2) + 1);
+ for (i = 0, j= 0; cmd[i] != '\0'; i++, j++) {
+ if (cmd[i] == '$' || cmd[i] == '`' || cmd[i] == '\\' ||
+ cmd[i] == '"')
+ escCmd[j++] = '\\';
+ escCmd[j] = cmd[i];
+ }
+ escCmd[j] = 0;
+ }
+
if (shutUp) {
if (!(job->flags & JOB_SILENT) && !noSpecials &&
commandShell->hasEchoCtl) {
DBPRINTF("%s\n", commandShell->echoOff);
} else {
- shutUp = FALSE;
+ if (commandShell->hasErrCtl)
+ shutUp = FALSE;
}
}
@@ -743,7 +761,7 @@
DBPRINTF("%s\n", commandShell->ignErr);
DBPRINTF("%s\n", commandShell->echoOn);
} else {
- DBPRINTF("%s\n", commandShell->ignErr);
+ DBPRINTF("%s\n", commandShell->ignErr);
}
} else if (commandShell->ignErr &&
(*commandShell->ignErr != '\0'))
@@ -757,11 +775,16 @@
* to ignore errors. Set cmdTemplate to use the weirdness
* instead of the simple "%s\n" template.
*/
- if (!(job->flags & JOB_SILENT) && !shutUp &&
- commandShell->hasEchoCtl) {
- DBPRINTF("%s\n", commandShell->echoOff);
- DBPRINTF(commandShell->errCheck, cmd);
+ if (!(job->flags & JOB_SILENT) && !shutUp) {
+ if (commandShell->hasEchoCtl) {
+ DBPRINTF("%s\n", commandShell->echoOff);
+ }
+ DBPRINTF(commandShell->errCheck, escCmd);
shutUp = TRUE;
+ } else {
+ if (!shutUp) {
+ DBPRINTF(commandShell->errCheck, escCmd);
+ }
}
cmdTemplate = commandShell->ignErr;
/*
@@ -776,6 +799,30 @@
} else {
errOff = FALSE;
}
+ } else {
+
+ /*
+ * If errors are being checked and the shell doesn't have error control
+ * but does supply an errOut template, then setup commands to run
+ * through it.
+ */
+
+ if (!commandShell->hasErrCtl && commandShell->errOut &&
+ (*commandShell->errOut != '\0')) {
+ if (!(job->flags & JOB_SILENT) && !shutUp) {
+ if (commandShell->hasEchoCtl) {
+ DBPRINTF("%s\n", commandShell->echoOff);
+ }
+ DBPRINTF(commandShell->errCheck, escCmd);
+ shutUp = TRUE;
+ }
+ /* If it's a comment line, treat it like an ignored error */
+ if (escCmd[0] == commandShell->commentChar)
+ cmdTemplate = commandShell->ignErr;
+ else
+ cmdTemplate = commandShell->errOut;
+ errOff = FALSE;
+ }
}
if (DEBUG(SHELL) && strcmp(shellName, "sh") == 0 &&
@@ -790,7 +837,8 @@
}
DBPRINTF(cmdTemplate, cmd);
free(cmdStart);
-
+ if (escCmd)
+ free(escCmd);
if (errOff) {
/*
* If echoing is already off, there's no point in issuing the
@@ -803,7 +851,7 @@
}
DBPRINTF("%s\n", commandShell->errCheck);
}
- if (shutUp) {
+ if (shutUp && commandShell->hasEchoCtl) {
DBPRINTF("%s\n", commandShell->echoOn);
}
return 0;
@@ -2883,6 +2931,10 @@
newShell.errCheck = &argv[0][6];
} else if (strncmp(*argv, "ignore=", 7) == 0) {
newShell.ignErr = &argv[0][7];
+ } else if (strncmp(*argv, "errout=", 7) == 0) {
+ newShell.errOut = &argv[0][7];
+ } else if (strncmp(*argv, "comment=", 8) == 0) {
+ newShell.commentChar = argv[0][8];
} else {
Parse_Error(PARSE_FATAL, "Unknown keyword \"%s\"",
*argv);
Index: job.h
===================================================================
RCS file: /cvsroot/src/usr.bin/make/job.h,v
retrieving revision 1.20
diff -u -r1.20 job.h
--- job.h 7 Aug 2003 11:14:52 -0000 1.20
+++ job.h 17 Dec 2003 17:48:29 -0000
@@ -235,9 +235,13 @@
* Some special stuff goes on if a shell doesn't have error control. In such
* a case, errCheck becomes a printf template for echoing the command,
* should echoing be on and ignErr becomes another printf template for
- * executing the command while ignoring the return status. If either of these
- * strings is empty when hasErrCtl is FALSE, the command will be executed
- * anyway as is and if it causes an error, so be it.
+ * executing the command while ignoring the return status. Finally errOut
+ * is a printf template for running the command and causing the shell to
+ * exit on error. If any of these strings are empty when hasErrCtl is FALSE,
+ * the command will be executed anyway as is and if it causes an error, so be
+ * it. Any templates setup to echo the command will escape any '$ ` \ "'i
+ * characters in the command string to avoid common problems with
+ * echo "%s\n" as a template.
*/
typedef struct Shell {
const char *name; /* the name of the shell. For Bourne and C
@@ -257,6 +261,9 @@
* individual commands */
const char *errCheck; /* string to turn error checking on */
const char *ignErr; /* string to turn off error checking */
+ const char *errOut; /* string to use for testing exit code */
+ char commentChar; /* character used by shell for comment lines */
+
/*
* command-line flags
*/