Re: Proposed syntax for static make-also rule

[Henning Makholm]

Scripsit Henning Makholm <address@hidden>

> OK, plan changed. New proposed separators
> are "&:" and "|:" instead. That's more intuitive, as you say.

So far, here is a patch relative to the 3.80 tarball that implements
"&:" and "|:". I have yet to write a manual update that documents it.

Paul, what is your stance on restructuring the documentation such that
&: and |: are the main documented ways of writing multi-target
(pattern or explicit) rules with commands, and documenting ":" alone
as a shorthand whose meaning, for historical reasons, differs
according to whether the rule is a pattern rule or not?

diff -ur make-3.80.orig/commands.c make-3.80/commands.c
--- make-3.80.orig/commands.c   Fri Sep 13 00:15:58 2002
+++ make-3.80/commands.c        Sat Dec 21 02:25:48 2002
@@ -544,7 +544,8 @@
 
   /* Also remove any non-precious targets listed in the `also_make' member.  */
   for (d = child->file->also_make; d != 0; d = d->next)
-    delete_target (d->file, child->file->name);
+    if (d->file != child->file)
+      delete_target (d->file, child->file->name);
 
   child->deleted = 1;
 }
diff -ur make-3.80.orig/default.c make-3.80/default.c
--- make-3.80.orig/default.c    Sun Mar 26 08:56:54 2000
+++ make-3.80/default.c Sat Dec 21 04:52:14 2002
@@ -505,6 +505,8 @@
 #endif
 
 #endif /* !VMS */
+
+    ".MAKE_FEATURE_&:", "&",
     0, 0
   };
 
diff -ur make-3.80.orig/file.c make-3.80/file.c
--- make-3.80.orig/file.c       Fri Oct  4 04:13:42 2002
+++ make-3.80/file.c    Sat Dec 21 02:09:27 2002
@@ -533,6 +533,8 @@
 
   file->command_state = state;
 
+  /* Note: For files with "&:" rules, the also_make list contains the
+     file itself, but that does not hurt here.  */
   for (d = file->also_make; d != 0; d = d->next)
     d->file->command_state = state;
 }
diff -ur make-3.80.orig/read.c make-3.80/read.c
--- make-3.80.orig/read.c       Fri Oct  4 04:13:42 2002
+++ make-3.80/read.c    Sat Dec 21 03:50:37 2002
@@ -134,7 +134,7 @@
 static void record_files PARAMS ((struct nameseq *filenames, char *pattern, 
char *pattern_percent,
                        struct dep *deps, unsigned int cmds_started, char 
*commands,
                        unsigned int commands_idx, int two_colon,
-                       int have_sysv_atvar,
+                        int premodifier, int have_sysv_atvar,
                         const struct floc *flocp, int set_default));
 static void record_target_var PARAMS ((struct nameseq *filenames, char *defn,
                                        int two_colon,
@@ -441,6 +441,7 @@
   struct dep *deps = 0;
   long nlines = 0;
   int two_colon = 0;
+  int premodifier ;
   char *pattern = 0, *pattern_percent;
   struct floc *fstart;
   struct floc fi;
@@ -453,7 +454,7 @@
          fi.lineno = tgts_started;                                           \
          record_files (filenames, pattern, pattern_percent, deps,            \
                         cmds_started, commands, commands_idx, two_colon,      \
-                        have_sysv_atvar, &fi, set_default);                   \
+                        premodifier, have_sysv_atvar, &fi, set_default);      \
         }                                                                     \
       filenames = 0;                                                         \
       commands_idx = 0;                                                        
      \
@@ -970,13 +971,12 @@
                been expanded... we'll never get here.  */
           }
 
-        p2 = next_token (variable_buffer);
-
-        /* If the word we're looking at is EOL, see if there's _anything_
+        /* If we didn't find any colon, see if there's _anything_
            on the line.  If not, a variable expanded to nothing, so ignore
            it.  If so, we can't parse this line so punt.  */
