Re: Proposed / contributed enhancement to GNU m4.

[Eric Blake]

According to David Warme on 2/2/2010 8:30 AM:
> Please forward necessary "copyright details off-list" to me.

Done.

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

[snip nice example]
Yes, this is a very nice thing to add - a mode in which m4 can output
makefile dependencies that track which files were found.  And it would be
a great addition to m4 1.6.  But I have some suggestions:

> 
> The "normal" invocation would be
> 
>      m4 -Ih -I../include A B >output
>
> The "makedep" invocation would be as follows:
>
>      m4 -Ih -I../include --makedep=xyzzy A B >output.dep

We already have an open TODO item to allow -o/--output as an option to
specify the name of m4's output file (not implemented in 1.4.x because of
a deprecation period where -o meant --error-output; whether we use -o
right away is debatable, but we can implement --output without problem).
Furthermore, it is always nicer to generate makefile dependencies as a
side-effect of normal processing, and not as a separate compile (among
other things, it eliminates races if files change in between the two
separate runs).  Therefore, I would suggest a command line more like:

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

where m4 creates 'output' containing the normal expansion, and creates
'output.dep' containing the Makefile snippets that list the dependencies
of 'output:'.  In other words, --output should specify the name of the
target, and --makedep should specify the name of the Makefile snippet; and
use of --makedep without --output would fail.

> 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?!?  :^)

Hmmm.  The automake list has more experience about what line length
limitations are required for various make implementations.  But IF we wrap
at all, I'd prefer that either: the wrap column is configurable via yet
another command-line option (although defaulting to 78 is okay in that
case), or that we wrap after EVERY dependency rather than trying to make
maximal line length.  It is NOT appropriate to hard-code wrapping without
providing some tuning.  But I'm questioning whether we SHOULD wrap; it
seems like wrapping long lines to guarantee they fit Makefile constraints
is something better left to post-processing tools, like fold (the Unix
mantra, after all, is to write applications that do one thing and do it well).

>> 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"!

But the point is that patches SHOULD be simple text; inline review is
invaluable.  That's another nice aspect of git, in how it can format
emails containing patches that use the correct MIME types for inline patch
review.

Overall, thanks again for picking up this work.

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

Eric Blake             address@hidden

signature.asc
Description: OpenPGP digital signature