Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make make(1): in jobs mode, extract writing of shell...



details:   https://anonhg.NetBSD.org/src/rev/d98119caa08e
branches:  trunk
changeset: 947133:d98119caa08e
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sat Dec 12 01:42:33 2020 +0000

description:
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.

diffstat:

 usr.bin/make/job.c |  79 +++++++++++++++++++++++++++++++++--------------------
 usr.bin/make/job.h |   5 +--
 2 files changed, 51 insertions(+), 33 deletions(-)

diffs (235 lines):

diff -r 8c9e27730325 -r d98119caa08e usr.bin/make/job.c
--- a/usr.bin/make/job.c        Sat Dec 12 00:53:23 2020 +0000
+++ b/usr.bin/make/job.c        Sat Dec 12 01:42:33 2020 +0000
@@ -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 @@
 } 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 @@
 }
 
 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
-JobPrintln(Job *job, const char *line)
+ShellWriter_PrintCmd(ShellWriter *wr, const char *tmpl, const char *escCmd)
 {
-       JobPrintf(job, "%s\n", line);
+       ShellWriter_PrintFmt(wr, tmpl, escCmd);
+}
+
+static void
+ShellWriter_Println(ShellWriter *wr, const char *line)
+{
+       ShellWriter_PrintFmt(wr, "%s\n", line);
 }
 
 /*
@@ -761,14 +778,14 @@
  * 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 @@
  * 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 @@
 }
 
 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 @@
  * 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 @@
 
        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 @@
        }
 
        if (cmdFlags.ignerr) {
-               JobPrintSpecials(job, escCmd, run, &cmdFlags, &cmdTemplate);
+               JobPrintSpecials(job, wr, escCmd, run, &cmdFlags, &cmdTemplate);
        } else {
 
                /*
@@ -897,8 +914,9 @@
                    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 @@
        }
 
        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 @@
                 * 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 @@
 {
        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 @@
                        break;
                }
 
-               JobPrintCommand(job, ln->datum);
+               JobPrintCommand(job, &wr, ln->datum);
                seen = TRUE;
        }
 
diff -r 8c9e27730325 -r d98119caa08e usr.bin/make/job.h
--- a/usr.bin/make/job.h        Sat Dec 12 00:53:23 2020 +0000
+++ b/usr.bin/make/job.h        Sat Dec 12 01:42:33 2020 +0000
@@ -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 @@
      * 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 */



Home | Main Index | Thread Index | Old Index