Re: [PATCH] New tests on obsoleted usages of automake/autoconf macros.

2010-06-18 Thread Stefano Lattarini
At Thursday 17 June 2010, Ralf Wildenhues wrote:
 Hi Stefano,
 
 * Stefano Lattarini wrote on Sat, Jun 12, 2010 at 11:31:57PM CEST:
  New tests on obsoleted usages of automake/autoconf macros (as
  AC_INIT, AM_INIT_AUTOMAKE and AC_OUTPUT).
 
 I have some detail questions about these tests, below.
 Thanks for working on them!
 
 Cheers,
 Ralf
 
  --- /dev/null
  +++ b/tests/backcompat2.test
 
  @@ -0,0 +1,69 @@
  +# A trick to make the test run muuuch faster, by avoiding repeated
  +# runs of aclocal (one order of magnitude improvement in speed!).
  +echo 'AC_INIT(x,0) AM_INIT_AUTOMAKE' configure.in
  +$ACLOCAL
 
 Hmm, does that mean this test completely relies on caching?
It *should* mean that the test relies on each autoconf call requiring an
aclocal.m4 that defines just AM_INIT_AUTOMAKE and the macros it requires,
and nothing more.  And this aclocal.m4 is created by the first $ACLOCAL
run.

Just to be sure, I amended the script to remove the autom4te.* stuff after
the call to $ACLOCAL (and only then).

 You probably need a $sleep or rm -rf autom4te.cache before
 each of the following edits to configure.in, to avoid autotools
 operating on old cached transformations of the file.
But then we would need to do that in each test where we modify and reuse
'configure.in' (see e.g. acloca10.test, pr401.test, nodef.test, etc..).  Right?

 Am I overlooking something here?
