Hi Ralf, thanks for the review.
On Wednesday 08 September 2010, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Tue, Sep 07, 2010 at 12:19:02PM CEST:
> > What about the attached patch (for maint)?
>
> OK, thanks! I have a question below, though.
>
> > As an aside, I'd like to vouch again for my patch "Refactoring
> > maintainer checks (use perl, add whitelisting)", which would
> > provide an easy way to white-list spurious failures.
>
> I've got that on my radar for a bigger chunk of review time.
>
> > Subject: [PATCH] Make some `confh*.test' tests more "semantic"
> > (plus tweakings).
> >
> > * tests/confh.test: Run "autoconf", "configure" and "make check",
> > instead of munging/grepping the generated `Makefile.in'.
> > * tests/confh4.test: Relax the grepping of Makefile.in w.r.t.
> > white spaces. Do not create usless dummy source file `foo.c'
> > and useless dummy header file `acconfig.h'.
> > (configure.in): Remove superfluous call to `AC_OUTPUT'.
> > * tests/confh6.test: Add trailing `:' command.
> > * tests/confh7.test: In comments, add reference to ...
> > * tests/confh8.test: ... this new test, "semantic" sister
> > of `confh7.test'.
> > * tests/Makefile.am (TESTS): Updated.
> > Prompted by a report from Ralf Wildenhues.
> >
> > --- a/tests/confh.test
> > +++ b/tests/confh.test
> >
> > +cat > Makefile.am << 'END'
> > +.PHONY: test1 test2
> > +test1:
> > + @echo DIST_COMMON = $(DIST_COMMON) ## for debugging
> > + echo ' ' $(DIST_COMMON) ' ' | grep '[ /]acconfig\.h '
>
> Just curious, but why not
> echo ' $(DIST_COMMON) ' | ...
>
> and likewise in other tests? I can see that you'd like to
> surround, for Solaris make, a macro expansion by white space so
> that VPATH rewriting is done consistently also for the first and
> last word in the variable, but shell quoting shouldn't influence
> the result; if it does, then that would probably be a bug
> somewhere.
To be honest, I wasn't thinking about VPATH rewriting here; my idiom
was to avoid the test to fail spuriously in case $(DIST_COMMONS)
contains tabs instead of spaces. For semantic tests, a definition
like:
DIST_COMMON = a b c
and one like:
DIST_COMMON = a \
b \
\
c
should be equivalent.
> If you want to squash in this change,
Well, I don't, for the reasons stated above ...
> consider it preapproved.
... so I'll wait to hear back from you before applying the patch.
Regards,
Stefano
