Re: Proposed / contributed enhancement to GNU m4.

[David Warme]

> this is a non-trivial addition; you
> would need to assign copyright to FSF before this can be incorporated.
> Are you still interested?

Both I personally, the company that employs me, and the two corporations
that I "own" (i.e., hold large stakes in) are prepared to execute any
and all necessary legal disclaimers required to contribute this code.
Please forward necessary "copyright details off-list" to me.

> 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.

Consider the following input files (the files contain the text inside the
boxes -- the box itself does not appear in the files):

File ./A:      +--------------------------------------+
               |include(A1)dnl                        |
               |sinclude(A2)dnl                       |
               |sinclude(A3)dnl                       |
               |Last line of A                        |
               +--------------------------------------+

File ../include/A1:
               +--------------------------------------+
               |sinclude(A1a)dnl                      |
               |sinclude(A1b)dnl                      |
               |Last line of A1                       |
               +--------------------------------------+

File ../include/A1a:
               +--------------------------------------+
               |This is ../include/A1a                |
               +--------------------------------------+

File A1b does not exist.

File ./A2:     +--------------------------------------+
               |This is ./A2                          |
               +--------------------------------------+

File A3 does not exist.

File ./h/B:    +--------------------------------------+
               |This is is file h/B                   |
               +--------------------------------------+

The "normal" invocation would be

     m4 -Ih -I../include A B >output

and would be expected to produce the following output file:

File ./output: +--------------------------------------+
               |This is ../include/A1a                |
               |Last line of A1                       |
               |This is ./A2                          |
               |Last line of A                        |
               |This is file h/B                      |
               +--------------------------------------+

The "makedep" invocation would be as follows:

     m4 -Ih -I../include --makedep=xyzzy A B >output.dep

and the resulting output.dep file would contain:

      +---------------------------------------+
      |xyzzy: A ../include/A1 ..include/A1a \ |
      |  ./A2 ./h/B                           |
      +---------------------------------------+

This output has been artificially "narrowed" to show that the tool
correctly "wraps" long dependency rules across several lines using
the "backslash/newline" syntax of make.  My submitted patch actually
wraps lines at 78 columns (but allows arbitrarily long pathnames to
stand on lines by themselves).  (Who says card readers have long been
extinct?!?  :^)

Note that because m4 does not name its output file, the use of
the --makedep=TARGET option lets the caller define for him/herself
what the target of the resulting dependency rule should be.  Without
the TARGET arg, m4 would have to put in some sort of placeholder that
the user must then tweak using sed or awk.

Note also that "sinclude" files that do not exist do not become
dependencies.  Neither do occurrences of "-" (stdin) on the command
line.

> 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.

This is certainly doable.  I developed the patch on the latest
"public" (i.e., released) code base.  Thus far, I have never used
git, and never accessed any of the development m4 repositories.
We might task one of our more junior (i.e., cheaper) developers
to actually port this patch to the version of your choice.  Any
such employees we might use will also sign any necessary legal
disclaimers.

> A plain-text patch is easier to review than a compressed patch, if the
> resulting diff is < 100k.

I compressed the patch file not to make it smaller, but to make
it be binary.  I have had too many E-mail systems corrupt things
that were "simple text"!

> And output from diffstat helps provide a summary of what the
> reviewer can expect:
> ...

This patch is very straightforward.  The biggest challenge was
to find a way to discard the normal macro-processing output.
The easiest solution turned out to be making "diversion 0" be
an m4_tmpfile() instead of stdout.  This seemed to bash the
least amount of code.

> Hmm - only 9 lines in the documentation?  It seems like this feature may
> warrant more than that; perhaps even an entire node.

Please read the 9 lines of documentation, and then examine the patch.
It does what it says.

I would agree that sprouting a node might be worthwhile if you
were going to provide a more step-by-step tutorial on how to
incorporate such features into particular Makefiles.  This seems
overkill to me.  Better to refer them to the documentation for
GNU make.  I think most users will:

a. See the option in the manual/info/command-usage,
b. Try it out (maybe with and without --makedep),
c. See what comes out of m4,
d. Then go, "gee, I know what to do with this!"

> Also, it would need a NEWS entry.

Oops -- sorry, I overlooked that one.

> 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).

Having discussed this feature with you previously (probably more than
one or two years ago), you had said it was a long-standing item on the
TODO list.  So I actually went looking for this one.  To my surprise,
it was not mentioned anywhere in the latest released version of TODO.
I would have removed the item had it been there.

David Warme                 address@hidden