I'd like to ask you the same question :-)
Can you explain in few words how autotools caching works exactly?
I'm starting to feel rather confused...

  +
  +cat  Makefile.am 'END'
  +got: Makefile
  +   @{ \
 
 Please put ':;' before '{', see `info Autoconf Limitations of
 Builtins' on {...}.
OK.

  + echo 'PACKAGE = $(PACKAGE)'; \
  + echo 'VERSION = $(VERSION)'; \
  + echo 'PACKAGE_NAME = $(PACKAGE_NAME)'; \
  + echo 'PACKAGE_VERSION = $(PACKAGE_VERSION)'; \
  + echo 'PACKAGE_STRING = $(PACKAGE_STRING)'; \
  + echo 'PACKAGE_TARNAME = $(PACKAGE_TARNAME)'; \
  + echo 'PACKAGE_BUGREPORT = $(PACKAGE_BUGREPORT)'; \
  + echo 'PACKAGE_URL = $(PACKAGE_URL)'; \
 
 git Automake still aims to build with Autoconf 2.62, which didn't
 provide PACKAGE_URL yet,
Oops.
 that is only 2.64+.  Does this test work with older Autoconf
 versions?
Nope.  But the amended test works with autoconf 2.62 and autom4te 2.62
(tested).

  --- /dev/null
  +++ b/tests/backcompat6.test
 
  +# Anyone doing something like this in a real-life package probably
  +# deserves to be killed.
 
 I'm not yet sure whether I really want this particular test.
Well, it's up to you to decide :-)
It depends on which degree of backward-compatibility and/or users' bad
practices you are willing to support (the 'configure.in' in this test is
admittedly a bit extreme in those respects).
For the moment, I've watered down the test a bit.  If you still think it's not
worth having, simply remove it.  But IMHO we should have at least a test
having in 'configure.in' something like:
  PACKAGE=foo
  VERSION=bar
  AM_INIT_AUTOMAKE($PACKAGE, $VERSION)

  diff --git a/tests/init.test b/tests/init.test
  index 38ec681..61a5b63 100755
  --- a/tests/init.test
  +++ b/tests/init.test
  [CUT]
  -cat configure.in END
  -AC_INIT
  -AM_INIT_AUTOMAKE
  +for ac_init_args in '' '([x])'; do
  +  for am_init_args in '' '([1.10])'; do
  +rm -rf aclocal.m4 autom4ate*.cache
 
 Typo autom4ate.
Oops. Fixed.

Regards,
  Stefano
From 77b0461a42045a68d316328cc5a6963c0f1aa796 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Fri, 18 Jun 2010 12:56:47 +0200
Subject: [PATCH] New tests on obsoleted usages of automake/autoconf macros.

* tests/backcompat.test: New test script.
* tests/backcompat2.test: Likewise.
* tests/backcompat3.test: Likewise.
* tests/backcompat4.test: Likewise.
* tests/backcompat5.test: Likewise.
* tests/backcompat6.test: Likewise.
* tests/init.test: Extended and improved, esp. by trying more
combinations of calls to AC_INIT and AM_INIT_AUTOMAKE with few
arguments.
* tests/Makefile.am (TESTS): Updated.
---
 ChangeLog  |   15 +
 tests/Makefile.am  |6 ++
 tests/Makefile.in  |   19 --
 tests/backcompat.test  |   66 
 tests/backcompat2.test |   69 +
 tests/backcompat3.test |  154 
 tests/backcompat4.test |   62 +++
 tests/backcompat5.test |  118 
 tests/backcompat6.test |  104 
 tests/init.test|   29 ++
 10 files changed, 625 insertions(+), 17 deletions(-)
 create mode 100755 tests/backcompat.test
 create mode 100755 tests/backcompat2.test
 create mode 100755 tests/backcompat3.test
 create mode 100755 tests/backcompat4.test
 create mode 100755 tests/backcompat5.test
 create mode 100755 tests/backcompat6.test

diff --git a/ChangeLog b/ChangeLog
index c4e69a5..8c03021 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2010-06-18 

Re: [PATCH] Modernize, improve and/or extend tests `colon*.test.

2010-06-18 Thread Stefano Lattarini
At Thursday 17 June 2010, Ralf Wildenhues wrote:
 * Stefano Lattarini wrote on Sat, Jun 12, 2010 at 11:48:15PM CEST:
  Another patch tweaking scripts in the testsuite.
 
 Thanks, most of this is uncontroversial, but a couple of things I
  don't understand:
  --- a/tests/colon3.test
  +++ b/tests/colon3.test
  [CUT]
  -
  -# Makefile should depend on two.in.
  -echo Grep2
  -grep '^Makefile:.* \$(srcdir)/two.in' zardoz.in
  -# Likewise three.in.
  -echo Grep3
  -grep '^Makefile:.* \$(srcdir)/three.in' zardoz.in
  +$PERL -ne '
  +  s/\bzardoz\.(in|am)\b/zrdz.$1/g;
  +  print if /zardoz/;
  +' zardoz.in out
  +test ! -s out || { cat out; Exit 1; }
 
 Your changes seem to have valued efficiency so far.  But calling
 perl is bound to be more expensive than a couple of greps.
Yes, but it's blazingly fast if compared with an aclocal/automake 
call.

In fact:

 $ time for i in 1 2 3 4; do sh ./colon3.test /dev/null 21; done
 real0m9.366s
 user0m6.656s
 sys 0m1.952s
 $ sed 's/^\$AUTOMAKE$/; exit/' colon3.test foo.test
 $ time for i in 1 2 3 4; do sh ./foo.test /dev/null 21; done
 real0m8.975s
 user0m6.660s
 sys 0m1.888s

And with colon3.test modified by the attached amended patch:

 $ time for i in 1 2 3 4; do sh ./colon3.test /dev/null 21; done
 real0m9.302s
 user0m6.656s
 sys 0m1.924s
 $ sed 's/^\$AUTOMAKE$/; exit/' colon3.test foo.test
 $ time for i in 1 2 3 4; do sh ./foo.test /dev/null 21; done
 real0m8.831s
 user0m6.608s
 sys 0m1.904s

 The change looks good to me, how come you went this way though?
 Just curious.
Because writing that perl script was waaay simpler than cooking up a 
sed script or a sed/grep pipeline doing the same thing portably.  
And even if that perl script isn't much efficient, it is very simple to
understand IMO.
Maybe you can think of a *simple* way to do that which doesn't involve 
perl?  Just curious (no pun intended :-)

 
  +# Makefile should depend on two.in and three.in.
  +grep '^Makefile:.* \$(srcdir)/two\.in' zardoz.in
  +grep '^Makefile:.* \$(srcdir)/three\.in' zardoz.in
 
 FYI, such greps can break in the future, if the dependency lines
  happen to be line-wrapped.  Yes, this has been there before.
Thanks for pointing this out.  I modified the test so that the perl 
script takes care of line wrapping too.

 
  --- a/tests/colon5.test
  +++ b/tests/colon5.test
  [CUT]
 
  +cat  Makefile.am 'END'
  +.PHONY: test
  +test:
  +   case ' $(DIST_COMMON) ' in \
  + *' $(srcdir)/Makefile.dep '*) exit 0;; \
 
 Hmm.  The variable might just as validly list Makefile.dep, without
 preceding `$(srcdir)/'.  That is really necessary only if the file
  is listed both as a prerequisite and as a target somewhere in the
  makefile.
But why should we relax this test if it's working correctly?  After 
all, Automake is expected to have full control in the generation of 
the autotools-related rebuild rules, right?  And we can relax the test 
in the future if the necessity arises.
For the moment, I haven't changed this fragment.  Obviously, you are
free to amend it yourself if you still think it's too strict.

  +
  +sed '/@SET_MAKE@/d' Makefile.in Makefile.sed
  +$MAKE -f Makefile.sed SHELL=$SHELL test
 
 I see this idiom is used a couple of times in the testsuite.  How
  come?
NOTE: the following is just my opinion...  Absolutely no warranty :-)
  Because running ./configure is a bit more expensive than sed?
Yes, in part (and sometimes it is indeed *much* more expensive).  And 
also because, if one wants to run just a small subset of the Makefile's
rules, or to check just a small subset of its variables, he doesn't 
need all the prerequisites ./configure would look for (erroring out if 
it fails to find any of them).  For example, the older versions of
'ansi.test' seemed to use the sed hack to do this.

For the record, if I run the colon5.test in its present sed hack
form, I get:
  $ time for i in {1..10}; do ./colon5.test; done
  real0m24.281s
  user0m16.965s
  sys 0m4.604s
while if I modify it to run also autoconf and configure:
  $ time for i in {1..10}; do ./colon5.test; done
  real0m39.141s
  user0m23.913s
  sys 0m9.525s 

Regards,
Stefano
From 34d0f933468b10233ba14819028bddea62d9743e Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Fri, 18 Jun 2010 12:12:54 +0200
Subject: [PATCH] Modernize, improve and/or extend tests `colon*.test.

