automake-patches
[Top][All Lists]
Advanced

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

Re: [PATCH] Extend checks on remake rules.


From: Ralf Wildenhues
Subject: Re: [PATCH] Extend checks on remake rules.
Date: Sun, 5 Dec 2010 20:53:31 +0100
User-agent: Mutt/1.5.20 (2010-08-04)

* Stefano Lattarini wrote on Sun, Dec 05, 2010 at 06:06:00PM CET:
> I'll wait the customary 72 hours (until wednesday evening) before pushing.

This patch should have proper testing on several systems, and seems ok
when tested and with nitse below addressed.  If you still don't have
access, please ping the person involved again, and me so that I can test
the patch eventually.

A couple of issues in this patch I'm seeing just from glancing at it
is that it touches this sed limitation (quoting autoconf.info):

     If a sed script is specified on the command line and ends in an
     `a', `c', or `i' command, the last line of inserted text should be
     followed by a newline.  Otherwise some `sed' implementations
     (e.g., OpenBSD 3.9) do not append a newline to the inserted text.

In remake1a, your sanity check requires that 'make -n' works to really
not remake anything.  If that were the main purpose of the test, that
would be fine, but as a sanity check I'd rather not use -n here.

What is remake11.test for?

Some of the tests look like they are testing the same things really
as other new or existing tests.  Can you explain them (ideally, the
comment at the start of the test should be unique for each test so
we can infer from that that each of the tests is actually required)?
But maybe I'm just overlooking something?

I wish you'd get yourself convinced to get rid of some of the 'for
debugging' type comments, these things seem a bit obvious to need a
comment.  OTOH, I can see that some of them really help getting over
the "wait, isn't that a typo" sort of feeling.  Hmm, maybe leave them
in, skipping over them isn't that hard.

> The updated rebased patch is attached.  In it I have:
>  - removed the overly hackish and complex tests remake2{a,b,c}.test,
>    introduced by the previous version of my patch; I'll hopefully
>    be able to find a saner way to improve coverage in that area (it
>    can always be done in a follow-up patch without problems);
>  - removed a uselessly-added "make distcheck" from `remake4.test';
>  - removed all usages of in-line shell commments in make rules (since
>    it turned out they're unportable to at least Tru64/OSF 5.1 make);
>    this affected only test `remake12.test';
>  - removed usages of `##' special automake comments which weren't on
>    lines of their own (this usage is not part of the automake API)
>  - added or extended comments for trickier tests;
>  - thrown in some micellanous minor improvements and simplifications.

Good!

Thanks,
Ralf



reply via email to

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