Re: Proposed / contributed enhancement to GNU m4.

[Eric Blake]

According to David Warme on 2/1/2010 5:35 PM:
> I would like to contribute a much-needed enhancement to GNU m4.
> 
> It adds the following command line switch:
> 
>     --makedep=TARGET

Sounds interesting.  But before I look at it, which git branch did you
base the work on?  I'd prefer that it be branch-1.6 or master (and not
branch-1.4), since a new feature fits better on a feature release than on
a stable maintenance series.  Also, this is a non-trivial addition; you
would need to assign copyright to FSF before this can be incorporated.
Are you still interested?  If so, I can send you copyright details off-list.

> This switch causes GNU m4 to discard all normally generated
> macro-expansion output.  Instead, it generates a Makefile dependency
> rule on standard output.  This dependency rule has a target (i.e.,
> left-hand-side of the rule) as specified by the string TARGET.

Can you show a sample usage sequence, of an input m4 file (along with its
use of [s]include), as well as the resulting makefile snippet that this
would generate?  It's easier to review a patch when we also have a sample
of what the patch is trying to accomplish.

> I have attached a patch file that contains my proposed implementation of
> this new feature.

A plain-text patch is easier to review than a compressed patch, if the
resulting diff is < 100k.  And output from diffstat helps provide a
summary of what the reviewer can expect:

$ diffstat m4-makedep.patch
 ChangeLog      |    5 +++++
 doc/m4.texinfo |    9 +++++++++
 src/builtin.c  |    1 +
 src/m4.c       |   25 +++++++++++++++++++++++++
 src/m4.h       |    3 +++
 src/output.c   |   33 +++++++++++++++++++++++++++++----
 src/path.c     |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 125 insertions(+), 4 deletions(-)

Hmm - only 9 lines in the documentation?  It seems like this feature may
warrant more than that; perhaps even an entire node.  Also, it would need
a NEWS entry.  And depending on which branch you base it on, this comment
in master:TODO needs tweaking:

  + Make m4 show include dependencies like gcc so Makefile targets are
    updated when their (included) input files are updated (Erick B).

(I'm not sure who that Erick B refers to; the comment was added in Nov
2000 prior to my first use of m4).

And that's as far as I've gotten in my review so far.

-- 
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden

signature.asc
Description: OpenPGP digital signature