Module Name:    src
Committed By:   rillig
Date:           Sat Dec 12 01:42:33 UTC 2020

Modified Files:
        src/usr.bin/make: job.c job.h

Log Message:
make(1): in jobs mode, extract writing of shell commands

Right now, the test sh-flags.mk demonstrates many variants to configure
echoing of the shell commands (-s, .SILENT, '@'), error handling (-i,
.IGNORE, '-') and whether the commands are run (-n, -N, .MAKE,
.RECURSIVE, '+').

Even more variants are possible by configuring the shell to have error
control.  None of the built-in shell definitions has error control, so
it is unlikely that anybody uses them, but who knows.

Being able to configure these details at 3 levels is good, but what
makes all this really hard to understand is that some of these switches
interact in non-obvious ways.  For example, in jobs mode, a single
command can change job->ignerr (in JobPrintSpecialsEchoCtl), which will
affect all further commands of that job.

The goal of this refactoring is to make the code easier to understand by
making the switches on the job level constant and by moving all
modifications to them to the ShellWriter.


To generate a diff of this commit:
cvs rdiff -u -r1.370 -r1.371 src/usr.bin/make/job.c
cvs rdiff -u -r1.67 -r1.68 src/usr.bin/make/job.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/make/job.c
diff -u src/usr.bin/make/job.c:1.370 src/usr.bin/make/job.c:1.371
--- src/usr.bin/make/job.c:1.370	Sat Dec 12 00:33:25 2020
+++ src/usr.bin/make/job.c	Sat Dec 12 01:42:33 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: job.c,v 1.370 2020/12/12 00:33:25 rillig Exp $	*/
+/*	$NetBSD: job.c,v 1.371 2020/12/12 01:42:33 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -143,7 +143,7 @@
 #include "trace.h"
 
 /*	"@(#)job.c	8.2 (Berkeley) 3/19/94"	*/
-MAKE_RCSID("$NetBSD: job.c,v 1.370 2020/12/12 00:33:25 rillig Exp $");
+MAKE_RCSID("$NetBSD: job.c,v 1.371 2020/12/12 01:42:33 rillig Exp $");
 
 /*
  * A shell defines how the commands are run.  All commands for a target are
@@ -231,6 +231,16 @@ typedef struct CommandFlags {
 } CommandFlags;
 
 /*
+ * Write shell commands to a file.
+ *
+ * TODO: keep track of whether commands are echoed.
+ * TODO: keep track of whether error checking is active.
+ */
+typedef struct ShellWriter {
+	FILE *f;
+} ShellWriter;
+
+/*
  * error handling variables
  */
 static int job_errors = 0;	/* number of errors reported */
@@ -740,18 +750,25 @@ EscapeShellDblQuot(const char *cmd)
 }
 
 static void
-JobPrintf(Job *job, const char *fmt, const char *arg)
+ShellWriter_PrintFmt(ShellWriter *wr, const char *fmt, const char *arg)
 {
 	DEBUG1(JOB, fmt, arg);
 
-	(void)fprintf(job->cmdFILE, fmt, arg);
-	(void)fflush(job->cmdFILE);
+	(void)fprintf(wr->f, fmt, arg);
+	/* XXX: Is flushing needed in any case, or only if f == stdout? */
+	(void)fflush(wr->f);
+}
+
+static void
+ShellWriter_PrintCmd(ShellWriter *wr, const char *tmpl, const char *escCmd)
+{
+	ShellWriter_PrintFmt(wr, tmpl, escCmd);
 }
 
 static void
-JobPrintln(Job *job, const char *line)
+ShellWriter_Println(ShellWriter *wr, const char *line)
 {
-	JobPrintf(job, "%s\n", line);
+	ShellWriter_PrintFmt(wr, "%s\n", line);
 }
 
 /*
@@ -761,14 +778,14 @@ JobPrintln(Job *job, const char *line)
  * it any more complex than it already is?
  */
 static void
-JobPrintSpecialsErrCtl(Job *job, Boolean cmdEcho)
+JobPrintSpecialsErrCtl(Job *job, ShellWriter *wr, Boolean cmdEcho)
 {
 	if (job->echo && cmdEcho && shell->hasEchoCtl) {
-		JobPrintln(job, shell->echoOff);
-		JobPrintln(job, shell->errOff);
-		JobPrintln(job, shell->echoOn);
+		ShellWriter_Println(wr, shell->echoOff);
+		ShellWriter_Println(wr, shell->errOff);
+		ShellWriter_Println(wr, shell->echoOn);
 	} else {
-		JobPrintln(job, shell->errOff);
+		ShellWriter_Println(wr, shell->errOff);
 	}
 }
 
