m4-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Proposed / contributed enhancement to GNU m4.


From: David Warme
Subject: Re: Proposed / contributed enhancement to GNU m4.
Date: Wed, 03 Feb 2010 17:59:45 -0500
User-agent: Thunderbird 2.0.0.18 (X11/20081105)


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

I disagree.

Make checks dependencies first -- in order to decide whether the
"compile" step is necessary or not.  Inclusion of dependency files
forces make to build/update them before including them.  Creation of
dependencies versus actually compiling/updating the target of the
dependencies are two logically separate steps.

I find that there are actually WORSE problems that result from
Makefile commands that simultaneously update two (or more) make
targets.  Everything from:
(a) commands that get executed multiple times instead of once; to
(b) running "make" multiple times always causing something to get
re-made -- i.e., make can never get everything up-to-date because
it cannot update A without also altering the date on B.  That newer
B is going to cause other things to get rebuilt next time you run
make.  Trust me -- dependencies should be generated separately from
the commands that update their targets.

The race conditions you mention come from poor Makefile design,
buggy "parallel" makes, or from multiple users running make in the
same subdir at the same time.  These are beyond the scope and
concern of m4.

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

Unnecessarily complex.  Do not couple features that do not need to be
coupled.

1. As stated above, making dependencies and "normal" operation should be
   two distinct invocations of m4.

2. In --makedep mode, therefore, --output should be specifying the name
   of the *dependency file* to generate, not the normal output file.
   In that case, a target still needs to be specified somehow.

   Note that when --output and --makedep are integrated, then the
   documentation will have to specify that in --makedep mode, the
   dependency is written to stdout unless an --output option specifies
   some other place to write it.

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

The m4 --makedep feature should wrap long lines -- if for no other
reason than that "gcc -M" also wraps its lines in this same manner.
I would bet that this wrapping is done more to provide readable, yet
compact dependencies than to cater to line-length limitations of
any particular "make" implementation.

Regarding the use of fold or other tools to perform the wrapping
instead of doing it inside of m4, I would argue back the other
way -- if the user doesn't like the wrapped lines, just post-
process the output with sed, awk, or whatever.  Same thing regarding
the particular column things get wrapped at.  Does gcc provide an
option to specify the column it wraps its dependencies at?  No it
does not.  This is unnecessary complexity.

Summary:

- gcc compiles or does -M, but NOT both at the same time.
- gcc always wraps its dependency lines at a hard-coded column.
- The formatting/wrapping of the dependencies generated by this
  patch are modeled after those generated by gcc.

If it is good enough for gcc, it should be good enough for m4!

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

You misunderstood what I was saying.  I have been bitten uncountably
many times by E-mail systems (on either the sending end, receiving
end, or by Message-Transfer-Agents in the middle) that decided for
some odd reason to alter, reformat, or otherwise bash SIMPLE TEXT.
(Everything from upper-casing everything, wrapping lines it thought
were too long, or just throwing away the "overly long" portion of
lines!)  Ever try to review C code after it went through a "paragraph
fill" transform (like emacs M-q fill-paragraph command)?  I have not
encountered an E-mail system in a very long time that commits such
harm to attachments that are obviously binary blobs of data.  I
have found that this is the safest way to make sure something
arrives unscathed at the other end of an E-mail.

As I delve further into this process, perhaps I will become more
proficient with the current level of integration between git,
E-mail, and inline patch review.  Graybeards CAN learn new tricks!

> Overall, thanks again for picking up this work.

You are welcome!

David



David M. Warme, Ph.D.
Principal Computer Scientist
Group W, Inc.
8315 Lee Highway, Suite 303
Fairfax, VA  22031

Voice:          1-703-752-5825
E-mail:         address@hidden




reply via email to

[Prev in Thread] Current Thread [Next in Thread]