-        if (wtype == w_eol)
+        if (colonp == 0)
           {
+            p2 = next_token (variable_buffer);
             if (*p2 != '\0')
               /* There's no need to be ivory-tower about this: check for
                  one of the most common bugs found in makefiles...  */
@@ -989,11 +989,36 @@
         /* Make the colon the end-of-string so we know where to stop
            looking for targets.  */
         *colonp = '\0';
+
+        /* Check for premodifiers in the separator.  */
+        if (colonp > variable_buffer &&
+            (colonp[-1] == '&' || colonp[-1] == '|'))
+          {
+            premodifier = colonp[-1] ;
+            /* Nobody is probably going to use this (it's easier to simply
+               put a space before the colon), but for consistency, allow the
+               modifier to be quoted.
+               First counting back and then calling find_char_unquote is
+               somewhat roundabout, but means that we don't have to
+               duplicate the backslashes-quoting-other-backslashes
+               logic here.  */
+            p2 = colonp-1 ;
+            while (p2 > variable_buffer && p2[-1] == '\\')
+              p2--;
+            p2 = find_char_unquote (p2, premodifier, 0, 0) ;
+            if (p2 != 0)
+              *p2 = '\0' ;
+            else
+              premodifier = 0 ;
+          }
+        else
+          premodifier = 0 ;
+
+        p2 = next_token (variable_buffer);
         filenames = multi_glob (parse_file_seq (&p2, '\0',
                                                 sizeof (struct nameseq),
                                                 1),
                                 sizeof (struct nameseq));
-        *p2 = ':';
 
         if (!filenames)
           {
@@ -1002,9 +1027,8 @@
             no_targets = 1;
             continue;
           }
-        /* This should never be possible; we handled it above.  */
-        assert (*p2 != '\0');
-        ++p2;
+
+        p2 = colonp + 1;
 
         /* Is this a one-colon or two-colon entry?  */
         two_colon = *p2 == ':';
@@ -1146,6 +1170,9 @@
             pattern_percent = find_percent (pattern);
             if (pattern_percent == 0)
               fatal (fstart, _("target pattern contains no `%%'"));
+            if (premodifier != '\0')
+              fatal (fstart, _("static pattern rule can't have `%c:'"),
+                     premodifier);
             free((char *)target);
           }
         else
@@ -1721,6 +1748,7 @@
    with dependencies DEPS, commands to execute described
    by COMMANDS and COMMANDS_IDX, coming from FILENAME:COMMANDS_STARTED.
    TWO_COLON is nonzero if a double colon was used.
+   PREMODIFIER is '&' or '|' if '&:' or '|:' was used.
    If not nil, PATTERN is the `%' pattern to make this
    a static pattern rule, and PATTERN_PERCENT is a pointer
    to the `%' within it.
@@ -1730,7 +1758,7 @@
 
 static void
 record_files (filenames, pattern, pattern_percent, deps, cmds_started,
-             commands, commands_idx, two_colon, have_sysv_atvar,
+             commands, commands_idx, two_colon, premodifier, have_sysv_atvar,
               flocp, set_default)
      struct nameseq *filenames;
      char *pattern, *pattern_percent;
@@ -1739,12 +1767,14 @@
      char *commands;
      unsigned int commands_idx;
      int two_colon;
