Re: [PATCH] {master} Improve and extend tests on `:=' variable assignments.

[Ralf Wildenhues]

Hello Stefano,

* Stefano Lattarini wrote on Mon, Nov 29, 2010 at 01:55:27PM CET:
> On Monday 29 November 2010, Ralf Wildenhues wrote:
> > * Stefano Lattarini wrote on Thu, Nov 25, 2010 at 02:37:28PM CET:
> > > The attached patch is based off of maint, and intended for master.
> > > OK to apply?
> > 
> > With nits addressed.

> > > --- a/tests/colneq2.test
> > > +++ b/tests/colneq2.test
> > > @@ -20,14 +20,23 @@
> > >  
> > >  set -e
> > >  
> > > +cat >> configure.in << 'END'
> > > +AC_OUTPUT
> > > +END
> > > +
> > >  cat > Makefile.am << 'END'
> > >  t = a b c
> > > -EXTRA_DIST = $(t:=.test)
> > 
> > Please leave the EXTRA_DIST line in.  It is something different if
> > automake skips a normal variable containing this, and a variable that is
> > special to automake.
> > 
> In truth, I was planning to add a new test about substitutions and
> indirections in definition of "special" automake variables, as TESTS,
> bin_PROGRAMS and the like; that's why I felt justified in removing the
> line above.  But I ended up not completing and submitting that patch
> because the patch queue was (and is) already too long.

Let's recap what happened: you sent a patch that takes away a (minor)
bit of test coverage; I approve the patch but ask you to keep that
coverage in, now you update the patch with an unrelated new change whose
applicability depends on completely different factors (namely deciding
whether some behavior is desirable or not), plus also a related new test
coverage patch.

This procedure is not helpful.  Instead of turning N pending patches
into N+2 pending patches, why not turn it into N-1 pending patches.
And THEN, send a new patch doing whatever else you wanted to do.  Make
that independent.  I approved the original submission with nits
addressed, so why not just apply it with those nits addressed, then we
can at least forget about that part of the story and concentrate on the
rest only?  And keep making progress, instead of wasting both patch
production time on your side and review time on my side?

> Now I have
> completed at least the testcase for EXTRA_DIST (see attachement); so,
> OK to keep this change if that testcase is added in a prior commit?

No.  Sorry.

> > Uh, oh.  Thin ice.  This is OK, but we gotta remember that it won't
> > work reliably if some of these variables are actually automake-set
> > before they are overridden.  automake generally orders all of its
> > variable settings before all of the user ones (so the user ones are
> > preferred).  When I override, e.g., libdir here, however, it doesn't
> > get reordered to the user part.  I wonder whether that is a bug in
> > automake.
> >
> I've added a new (xfailing) testcase to expose this bug/limitation.
> OK to apply it?

No.  The above paragraph was meant as an incentive to think, dig, and
find out whether this is a bug or not, and if it is, whether it is
possible in general to fix it without harming other legitimate use cases
or not.  The test case does not address that question.  Not everything
can be answered with a test case, and no, I don't want test cases that
basically ask me whether the test case is right or wrong.

Thanks,
Ralf