Skip to content

Commit

Permalink
mamake: show an even more useful trace (re: 3bd7e69)
Browse files Browse the repository at this point in the history
I still wasn't happy with it. It would be nice to trace the rule
name/target as well, but doing that on the same line makes the PS4
prefix too long and the trace too hard to read. And the log still
looks crowded and hard to read anyway. So, this commit takes an
entirely different approach.

src/cmd/INIT/mamake.c:
- run(): Revert the PS4 changes.
- make(): case 'done': Before running each shell action, directly
  print an empty line (for legibility) followed by trace header to
  standard error that looks like:
  # src/cmd/ksh93/Mamfile: 22-1188: ksh
  ...so, the Mamfile path relative to ${PACKAGEROOT}, the line
  number range of the make...done block, and the rule target.
  In the latter, if the target is a canonical path starting with
  the value of ${INSTALLROOT}, it is rendered like this:
  # src/lib/libast/Mamfile: 4325-4329: ${INSTALLROOT}/lib/libast.a
  ...both for brevity and to reflect that this is how the target
  rules are actually written.
- make(): case 'done': Improve the -e ("explain") option: make the
  presentation match the new trace header and recognise a virtual
  target (which was previously misexplained as 'target not found').
- update(): Add simple trace header and optional -e explanation for
  recursive mamake invocations.
- main(): Disable state.explain if state.force is active -- this
  causes -e to be ignored if -F is given (as documented) without
  checking for !state.force each time state.explain is checked.
  • Loading branch information
McDutchie committed Feb 5, 2024
1 parent 199d708 commit 0a2e5d4
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 23 deletions.
10 changes: 10 additions & 0 deletions src/cmd/INIT/README-mamake.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ In the legacy mode, `info` and `meta` are also ignored.
A `make`...`done` block defines the target rule named *rule* using the other commands described here.
Unless the `virtual` attribute is used, *rule* names the pathname of the file generated or referenced by the rule.

`mamake` processes the commands within the block if the *rule* target is out
of date or if the rule has the `virtual` attribute (see below).

The *rule* name may be repeated as the operand to the `done` command.
In that case, it is matched against the current `make` *rule* and
any mismatch will produce a "mismatched done statement" error.
Expand Down Expand Up @@ -117,6 +120,7 @@ The following *attribute*s are available:
then `foo.h` should be marked as an implicit prerequisite of `foo.c`
so that touching `foo.h` does not make `foo.c` out of date while making `foo.o` out of date.
* `notrace`: Disables echoing (xtrace) of shell action commands.
This does not disable the trace header for the containing rule (see *Shell actions* below).
* `virtual`: Marks a rule that is not associated with any file.
The commands within are executed every time the Mamfile is processed.
By convention, a virtual rule named `all` makes everything,
Expand Down Expand Up @@ -195,6 +199,12 @@ it is replaced by the canonical path to it in the source directory.
When `mamake` encounters the `done` command,
the script is executed by the shell whose path is in the `SHELL` environment variable
or, absent that, by `/bin/sh`.
Before executing the script, a trace header in the following format is added to the log:
```
# path/to/Mamfile: startline-endline: rule
```
During script execution, shell action comands are traced using the
shell's xtrace option, unless the rule has the `notrace` attribute.

### Binding libraries ###

Expand Down
58 changes: 35 additions & 23 deletions src/cmd/INIT/mamake.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* coded for portability
*/

#define RELEASE_DATE "2024-02-04"
#define RELEASE_DATE "2024-02-05"
static char id[] = "\n@(#)$Id: mamake (ksh 93u+m) " RELEASE_DATE " $\0\n";

#if _PACKAGE_ast
Expand Down Expand Up @@ -1276,25 +1276,7 @@ run(Rule_t* r, char* s)
);
/* show trace for the shell action commands */
if (!(r->flags & RULE_notrace))
{
char *cp;
/* construct a nice PS4 */
append(buf,"PS4='+ (");
cp = state.pwd + strlen(state.pwd) - 1;
while (*cp != '/' && *cp != '\'' && cp > state.pwd)
cp--;
append(buf, cp + 1);
add(buf, '/');
append(buf, state.file);
if (r->line && state.sp)
{
char nbuf[64];
sprintf(nbuf, ":%lu-%lu", r->line, state.sp->line);
append(buf, nbuf);
}
append(buf,") '\n"
"set -x\n");
}
append(buf,"set -x\n");
}
if (state.view)
{
Expand Down Expand Up @@ -1770,14 +1752,34 @@ make(Rule_t* r)
}
if (cmd && state.active && (state.force || r->time < z || !r->time && !z))
{
if (state.explain && !state.force)
char *fname = state.sp->file, *rname = r->name, *rnamepre = "", *val;
int len;
/* show a nice trace header */
/* ...mamfile path: make relative to ${PACKAGEROOT} */
if (*fname == '/'
&& (val = search(state.vars, "PACKAGEROOT", NULL)) && (len = strlen(val))
&& strncmp(fname, val, len) == 0 && fname[len] == '/' && fname[++len])
fname += len;
/* ...rule name: change install root path prefix back to '${INSTALLROOT}' for brevity */
if (*rname == '/'
&& (val = search(state.vars, "INSTALLROOT", NULL)) && (len = strlen(val))
&& strncmp(rname, val, len) == 0 && rname[len] == '/' && rname[len + 1])
rname += len, rnamepre = "${INSTALLROOT}";
fprintf(stderr, "\n# %s: %lu-%lu: make %s%s\n",
fname, r->line, state.sp->line, rnamepre, rname);
/* -e option */
if (state.explain)
{
fprintf(stderr, "# reason: ");
if (!r->time)
fprintf(stderr, "%s [not found]\n", r->name);
fprintf(stderr, "target %s\n",
(r->flags & RULE_virtual) ? "is virtual" : "not found");
else
fprintf(stderr, "%s [%lu] older than prerequisites [%lu]\n", r->name, r->time, z);
fprintf(stderr, "target [%lu] older than prerequisites [%lu]\n", r->time, z);
}
/* expand MAM variables in the shell action */
substitute(buf, use(cmd));
/* run the shell action */
x = run(r, use(buf));
if (z < x)
z = x;
Expand Down Expand Up @@ -1949,6 +1951,9 @@ update(Rule_t* r)
substitute(buf, cmd);
append(buf, r->name);
substitute(buf, arg);
fprintf(stderr, "\n# ... making %s ...\n", r->name);
if (state.explain)
fprintf(stderr, "# reason: recursion\n");
run(r, use(buf));
drop(buf);
return 0;
Expand Down Expand Up @@ -2343,6 +2348,13 @@ main(int argc, char** argv)
}
#endif

/*
* option incompatibility
*/

if (state.force)
state.explain = 0;

/*
* load the environment
*/
Expand Down

0 comments on commit 0a2e5d4

Please sign in to comment.