@@ -780,19 +797,19 @@ JobPrintSpecialsErrCtl(Job *job, Boolean
  * Set cmdTemplate to use the weirdness instead of the simple "%s\n" template.
  */
 static void
-JobPrintSpecialsEchoCtl(Job *job, CommandFlags *inout_cmdFlags,
+JobPrintSpecialsEchoCtl(Job *job, ShellWriter *wr, CommandFlags *inout_cmdFlags,
 			const char *escCmd, const char **inout_cmdTemplate)
 {
 	job->ignerr = TRUE;
 
 	if (job->echo && inout_cmdFlags->echo) {
 		if (shell->hasEchoCtl)
-			JobPrintln(job, shell->echoOff);
-		JobPrintf(job, shell->echoTmpl, escCmd);
+			ShellWriter_Println(wr, shell->echoOff);
+		ShellWriter_PrintCmd(wr, shell->echoTmpl, escCmd);
 		inout_cmdFlags->echo = FALSE;
 	} else {
 		if (inout_cmdFlags->echo)
-			JobPrintf(job, shell->echoTmpl, escCmd);
+			ShellWriter_PrintCmd(wr, shell->echoTmpl, escCmd);
 	}
 	*inout_cmdTemplate = shell->runIgnTmpl;
 
@@ -805,15 +822,15 @@ JobPrintSpecialsEchoCtl(Job *job, Comman
 }
 
 static void
-JobPrintSpecials(Job *job, const char *escCmd, Boolean run,
+JobPrintSpecials(Job *job, ShellWriter *wr, const char *escCmd, Boolean run,
 		 CommandFlags *inout_cmdFlags, const char **inout_cmdTemplate)
 {
 	if (!run)
 		inout_cmdFlags->ignerr = FALSE;
 	else if (shell->hasErrCtl)
-		JobPrintSpecialsErrCtl(job, inout_cmdFlags->echo);
+		JobPrintSpecialsErrCtl(job, wr, inout_cmdFlags->echo);
 	else if (shell->runIgnTmpl != NULL && shell->runIgnTmpl[0] != '\0') {
-		JobPrintSpecialsEchoCtl(job, inout_cmdFlags, escCmd,
+		JobPrintSpecialsEchoCtl(job, wr, inout_cmdFlags, escCmd,
 		    inout_cmdTemplate);
 	} else
 		inout_cmdFlags->ignerr = FALSE;
@@ -835,7 +852,7 @@ JobPrintSpecials(Job *job, const char *e
  * after all other targets have been made.
  */
 static void
-JobPrintCommand(Job *job, const char *ucmd)
+JobPrintCommand(Job *job, ShellWriter *wr, const char *ucmd)
 {
 	Boolean run;
 
@@ -876,7 +893,7 @@ JobPrintCommand(Job *job, const char *uc
 
 	if (!cmdFlags.echo) {
 		if (job->echo && run && shell->hasEchoCtl) {
-			JobPrintln(job, shell->echoOff);
+			ShellWriter_Println(wr, shell->echoOff);
 		} else {
 			if (shell->hasErrCtl)
 				cmdFlags.echo = TRUE;
@@ -884,7 +901,7 @@ JobPrintCommand(Job *job, const char *uc
 	}
 
 	if (cmdFlags.ignerr) {
-		JobPrintSpecials(job, escCmd, run, &cmdFlags, &cmdTemplate);
+		JobPrintSpecials(job, wr, escCmd, run, &cmdFlags, &cmdTemplate);
 	} else {
 
 		/*
@@ -897,8 +914,9 @@ JobPrintCommand(Job *job, const char *uc
 		    shell->runChkTmpl[0] != '\0') {
 			if (job->echo && cmdFlags.echo) {
 				if (shell->hasEchoCtl)
-					JobPrintln(job, shell->echoOff);
-				JobPrintf(job, shell->echoTmpl, escCmd);
+					ShellWriter_Println(wr, shell->echoOff);
+				ShellWriter_PrintCmd(wr,
+				    shell->echoTmpl, escCmd);
 				cmdFlags.echo = FALSE;
 			}
 			/*
@@ -914,11 +932,11 @@ JobPrintCommand(Job *job, const char *uc
 	}
 
 	if (DEBUG(SHELL) && strcmp(shellName, "sh") == 0 && !job->xtraced) {
-		JobPrintln(job, "set -x");
+		ShellWriter_Println(wr, "set -x");
 		job->xtraced = TRUE;
 	}
 
-	JobPrintf(job, cmdTemplate, xcmd);
+	ShellWriter_PrintCmd(wr, cmdTemplate, xcmd);
 	free(xcmdStart);
 	free(escCmd);
 	if (cmdFlags.ignerr) {
@@ -928,13 +946,13 @@ JobPrintCommand(Job *job, const char *uc
 		 * for the whole command...
 		 */
 		if (cmdFlags.echo && job->echo && shell->hasEchoCtl) {
-			JobPrintln(job, shell->echoOff);
+			ShellWriter_Println(wr, shell->echoOff);
 			cmdFlags.echo = FALSE;
 		}
-		JobPrintln(job, shell->errOn);
+		ShellWriter_Println(wr, shell->errOn);
 	}
 	if (!cmdFlags.echo && shell->hasEchoCtl)
-		JobPrintln(job, shell->echoOn);
+		ShellWriter_Println(wr, shell->echoOn);
 }
 
 /*
@@ -950,6 +968,7 @@ JobPrintCommands(Job *job)
 {
 	StringListNode *ln;
 	Boolean seen = FALSE;
+	ShellWriter wr = { job->cmdFILE };
 
 	for (ln = job->node->commands.first; ln != NULL; ln = ln->next) {
 		const char *cmd = ln->datum;
@@ -960,7 +979,7 @@ JobPrintCommands(Job *job)
 			break;
 		}
 
-		JobPrintCommand(job, ln->datum);
+		JobPrintCommand(job, &wr, ln->datum);
 		seen = TRUE;
 	}
 

Index: src/usr.bin/make/job.h
diff -u src/usr.bin/make/job.h:1.67 src/usr.bin/make/job.h:1.68
--- src/usr.bin/make/job.h:1.67	Thu Dec 10 21:41:35 2020
+++ src/usr.bin/make/job.h	Sat Dec 12 01:42:33 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: job.h,v 1.67 2020/12/10 21:41:35 rillig Exp $	*/
+/*	$NetBSD: job.h,v 1.68 2020/12/12 01:42:33 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -148,8 +148,7 @@ typedef struct Job {
      * first of these commands, if any. */
     StringListNode *tailCmds;
 
-    /* When creating the shell script, this is where the commands go.
-     * This is only used before the job is actually started. */
+    /* This is where the shell commands go. */
     FILE *cmdFILE;
 
     int exit_status;		/* from wait4() in signal handler */

Reply via email to