[PATCH] Implement "grouped targets" in ordinary rules.

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

[PATCH] Implement "grouped targets" in ordinary rules.

Kaz Kylheku (gmake)
This patch (against 4.2.1) adds the + (plus) syntax:

   + tgt1 tgt2 ... tgtn : pre1 pre2 ...
         recipe

When the + is present, then the targets are understood to be built
together by one invocation of the recipe: each one lists the others in
its "also_make" list, similarly to what multiple-target pattern rules
already do.

* doc/make.texi: Section on Multiple Targets updated.

* read.c (enum make_word_type): New enum member, w_plus.
(record_files): New argument, are_also_makes flag which indicates
that the targets are all linked together as also-makes.
If this is true, then we build an also_make list which contains
each target represented as a dep. When all targets are registered,
we then make a second pass, installing a copy of this also_make
list into each target, possibly catenating it with an existing
also_make list.
(record_waiting_files): Pass new argument to record_files.
(eval): Check for the + token and set a new local variable
also_make_targets if that is so. This is then passed to
record_files by the record_waiting_files macro.
(get_next_mword): If a + is followed by whitespace, or end of string,
rathe than =, then instead of falling through, recognize it as the new
w_plus token.
---
  doc/make.texi | 41 ++++++++++++++++++++++++++++-----
  read.c        | 64 +++++++++++++++++++++++++++++++++++++++++++++++----
  2 files changed, 95 insertions(+), 10 deletions(-)

diff --git a/doc/make.texi b/doc/make.texi
index 01bcec7..033c72d 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -3012,13 +3012,27 @@ both pieces to the suffix list.  In practice,
suffixes normally begin with
  @cindex targets, multiple
  @cindex rule, with multiple targets

-A rule with multiple targets is equivalent to writing many rules, each
with
-one target, and all identical aside from that.  The same recipe applies
to
-all the targets, but its effect may vary because you can substitute the
-actual target name into the recipe using @samp{$@@}.  The rule
contributes
-the same prerequisites to all the targets also.
+When a rule has multiple targets, the semantics depends on what kind
+of rule it is, and whether the "grouped targets" feature is invoked.

-This is useful in two cases.
+There are two possible semantics. A rule with multiple targets may be
+equivalent to writing many rules, each with one target, and all
identical aside
+from that.  Under this semantics, the same recipe applies to all the
targets,
+but its effect may vary because you can substitute the actual target
name into
+the recipe using @samp{$@@}.  The rule contributes the same
prerequisites to
+all the targets also. This is known as "replicated recipe semantics".
+The other semantics is "grouped target" semantics. Under this
semantics,
+the files are all understood to be updated or created together by a
single
+invocation of the recipe.
+
+Pattern rules implicitly follow "grouped target" semantics.
+
+Ordinary rules use "replicated recipe" semantics by default. If
+the symbol @samp{+} is placed before the first target, separated
+from it by whitespace, then the rule follows "grouped target"
+semantics.
+
+"Replicated recipe semantics" is useful in two cases.

  @itemize @bullet
  @item
@@ -3064,6 +3078,21 @@ types of output, one if given @samp{-big} and one
if given
  for an explanation of the @code{subst} function.
  @end itemize

+"Grouped target" semantics is useful when a recipe builds multiple
+files, which are further involved in dependencies. For instance,
+it can express a rule rule for building a parser using Yacc, whose
+recipe generates @samp{y.tab.c} and @samp{y.tab.h} at the same time.
+Both are made to be dependent on @samp{parser.y}. If neither one
+exists, or if both are out of date with respect to @samp{parser.y},
+the recipe is invoked only once. Furthermore, if for some reason
+just one of these two files is missing or out of date, GNU Make
+knows that running the recipe also updates the other.
+
+Note that under "grouped target" semantics, the @samp{$@@} variable
+still expands to a single target: it refers to that target which
+triggered the execution of the recipe. The recipe cannot rely on
+@samp{$@@} being a stable reference to a particular one of the targets.
+
  Suppose you would like to vary the prerequisites according to the
  target, much as the variable @samp{$@@} allows you to vary the recipe.
  You cannot do this with multiple targets in an ordinary rule, but you
diff --git a/read.c b/read.c
index b870aa8..983bdc5 100644
--- a/read.c
+++ b/read.c
@@ -71,7 +71,7 @@ struct vmodifiers
  enum make_word_type
    {
       w_bogus, w_eol, w_static, w_variable, w_colon, w_dcolon,
w_semicolon,
-     w_varassign
+     w_varassign, w_plus
    };


