MAKE: some older keywords

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

MAKE: some older keywords

Marc Espie-2
So there is some luggage in make that I think we should drop.

Currently, our make "supports" the keywords
.EXEC, .INVISIBLE, .JOIN, .MADE,
to the extent that I never touched the code that handles those, but
frankly I don't quite see their purpose (they're hacks) and they
complicate matters.

A quick grep shows that nothing in base/xenocara actually uses them.

To be sure, I'm going to "poison" them and do a full build of everything
including ports.

If you want to know what they are supposed to do, it's documented in
PSD.doc/make.ms

I've attached the pdf file generated through groff.

Yes, it's not even in our manpage (except for .MADE)

There are two possible options there: actually fully document those keywords
(and probably grab some examples from the tutorial, because frankly, it's
fairly hard to make sense of them) or drop the keywords entirely as they
appear to be completely unused.

I would lean toward the second option. I'm pretty sure that stuff is not
even correctly supported, to the detriment of actually having a modern
make that works.

(if you read that tutorial, you'll notice the author isn't even sure about
.INVISIBLE, btw)

Note that this wouldn't be the first time we would neuter legacy make commands,
we already did so for .INCLUDES, .LIBS, .NULL...

tut.pdf (160K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MAKE: some older keywords

Marc Espie-2
More specifically, I'm running with this patch

The specific choice of keywords to deprecate is up for grabs, the
infrastructure for actually being able to error out on these keywords is
probably something I should commit anyhow.



diff --git a/gnode.h b/gnode.h
index 283fead..04e5462 100644
--- a/gnode.h
+++ b/gnode.h
@@ -77,6 +77,8 @@
 #define SPECIAL_NOTHING 6U /* this is used for things we
  * recognize for compatibility but
  * don't do anything with... */
+#define SPECIAL_DEPRECATED 7U /* this is an old keyword and it will
+ * trigger a fatal error. */
 #define SPECIAL_INVISIBLE 8U
 #define SPECIAL_JOIN 9U
 #define SPECIAL_MADE 11U
diff --git a/parse.c b/parse.c
index dfc2abc..555d2cd 100644
--- a/parse.c
+++ b/parse.c
@@ -184,13 +184,13 @@ static struct {
  unsigned int special;
  unsigned int special_op;
 } specials[] = {
-    { P(NODE_EXEC), SPECIAL_EXEC, OP_EXEC },
+    { P(NODE_EXEC), SPECIAL_DEPRECATED, OP_EXEC },
     { P(NODE_IGNORE), SPECIAL_IGNORE, OP_IGNORE },
-    { P(NODE_INCLUDES), SPECIAL_NOTHING, 0 },
-    { P(NODE_INVISIBLE), SPECIAL_INVISIBLE, OP_INVISIBLE },
-    { P(NODE_JOIN), SPECIAL_JOIN, OP_JOIN },
-    { P(NODE_LIBS), SPECIAL_NOTHING, 0 },
-    { P(NODE_MADE), SPECIAL_MADE, OP_MADE },
+    { P(NODE_INCLUDES), SPECIAL_DEPRECATED, 0 },
+    { P(NODE_INVISIBLE), SPECIAL_DEPRECATED, OP_INVISIBLE },
+    { P(NODE_JOIN), SPECIAL_DEPRECATED, OP_JOIN },
+    { P(NODE_LIBS), SPECIAL_DEPRECATED, 0 },
+    { P(NODE_MADE), SPECIAL_DEPRECATED, OP_MADE },
     { P(NODE_MAIN), SPECIAL_MAIN, 0 },
     { P(NODE_MAKE), SPECIAL_MAKE, OP_MAKE },
     { P(NODE_MAKEFLAGS), SPECIAL_MFLAGS, 0 },
@@ -198,7 +198,7 @@ static struct {
     { P(NODE_NOTMAIN), SPECIAL_NOTMAIN, OP_NOTMAIN },
     { P(NODE_NOTPARALLEL), SPECIAL_NOTPARALLEL, 0 },
     { P(NODE_NO_PARALLEL), SPECIAL_NOTPARALLEL, 0 },
-    { P(NODE_NULL), SPECIAL_NOTHING, 0 },
+    { P(NODE_NULL), SPECIAL_DEPRECATED, 0 },
     { P(NODE_OPTIONAL), SPECIAL_OPTIONAL, OP_OPTIONAL },
     { P(NODE_ORDER), SPECIAL_ORDER, 0 },
     { P(NODE_PARALLEL), SPECIAL_PARALLEL, 0 },
@@ -418,6 +418,11 @@ ParseDoSrc(
     const char *esrc)
 {
  GNode *gn = Targ_FindNodei(src, esrc, TARG_CREATE);
+ if (gn->special == SPECIAL_DEPRECATED) {
+ Parse_Error(PARSE_FATAL, "Deprecated keyword found %s\n",
+    gn->name);
+ return;
+ }
  if (gn->special_op) {
  Array_ForEach(targets, ParseDoSpecial, gn->special_op);
  return;
@@ -702,6 +707,13 @@ handle_special_targets(Lst paths)
 
  for (i = 0; i < gtargets.n; i++) {
  type = gtargets.a[i]->special;
+ if (type == SPECIAL_DEPRECATED) {
+ Parse_Error(PARSE_FATAL,
+    "Deprecated keyword found %s\n",
+    gtargets.a[i]->name);
+ specType = SPECIAL_ERROR;
+ return 0;
+ }
  if (type == SPECIAL_PATH) {
  seen_path++;
  Lst_AtEnd(paths, find_suffix_path(gtargets.a[i]));

Reply | Threaded
Open this post in threaded view
|

Re: MAKE: some older keywords

Ingo Schwarze
In reply to this post by Marc Espie-2
Hi Marc,

Marc Espie wrote on Wed, Jan 15, 2020 at 01:52:37PM +0100:

> So there is some luggage in make that I think we should drop.
>
> Currently, our make "supports" the keywords
> .EXEC, .INVISIBLE, .JOIN, .MADE,
> to the extent that I never touched the code that handles those, but
> frankly I don't quite see their purpose (they're hacks) and they
> complicate matters.
>
> A quick grep shows that nothing in base/xenocara actually uses them.
>
> To be sure, I'm going to "poison" them and do a full build of everything
> including ports.

I don't think i can meaningfully review your patch to the code,
but when all the above tests complete to your satisfaction, i certainly
don't object to removing these keywords.

> If you want to know what they are supposed to do, it's documented in
> PSD.doc/make.ms
>
> I've attached the pdf file generated through groff.
>
> Yes, it's not even in our manpage (except for .MADE)
>
> There are two possible options there: actually fully document those keywords
> (and probably grab some examples from the tutorial, because frankly, it's
> fairly hard to make sense of them) or drop the keywords entirely as they
> appear to be completely unused.
>
> I would lean toward the second option. I'm pretty sure that stuff is not
> even correctly supported, to the detriment of actually having a modern
> make that works.

If your ports bulk build goes well, i fully support no longer documenting
them at all.  I mean, that's stuff then

 * not required by the standard
 * not used by anything
 * likely broken for quite some time

so it would be nothing but a distraction in the documentation.
Documentation is better when all parts of it are relevant.

If you see some good arguments why one or more of them should be
documented, i wouldn't stand in the way either, but from what you
said so far, i don't see the point.

If you run into *significant* use in the ports tree, that might
possibly modify the situation, of course.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: MAKE: some older keywords

Todd C. Miller-3
In reply to this post by Marc Espie-2
On Wed, 15 Jan 2020 13:52:37 +0100, Marc Espie wrote:

> Currently, our make "supports" the keywords
> .EXEC, .INVISIBLE, .JOIN, .MADE,
> to the extent that I never touched the code that handles those, but
> frankly I don't quite see their purpose (they're hacks) and they
> complicate matters.
>
> A quick grep shows that nothing in base/xenocara actually uses them.
>
> To be sure, I'm going to "poison" them and do a full build of everything
> including ports.
>
> There are two possible options there: actually fully document those keywords
> (and probably grab some examples from the tutorial, because frankly, it's
> fairly hard to make sense of them) or drop the keywords entirely as they
> appear to be completely unused.
>
> I would lean toward the second option. I'm pretty sure that stuff is not
> even correctly supported, to the detriment of actually having a modern
> make that works.

I concur.  If nothing is using them then it makes sense to drop
them.  I did read through the descriptions in make.ms and nothing
in there makes me think we need them.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: MAKE: some older keywords

Marc Espie-2
So I did a full bulk build with the following diff.
No failure due to old keyword.

This contains exactly:
- abort directly instead of setting pgn->must_make = false
- tag as "SPECIAL_DEPRECATED" all old keywords
- remove the conditionals with those OP. We just keep OP_INVISIBLE  because
it's actually also used by cohorts (those pesky :: dependencies in imake)



diff --git a/compat.c b/compat.c
index fd78d78..af18ce3 100644
--- a/compat.c
+++ b/compat.c
@@ -101,24 +101,13 @@ CompatMake(void *gnp, /* The node to make */
  if (pgn == NULL)
  pgn = gn;
 
- if (pgn->type & OP_MADE) {
- sib = gn;
- do {
- sib->mtime = gn->mtime;
- sib->built_status = UPTODATE;
- sib = sib->sibling;
- } while (sib != gn);
- }
-
  switch(gn->built_status) {
  case UNKNOWN:
  /* First mark ourselves to be built, then apply whatever
  * transformations the suffix module thinks are necessary.
  * Once that's done, we can descend and make all our children.
  * If any of them has an error but the -k flag was given,
- * our 'must_make' field will be set false again.  This is our
- * signal to not attempt to do anything but abort our
- * parent as well.  */
+ * we will abort. */
  gn->must_make = true;
  gn->built_status = BUILDING;
  /* note that, in case we have siblings, we only check all
@@ -132,10 +121,9 @@ CompatMake(void *gnp, /* The node to make */
  sib = sib->sibling;
  } while (sib != gn);
 
- if (!gn->must_make) {
+ if (gn->built_status == ABORTED) {
  Error("Build for %s aborted", gn->name);
- gn->built_status = ABORTED;
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
  return;
  }
 
@@ -228,12 +216,10 @@ CompatMake(void *gnp, /* The node to make */
  if (DEBUG(MAKE))
  printf("update time: %s\n",
     time_to_string(&gn->mtime));
- if (!(gn->type & OP_EXEC)) {
- pgn->child_rebuilt = true;
- Make_TimeStamp(pgn, gn);
- }
+ pgn->child_rebuilt = true;
+ Make_TimeStamp(pgn, gn);
  } else if (keepgoing)
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
  else {
  print_errors();
  exit(1);
@@ -242,22 +228,19 @@ CompatMake(void *gnp, /* The node to make */
  case ERROR:
  /* Already had an error when making this beastie. Tell the
  * parent to abort.  */
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
  break;
  case BUILDING:
  Error("Graph cycles through %s", gn->name);
  gn->built_status = ERROR;
- pgn->must_make = false;
+ pgn->built_status = ABORTED;
  break;
  case REBUILT:
- if ((gn->type & OP_EXEC) == 0) {
- pgn->child_rebuilt = true;
- Make_TimeStamp(pgn, gn);
- }
+ pgn->child_rebuilt = true;
+ Make_TimeStamp(pgn, gn);
  break;
  case UPTODATE:
- if ((gn->type & OP_EXEC) == 0)
- Make_TimeStamp(pgn, gn);
+ Make_TimeStamp(pgn, gn);
  break;
  default:
  break;
diff --git a/dump.c b/dump.c
index b3820eb..f35cacf 100644
--- a/dump.c
+++ b/dump.c
@@ -117,7 +117,7 @@ TargPrintNode(GNode *gn, bool full)
  }
  if (full) {
  printf("# %d unmade prerequisites\n", gn->children_left);
- if (! (gn->type & (OP_JOIN|OP_USE|OP_EXEC))) {
+ if (! (gn->type & OP_USE)) {
  if (!is_out_of_date(gn->mtime)) {
  printf("# last modified %s: %s\n",
       time_to_string(&gn->mtime),
diff --git a/engine.c b/engine.c
index 535d8dd..4d7c128 100644
--- a/engine.c
+++ b/engine.c
@@ -241,7 +241,7 @@ void
 Job_Touch(GNode *gn)
 {
  handle_all_signals();
- if (gn->type & (OP_JOIN|OP_USE|OP_EXEC|OP_OPTIONAL|OP_PHONY)) {
+ if (gn->type & (OP_USE|OP_OPTIONAL|OP_PHONY)) {
  /*
  * .JOIN, .USE, and .OPTIONAL targets are "virtual" targets
  * and, as such, shouldn't really be created.
@@ -347,7 +347,7 @@ Make_DoAllVar(GNode *gn)
 
  for (ln = Lst_First(&gn->children); ln != NULL; ln = Lst_Adv(ln)) {
  child = Lst_Datum(ln);
- if ((child->type & (OP_EXEC|OP_USE|OP_INVISIBLE)) != 0)
+ if ((child->type & (OP_USE|OP_INVISIBLE)) != 0)
  continue;
  if (OP_NOP(child->type) ||
     (target = Var(TARGET_INDEX, child)) == NULL) {
@@ -371,10 +371,7 @@ Make_DoAllVar(GNode *gn)
  * hosed.
  */
  do_oodate = false;
- if (gn->type & OP_JOIN) {
- if (child->built_status == REBUILT)
- do_oodate = true;
- } else if (is_strictly_before(gn->mtime, child->mtime) ||
+ if (is_strictly_before(gn->mtime, child->mtime) ||
    (!is_strictly_before(child->mtime, starttime) &&
    child->built_status == REBUILT))
    do_oodate = true;
@@ -413,9 +410,6 @@ Make_DoAllVar(GNode *gn)
 
  if (gn->impliedsrc)
  Var(IMPSRC_INDEX, gn) = Var(TARGET_INDEX, gn->impliedsrc);
-
- if (gn->type & OP_JOIN)
- Var(TARGET_INDEX, gn) = Var(ALLSRC_INDEX, gn);
 }
 
 /* Wrapper to call Make_TimeStamp from a forEach loop. */
@@ -434,7 +428,7 @@ Make_OODate(GNode *gn)
  * Certain types of targets needn't even be sought as their datedness
  * doesn't depend on their modification time...
  */
- if ((gn->type & (OP_JOIN|OP_USE|OP_EXEC|OP_PHONY)) == 0) {
+ if ((gn->type & (OP_USE|OP_PHONY)) == 0) {
  (void)Dir_MTime(gn);
  if (DEBUG(MAKE)) {
  if (!is_out_of_date(gn->mtime))
@@ -462,15 +456,7 @@ Make_OODate(GNode *gn)
  if (DEBUG(MAKE))
  printf(".USE node...");
  oodate = false;
- } else if (gn->type & OP_JOIN) {
- /*
- * A target with the .JOIN attribute is only considered
- * out-of-date if any of its children was out-of-date.
- */
- if (DEBUG(MAKE))
- printf(".JOIN node...");
- oodate = gn->child_rebuilt;
- } else if (gn->type & (OP_FORCE|OP_EXEC|OP_PHONY)) {
+ } else if (gn->type & (OP_FORCE|OP_PHONY)) {
  /*
  * A node which is the object of the force (!) operator or which
  * has the .EXEC attribute is always considered out-of-date.
diff --git a/expandchildren.c b/expandchildren.c
index 28a1004..4ceb5c1 100644
--- a/expandchildren.c
+++ b/expandchildren.c
@@ -66,7 +66,7 @@ LinkParent(GNode *cgn, GNode *pgn)
  Lst_AtEnd(&cgn->parents, pgn);
  if (!has_been_built(cgn))
  pgn->children_left++;
- else if ( ! (cgn->type & (OP_EXEC|OP_USE))) {
+ else if ( ! (cgn->type & OP_USE)) {
  if (cgn->built_status == REBUILT)
  pgn->child_rebuilt = true;
  (void)Make_TimeStamp(pgn, cgn);
diff --git a/gnode.h b/gnode.h
index 283fead..a787955 100644
--- a/gnode.h
+++ b/gnode.h
@@ -77,6 +77,8 @@
 #define SPECIAL_NOTHING 6U /* this is used for things we
  * recognize for compatibility but
  * don't do anything with... */
+#define SPECIAL_DEPRECATED 7U /* this is an old keyword and it will
+ * trigger a fatal error. */
 #define SPECIAL_INVISIBLE 8U
 #define SPECIAL_JOIN 9U
 #define SPECIAL_MADE 11U
@@ -186,10 +188,6 @@ struct command
 #define OP_OPTIONAL 0x00000008  /* Don't care if the target doesn't
      * exist and can't be created */
 #define OP_USE 0x00000010  /* Use associated commands for parents */
-#define OP_EXEC 0x00000020  /* Target is never out of date, but always
-     * execute commands anyway. Its time
-     * doesn't matter, so it has none...sort
-     * of */
 #define OP_IGNORE 0x00000040  /* Ignore errors when creating the node */
 #define OP_PRECIOUS 0x00000080  /* Don't remove the target when
      * interrupted */
@@ -198,13 +196,10 @@ struct command
      * commands should always be executed when
      * it is out of date, regardless of the
      * state of the -n or -t flags */
-#define OP_JOIN 0x00000400  /* Target is out-of-date only if any of its
-     * children was out-of-date */
-#define OP_MADE 0x00000800  /* Assume the node is already made; even if
-     * it really is out of date */
 #define OP_INVISIBLE 0x00001000  /* The node is invisible to its parents.
      * I.e. it doesn't show up in the parents's
-     * local variables. */
+     * local variables. Used by :: for
+     * supplementary nodes (cohorts). */
 #define OP_NOTMAIN 0x00002000  /* The node is exempt from normal 'main
      * target' processing in parse.c */
 #define OP_PHONY 0x00004000  /* Not a file target; run always */
@@ -232,7 +227,7 @@ struct command
  */
 #define OP_NOP(t) (((t) & OP_OPMASK) == OP_ZERO)
 
-#define OP_NOTARGET (OP_NOTMAIN|OP_USE|OP_EXEC|OP_TRANSFORM)
+#define OP_NOTARGET (OP_NOTMAIN|OP_USE|OP_TRANSFORM)
 
 
 #endif
diff --git a/make.1 b/make.1
index b653025..0b56a4f 100644
--- a/make.1
+++ b/make.1
@@ -1405,8 +1405,6 @@ Mark its prerequisites as
 Command lines attached to this target are executed if
 .Nm
 is interrupted by a SIGINT.
-.It Ic .MADE
-Mark its prerequisites as being up to date.
 .It Ic .MAKE
 Mark its prerequisites as
 .Dq Always build .
diff --git a/make.c b/make.c
index 1eff38e..1f0a7a2 100644
--- a/make.c
+++ b/make.c
@@ -279,7 +279,7 @@ Make_Update(GNode *cgn) /* the child node */
  printf("%s--=%d ",
     pgn->name, pgn->children_left);
 
- if ( ! (cgn->type & (OP_EXEC|OP_USE))) {
+ if ( ! (cgn->type & OP_USE)) {
  if (cgn->built_status == REBUILT)
  pgn->child_rebuilt = true;
  (void)Make_TimeStamp(pgn, cgn);
@@ -388,15 +388,6 @@ try_to_make_node(GNode *gn)
  if (DEBUG(MAKE))
  printf("up-to-date\n");
  gn->built_status = UPTODATE;
- if (gn->type & OP_JOIN) {
- /*
- * Even for an up-to-date .JOIN node, we need its
- * local variables, so that we have the right
- * value for .TARGET when computing the
- * local variables of its parent(s)...
- */
- Make_DoAllVar(gn);
- }
 
  Make_Update(gn);
  }
diff --git a/parse.c b/parse.c
index dfc2abc..100a048 100644
--- a/parse.c
+++ b/parse.c
@@ -184,13 +184,13 @@ static struct {
  unsigned int special;
  unsigned int special_op;
 } specials[] = {
-    { P(NODE_EXEC), SPECIAL_EXEC, OP_EXEC },
+    { P(NODE_EXEC), SPECIAL_DEPRECATED, 0 },
     { P(NODE_IGNORE), SPECIAL_IGNORE, OP_IGNORE },
-    { P(NODE_INCLUDES), SPECIAL_NOTHING, 0 },
-    { P(NODE_INVISIBLE), SPECIAL_INVISIBLE, OP_INVISIBLE },
-    { P(NODE_JOIN), SPECIAL_JOIN, OP_JOIN },
-    { P(NODE_LIBS), SPECIAL_NOTHING, 0 },
-    { P(NODE_MADE), SPECIAL_MADE, OP_MADE },
+    { P(NODE_INCLUDES), SPECIAL_DEPRECATED, 0 },
+    { P(NODE_INVISIBLE), SPECIAL_DEPRECATED, 0 },
+    { P(NODE_JOIN), SPECIAL_DEPRECATED, 0 },
+    { P(NODE_LIBS), SPECIAL_DEPRECATED, 0 },
+    { P(NODE_MADE), SPECIAL_DEPRECATED, 0 },
     { P(NODE_MAIN), SPECIAL_MAIN, 0 },
     { P(NODE_MAKE), SPECIAL_MAKE, OP_MAKE },
     { P(NODE_MAKEFLAGS), SPECIAL_MFLAGS, 0 },
@@ -198,7 +198,7 @@ static struct {
     { P(NODE_NOTMAIN), SPECIAL_NOTMAIN, OP_NOTMAIN },
     { P(NODE_NOTPARALLEL), SPECIAL_NOTPARALLEL, 0 },
     { P(NODE_NO_PARALLEL), SPECIAL_NOTPARALLEL, 0 },
-    { P(NODE_NULL), SPECIAL_NOTHING, 0 },
+    { P(NODE_NULL), SPECIAL_DEPRECATED, 0 },
     { P(NODE_OPTIONAL), SPECIAL_OPTIONAL, OP_OPTIONAL },
     { P(NODE_ORDER), SPECIAL_ORDER, 0 },
     { P(NODE_PARALLEL), SPECIAL_PARALLEL, 0 },
@@ -418,6 +418,11 @@ ParseDoSrc(
     const char *esrc)
 {
  GNode *gn = Targ_FindNodei(src, esrc, TARG_CREATE);
+ if (gn->special == SPECIAL_DEPRECATED) {
+ Parse_Error(PARSE_FATAL, "Deprecated keyword found %s\n",
+    gn->name);
+ return;
+ }
  if (gn->special_op) {
  Array_ForEach(targets, ParseDoSpecial, gn->special_op);
  return;
@@ -702,6 +707,13 @@ handle_special_targets(Lst paths)
 
  for (i = 0; i < gtargets.n; i++) {
  type = gtargets.a[i]->special;
+ if (type == SPECIAL_DEPRECATED) {
+ Parse_Error(PARSE_FATAL,
+    "Deprecated keyword found %s\n",
+    gtargets.a[i]->name);
+ specType = SPECIAL_ERROR;
+ return 0;
+ }
  if (type == SPECIAL_PATH) {
  seen_path++;
  Lst_AtEnd(paths, find_suffix_path(gtargets.a[i]));
diff --git a/targ.c b/targ.c
index bba0192..a8f3db6 100644
--- a/targ.c
+++ b/targ.c
@@ -307,12 +307,10 @@ Targ_PrintType(int type)
  switch (tbit) {
  PRINTBIT(OPTIONAL);
  PRINTBIT(USE);
- PRINTBIT(EXEC);
  PRINTBIT(IGNORE);
  PRINTBIT(PRECIOUS);
  PRINTBIT(SILENT);
  PRINTBIT(MAKE);
- PRINTBIT(JOIN);
  PRINTBIT(INVISIBLE);
  PRINTBIT(NOTMAIN);
  /*XXX: MEMBER is defined, so CONCAT(OP_,MEMBER) gives OP_"%" */