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
      */