@@ -141,7 +141,8 @@ static void do_undefine (char *name, enum
variable_origin origin,
  static struct variable *do_define (char *name, enum variable_origin
origin,
                                     struct ebuffer *ebuf);
  static int conditional_line (char *line, int len, const floc *flocp);
-static void record_files (struct nameseq *filenames, const char
*pattern,
+static void record_files (struct nameseq *filenames, int
are_also_makes,
+                          const char *pattern,
                            const char *pattern_percent, char *depstr,
                            unsigned int cmds_started, char *commands,
                            unsigned int commands_idx, int two_colon,
@@ -576,6 +577,7 @@ eval (struct ebuffer *ebuf, int set_default)
    unsigned int cmds_started, tgts_started;
    int ignoring = 0, in_ignored_define = 0;
    int no_targets = 0;           /* Set when reading a rule without
targets.  */
+  int also_make_targets = 0;    /* Set when reading grouped targets. */
    struct nameseq *filenames = 0;
    char *depstr = 0;
    long nlines = 0;
@@ -593,7 +595,8 @@ eval (struct ebuffer *ebuf, int set_default)
          {                                                              
       \
            fi.lineno = tgts_started;                                    
       \
            fi.offset = 0;                                                
       \
-          record_files (filenames, pattern, pattern_percent, depstr,    
       \
+          record_files (filenames, also_make_targets, pattern,          
       \
+                        pattern_percent, depstr,                        
       \
                          cmds_started, commands, commands_idx,
two_colon,      \
                          prefix, &fi);                                  
       \
            filenames = 0;                                                
       \
@@ -601,6 +604,7 @@ eval (struct ebuffer *ebuf, int set_default)
        commands_idx = 0;                                                
       \
        no_targets = 0;                                                  
       \
        pattern = 0;                                                      
       \
+      also_make_targets = 0;                                            
       \
      } while (0)

    pattern_percent = 0;
@@ -1027,6 +1031,20 @@ eval (struct ebuffer *ebuf, int set_default)
             beginning, expanding as we go, and looking for "interesting"
             chars.  The first word is always expandable.  */
          wtype = get_next_mword (line, NULL, &lb_next, &wlen);
+
+        /* If the line starts with + followed by whitespace, then
+           it specifies that the targets are understood to be
updated/created
+           together by a single invocation of the recipe. In this case,
+           we record a single entry for the first target which follows
the +,
+           and install the remaining targets as the also_make list of
deps
+           for that file, rather than iterating over the targets and
replicating
+           the rule for each one. */
+        if (wtype == w_plus) {
+          also_make_targets = 1;
+          lb_next += wlen;
+          wtype = get_next_mword (lb_next, NULL, &lb_next, &wlen);
+        }
+
          switch (wtype)
            {
            case w_eol:
@@ -1931,7 +1949,8 @@ record_target_var (struct nameseq *filenames, char
*defn,
     that are not incorporated into other data structures.  */

  static void
-record_files (struct nameseq *filenames, const char *pattern,
+record_files (struct nameseq *filenames, int are_also_makes,
+              const char *pattern,
                const char *pattern_percent, char *depstr,
                unsigned int cmds_started, char *commands,
                unsigned int commands_idx, int two_colon,
@@ -1939,6 +1958,7 @@ record_files (struct nameseq *filenames, const
char *pattern,
  {
    struct commands *cmds;
    struct dep *deps;
+  struct dep *also_make = 0;
    const char *implicit_percent;
    const char *name;

@@ -2158,6 +2178,15 @@ record_files (struct nameseq *filenames, const
char *pattern,
            f->cmds = cmds;
          }

+      if (are_also_makes)
+        {
+          struct dep *also = alloc_dep();
+          also->name = f->name;
+          also->file = f;
+          also->next = also_make;
+          also_make = also;
+        }
+
        f->is_target = 1;

        /* If this is a static pattern rule, set the stem to the part of
its
@@ -2222,6 +2251,27 @@ record_files (struct nameseq *filenames, const
char *pattern,
          O (error, flocp,
             _("*** mixed implicit and normal rules: deprecated
syntax"));
      }
+
+  /* If the files are also-makes, then populate a copy of the also-make
list
+     into each one. */
+
+  if (are_also_makes && also_make)
+    {
+      struct dep *i;
+
+      for (i = also_make; i != 0; i = i->next)
+        {
+          struct file *f = i->file;
+          struct dep *also_copy = copy_dep_chain(also_make);
+          struct dep *j;
+
+          for (j = also_copy; j->next != 0; j = j->next)
+            ;
+
+          j->next = f->also_make;
+          f->also_make = also_copy;
+        }
+    }
  }
 
  /* Search STRING for an unquoted STOPCHAR or blank (if BLANK is
nonzero).
@@ -2676,6 +2726,12 @@ get_next_mword (char *buffer, char *delim, char
**startp, unsigned int *length)
        break;

      case '+':
+      if (isspace(*p) || *p == 0)
+        {
+          wtype = w_plus;
+          break;
+        }
+      /* fallthrough */
      case '?':
      case '!':
        if (*p == '=')




_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement "grouped targets" in ordinary rules.

Paul Smith-20
On Tue, 2019-03-12 at 20:50 -0700, Kaz Kylheku (gmake) wrote:
> This patch (against 4.2.1) adds the + (plus) syntax:
>
>    + tgt1 tgt2 ... tgtn : pre1 pre2 ...
>          recipe

Hi Kaz; thanks very much for your interest in GNU make!  This is a
useful feature that many people have wanted.

You may find more info about this on the Savannah bug tracker:

https://savannah.gnu.org/bugs/index.php?8297

In the past when we've discussed this, we were considering using the
special symbol "&:" (one token, no spaces allowed) replacing the ":" to
denote a "grouped" rule like this.  I think that implementation would
be simpler to parse than the one you suggest.  There are a number of
places where adding a new special token to the parser can cause
backward-compatibility issues; for example if someone was using a
simple "+" as a prerequisite (which is perfectly legal).  So it would
be:

  tgt1 tgt2 ... tgtn &: pre1 pre2 ...

Other versions of make allow both grouped and non-grouped targets in
the same syntax by introducing a grouping syntax, like:

  [ tgt1 tg2 ] tgt3 [tgt4 tgt5] : ...

However I'm not sure that level of complexity is needed or desirable.
One could still do that with the "whole rule" implementation, at the
cost of repeating the recipe for each separate rule.

Second, I'm not sure that the solution you have is complete because the
additional targets are never registered as files with make.  That means
that only the first target is a known file, etc.  E.g., there's no
reason that this should not work:

  baz : bar

  foo bar &:

If someone runs "make baz" here I would expect make to run the single
recipe to generate both "foo" and "bar", but I'm not sure it will
behave properly in your patch because "bar" is not registered as a file
(as far as I understood what it does).

This relates to my last concern so far: a feature this significant
definitely needs to have tests created for it, both positive and
negative, to show how it works and that it works properly in many
different situations.  There is a test suite available in the GNU make
distribution which is fairly straightforward to use (at least for
simple tests).  Please let me know if you have questions.

And finally, please remember that any feature representing this much
work needs to have copyright assignment papers completed to assign
changes to the FSF.  I see you have assignments for GNU libc; let me
know if you need me to send you details about this.

Thanks again for your interest in making GNU make better!


_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement "grouped targets" in ordinary rules.

Kaz Kylheku (gmake)
On 2019-03-13 05:28, Paul Smith wrote:

> On Tue, 2019-03-12 at 20:50 -0700, Kaz Kylheku (gmake) wrote:
>> This patch (against 4.2.1) adds the + (plus) syntax:
>>
>>    + tgt1 tgt2 ... tgtn : pre1 pre2 ...
>>          recipe
>
> Hi Kaz; thanks very much for your interest in GNU make!  This is a
> useful feature that many people have wanted.
>
> You may find more info about this on the Savannah bug tracker:
>
> https://savannah.gnu.org/bugs/index.php?8297
>
> In the past when we've discussed this, we were considering using the
> special symbol "&:" (one token, no spaces allowed) replacing the ":" to
> denote a "grouped" rule like this.  I think that implementation would
> be simpler to parse than the one you suggest.

Hi Paul.

I will re-work it to use &: it's a lot better!

We don't have to care about this case, do we?

    a b c &: : prereq   # :& is an odd target name, eek!

I take it I should post to the Savannah item above?

I have a Savannah account due to having a few projects there.
I don't have permission to upload patches to the GNU Make project,
just to open tickets and comment.

> Other versions of make allow both grouped and non-grouped targets in
> the same syntax by introducing a grouping syntax, like:
>
>   [ tgt1 tg2 ] tgt3 [tgt4 tgt5] : ...

This is very similar to an original design that I proposed in a
discussion
in comp.unix.programmer. The proposal was to parenthesize all of the
targets like this:  ( tgt1 ... tgtn ) : prereqs ...

Even just this is quite a bit more work in the parsing.

If multiple target groups are allowed, also more complexity in the
file registration logic.

> However I'm not sure that level of complexity is needed or desirable.

Indeed, would be the use case? When would there be a generic rule
which updates differently-sized subsets of target pairs? How would the
recipe know what it's supposed to build?

We can imagine a case where targets are organized into tuples of
equal arity:

   [ tgt1.1 tgt1.2 ] [ tgt2.1 tgt2.2 ] : ...

This case calls for a pattern rule. Because surely, the elements of
these tuples must follow a naming pattern in which a common stem
is identifiable. If they don't follow such a naming pattern, how will
the recipe know what to build? There would have to be a new automatic
variable which gives the recipe the entire tuple of targets.
The $@ variable only indicates the one target which triggered the rule,
which could be any element of the tuple. Pattern rules address that
with the stem variable $*; there just has to be commonality in the
non-stem parts of the target names that the rule knows about.

> Second, I'm not sure that the solution you have is complete because the
> additional targets are never registered as files with make.

Funny you should say that because I had a bug like this in a
"first cut" at this, so I know exactly what you're talking about!

The algorithm for registering the targets is not altered (including
double colon handling). Each of the targets is registered in the
same way as before; the first target isn't special in any way.

If the flag indicates that they are to be linked as "also makes",
then, as they are being registered, a parallel list of "dep" objects
is built up, one for each registered file

Then, in a second pass, the files are re-visited (by traversing
this dep list, whose entries have a "struct file *" pointer to
each corresponding file). For each file, a copy of that whole
dep list is installed as its also_make list, resulting in a
"full mesh".

(P.S. there is a memory leak because I don't just use the original
dep list for the last file; a duplicate is made for that one too,
and the original is not freed. I already fixed this.)

I didn't want to start writing a bunch of tests before getting comments
about this feature: is there interest in it at all, how is the syntax,
and so on.

> And finally, please remember that any feature representing this much
> work needs to have copyright assignment papers completed to assign
> changes to the FSF.

Is the threshold that low?

This is just a few lines of code in some existing functions.

I'm smiling contently at the low amount of effort and LOC expenditure.

Basically, the actual algorithm for updating the files together
is already there, thanks to pattern rules. So unless I'm missing
something,
this is just a matter of connecting a few dots with some chains of
dep objects.  Essentially, it's just exposing existing semantics by
allowing it to be configured with some new syntax.

Cheers ...


_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement "grouped targets" in ordinary rules.

Paul Smith-20
On Wed, 2019-03-13 at 12:50 -0700, Kaz Kylheku (gmake) wrote:
> I take it I should post to the Savannah item above?
>
> I have a Savannah account due to having a few projects there.
> I don't have permission to upload patches to the GNU Make project,
> just to open tickets and comment.

I disabled the patch section because it seemed more trouble than it was
worth to keep it separate: patches can be attached to bugs which is how
I'd prefer it.

You can continue to use mailing lists if you want, it doesn't matter to
me (in fact mailing lists are often better for initial proposals to get
comments) although [hidden email] is better for discussing patches
and new features, than [hidden email].

Regarding the amount of change, it doesn't just cover the code change
but all the rest of the changes including docs and tests... here's the
doc covering this:

https://www.gnu.org/prep/maintain/maintain.html#Legally-Significant

>     a b c &: : prereq   # :& is an odd target name, eek!

This is already invalid syntax so you don't have to worry about that.
There's no way to create a target containing a colon in the name except
by hiding it inside a variable:

  FOO = foo:bar

  $(FOO) : baz

Cheers!


_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement "grouped targets" in ordinary rules.

Kaz Kylheku (gmake)
In reply to this post by Kaz Kylheku (gmake)
On 2019-03-13 12:50, Kaz Kylheku (gmake) wrote:
> Funny you should say that because I had a bug like this in a
> "first cut" at this, so I know exactly what you're talking about!

Aha; I found obsolete text in one of my comments which likely
inspired this suspicion:

   /* If the line starts with + followed by whitespace, then
      it specifies that the targets are understood to be updated/created
      together by a single invocation of the recipe. In this case,
      we record a single entry for the first target which follows the +,
      and install the remaining targets as the also_make list of deps
      for that file, rather than iterating over the targets and
replicating
      the rule for each one. */

All of that is incorrect.

_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement "grouped targets" in ordinary rules.

Kaz Kylheku (gmake)
In reply to this post by Paul Smith-20
On 2019-03-13 05:28, Paul Smith wrote:
> And finally, please remember that any feature representing this much
> work needs to have copyright assignment papers completed to assign
> changes to the FSF.  I see you have assignments for GNU libc; let me
> know if you need me to send you details about this.

Hi Paul,

Having previously made the changes against 4.2.1, I have rebased the
patch
against the current git and uploaded it to the ticket.

Please review; if this is good let me know about the assignment papers.

I had to reintroduce into find_char_unquote the ability to scan for more
than one character.

Observation: the get_next_mword function's "delim" parameter is
passed as NULL by all callers; it should be removed.

Cheers ...

_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement "grouped targets" in ordinary rules.

Paul Smith-20
On Thu, 2019-03-14 at 20:34 -0700, Kaz Kylheku (gmake) wrote:
> Having previously made the changes against 4.2.1, I have rebased the
> patch against the current git and uploaded it to the ticket.

I'll take a look this weekend.

> Please review; if this is good let me know about the assignment
> papers.

I'll email you off-list.

Thanks!


_______________________________________________
Help-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-make