+     int premodifier;
      int have_sysv_atvar;
      const struct floc *flocp;
      int set_default;
 {
   struct nameseq *nextf;
   int implicit = 0;
+  struct dep *also_make = 0;
   unsigned int max_targets = 0, target_idx = 0;
   char **targets = 0, **target_percents = 0;
   struct commands *cmds;
@@ -1787,7 +1817,7 @@
       if (implicit && implicit_percent == 0)
        fatal (flocp, _("mixed implicit and normal rules"));
 
-      if (implicit)
+      if (implicit && premodifier != '|')
        {
          if (targets == 0)
            {
@@ -1816,6 +1846,20 @@
         to go in more than one place in the data base.  */
       this = nextf != 0 ? copy_dep_chain (deps) : deps;
 
+      if (implicit)
+       {
+         /* This code is used for a |: pattern rule; rules with just
+            : or &: have already been handled.  */
+         char **onetarget = (char **) xmalloc(2 * sizeof (char *));
+         char *onepercent[2];
+         onetarget[0] = name;
+         onetarget[1] = 0;
+         onepercent[0] = implicit_percent;
+         onepercent[1] = 0;
+         create_pattern_rule(onetarget, onepercent, two_colon, this, cmds, 1);
+         continue;
+       }
+
       if (pattern != 0)
        {
          /* If this is an extended static rule:
@@ -1979,6 +2023,12 @@
              error (&f->cmds->fileinfo,
                      _("warning: ignoring old commands for target `%s'"),
                      f->name);
+             /* The new commands do not make any of the old commands'
+                auto_make's. However, if the old commands had auto_make
+                partners and one of those are remade, we'll assume that
+                the old commands will remake this file! Oh well: garbage
+                in, garbage out.  */
+             f->also_make = 0;
            }
 
          f->is_target = 1;
@@ -2074,6 +2124,18 @@
          f->cmds = cmds;
        }
 
+      /* Add to the chain of also_make files if this is an &: rule that has
+        commands. However, don't bother if there is only one target.  */
+      if (premodifier == '&' && cmds != 0 && (also_make != 0 || nextf != 0))
+       {
+         struct dep *new = (struct dep *) xmalloc (sizeof (struct dep));
+         new->ignore_mtime = 0;
+         new->name = f->name ;
+         new->file = f ;
+         new->next = also_make ;
+         also_make = new ;
+       }
+
       /* Free name if not needed further.  */
       if (f != 0 && name != f->name
          && (name < f->name || name > f->name + strlen (f->name)))
@@ -2124,13 +2186,20 @@
        }
     }
 
-  if (implicit)
+  if (implicit && target_idx != 0)
     {
       targets[target_idx] = 0;
       target_percents[target_idx] = 0;
       create_pattern_rule (targets, target_percents, two_colon, deps, cmds, 1);
       free ((char *) target_percents);
     }
+
+  if (also_make != 0)
+    {
+      struct dep *d;
+      for (d = also_make; d != 0; d = d->next)
+       d->file->also_make = also_make ;
+    }
 }
 
 /* Search STRING for an unquoted STOPCHAR or blank (if BLANK is nonzero).
@@ -2661,6 +2730,13 @@
       wtype = w_varassign;
       break;
 
+    case '&':
+    case '|':
+      if (*p != ':')
+        break ;
+      p++ ;
+      /* and fall through! */
+
     case ':':
       wtype = w_colon;
       switch (*p)
@@ -2698,7 +2774,7 @@
 
   /* This is some non-operator word.  A word consists of the longest
      string of characters that doesn't contain whitespace, one of [:=#],
-     or [?+]=, or one of the chars in the DELIM string.  */
+     or [?+]= or [&|]:, or one of the chars in the DELIM string.  */
 
   /* We start out assuming a static word; if we see a variable we'll
      adjust our assumptions then.  */
@@ -2763,6 +2839,12 @@
           if (*p == '=')
             goto done_word;
           break;
+
+        case '&':
+        case '|':
+          if (*p == ':')
+            goto done_word;
+          break ;
 
         case '\\':
           switch (*p)
diff -ur make-3.80.orig/remake.c make-3.80/remake.c
--- make-3.80.orig/remake.c     Thu Aug  8 02:11:19 2002
+++ make-3.80/remake.c  Sat Dec 21 02:06:33 2002
@@ -812,6 +812,11 @@
        So mark them as updated with the same status.  */
     for (d = file->also_make; d != 0; d = d->next)
       {
+       /* For static also_make rules, all files share the same linked
+          list, so explictly ignore the file itself.  */
+       if (d->file == file)
+         continue;
+       
        d->file->command_state = cs_finished;
        d->file->updated = 1;
        d->file->update_status = file->update_status;


-- 
Henning Makholm                     "That's okay. I'm hoping to convince the
                      millions of open-minded people like Hrunkner Unnerby."