* tests/colon.test: Rely on the `configure.in' stub created by
`./defs', rather than writing one from scratch.  Do not create
a useless dummy file.
* tests/colon4.test: Enable the `errexit' shell flag, and
related changes.  Also, rely on the `configure.in' stub created
by `./defs', rather than writing one from scratch.
* tests/colon7.test: Enable `errexit' shell flag, and related
changes.  Also, improve the generated `configure.in' file.
* tests/colon2.test: Likewise.  Also, add some new checks.
* tests/colon5.test: Improve the generated 

Re: pr-msvc-support merge

2010-06-18 Thread Ralf Wildenhues
* Peter Rosin wrote on Wed, Jun 16, 2010 at 10:30:22PM CEST:
 Den 2010-06-16 20:57 skrev Ralf Wildenhues:
 This explanation of yours lends itself nicely to a testsuite addition
 that exercises the 'compile' script (no need to go through Autoconf or
 Automake indirections), as in
cp $testsrcdir/../lib/compile .
./compile ... weird\path\that\has\backslash-t.c ...
...
 
 If you create a script named .../cl that just dumps args you can test
 that compile does the expected thing w/o having the real cl available.

Yep, good idea.

 I have a more general question though: for all of 'cmd //c', cygpath,
 and wineconv, are their conversions idempotent?  I.e., when I insert a
 converted path, do they produce the same path again (a testsuite
 addition could try to verify this).  This might be necessary because we
 have other pending patches for libtool that convert names to host format
 there already, and those should not be broken.
 
 If you look closely, the path conversion is only invoked if the path -
 errmmh file - starts with a single forward slash, so we should be
 completely safe if we are given pre-converted file names.
 
 (I find it difficult is it to not say path when I refer to either
  a directory or a file name, and so do you...)
 
 The cost here is that users with weird mount tables (where a relative
 file name does not mean the same thing in posix-land as it does in
 windows-land) will not get the correct outcome.
 
 But I really didn't do that optimization to cater for pre-converted
 file names, I did it to save a fork in the common case (relative file
 names and a sane mount table).
 
 So, to answer you question, I think that the MSYS converter is safe
 with Windows file names. I also think cygpath is generally safe, but
 I'm sure there are also moronic cases which will not work (directories
 named C: etc). For winepath I don't know.

OK, that sounds good enough to me.

 +   *.o | *.[oO][bB][jJ])
 
 What about *.[oO]?  Is that something we need to take into account?
 
 I thought about that, but decided not to because .o is only there to
 handle the (theoretical?) case where the project is totally unixy (not
 even using $objext) and in that case the likelihood of .O seems very
 remote. I simply didn't think .O would ever appear. What do you think?

Agreed.

 Should I keep attaching the current version of the patch?

Sure, but the patch is good to go once copyright papers are through,
with testing added.

Thanks,
Ralf