[FYI] patch pushed (was: Re: [PATCH 2/2] Extended tests on AC_CONFIG_AUX_DIR).

2010-12-16 Thread Stefano Lattarini
I've now pushed the patch to master.

Regards,
   Stefano



Re: [PATCH] Extend checks on remake rules.

2010-12-16 Thread Stefano Lattarini
On Thursday 16 December 2010, Ralf Wildenhues wrote:
 * Stefano Lattarini wrote on Thu, Dec 16, 2010 at 01:27:10AM CET:
  On Wednesday 15 December 2010, Ralf Wildenhues wrote:
 [ magic strings ]
   Why all the variation in these?  That makes the tests harder to read.
  
  I'd rather not change the use of magic strings on the other remake
  tests, unless you insist.  The test remak3a.test is IMHO simple enough
  not to cause confusion, and changing the tests remake8{a,b}.test would
  be quite cumbersome.
 
 I don't insist.

Thanks.

+   test x'$(am_fingerprint)' = x'DummyValue'
   
   Why do you quote DummyValue here?
  
  Partly for symmetry with the left side, and partly (especially I'd say)
  to make the leading `x' stick out better as not a part of the expected
  value.
  
   The leading x should not be needed on either side.
  
  True, should not -- but I got in the habit of always using it, even
  where technically not needed, rather then risking to forgot it one time
  when it's really required.
  
   (several instances below)
  
  Should I remove them? (sorry if I ask, but the should above does
  not make it clear if you're asking to remove them or just noting that
  they are not really required).
 
 Ah, the fun of English negation and passive voice.  I'm not totally firm
 in this area either, but I think that should not be needed has the
 meaning of is not needed, at least I think so rather than don't do
 this or I'll poke you with a stick.

I think the same way too, but since you wrote ok with nits addressed
in the beginning of your mail, I wasn't sure whether that should not
were pointing out a nit (hence to be addressed).

 I hope somebody corrects me on this if my interpretation is wrong.
 
  Attached is also the amended patch.  I will push in 72 hours if there
  are no more objections (and if all my testing on Linux and Solaris
  succeeds).
 
 I don't object, but TBH, I didn't look at the patch again.  ;-)

Thast shouldn't be really necessary if you've looked at my inline
comments.

 Thanks!
 Ralf
 

Thanks for the approval.  I've now merged the patch into master,
and pushed.

Regards,
   Stefano



[FYI] patch pushed (was: [PATCH] Improve tests on generated portions of configure help screen.)

2010-12-16 Thread Stefano Lattarini
I've now pushed the patch to maint, and merged maint into master.

Regards,
   Stefano



[FYI] patch pushed (was: Re: [PATCH] Improvements and extend tests on canonicalization.)

2010-12-16 Thread Stefano Lattarini
Pushed to master now.

Regards,
  Stefano



[PATCH] {master} Improve and extend tests `suffix*.test'.

2010-12-16 Thread Stefano Lattarini
Hello automakers.

Here it is one of the last (5 or 6) testsuite-only patches I have in
my repository.

Tested successfully:

  - Debian GNU/Linux, system GNU make (3.81)  bash (4.1), gcc 4.4.4,
libtool 2.2.6b, autoconf 2.68.

  - Debian GNU/Linux, GNU make 3.79, dash (0.5.5.1), gcc 4.0.1,
libtool 2.2.6b, autoconf 2.68

  - Debian GNU/Linux, GNU make 3.82, dash (0.5.2), gcc 2.95,
libtool 2.2.6b, autoconf 2.68

  - Debian GNU/Linux, GNU make 3.80, pdksh (5.2.14), gcc 3.3,
libtool libtool 1.5.6, autoconf 2.65

  - Debian GNU/Linux, GNU make 3.78.1, ksh (Version JM 93t+ 2009-05-01),
SunStudio C compiler (Sun C 5.10 Linux_i386 2009/06/03),
libtool 2.2.2, autoconf 2.62

  - Debian GNU/Linux, FreeBSD make (from freebsd-buildutils 8.0),
bash 2.05, gcc 3.4, libtool 2.2.2, autoconf 2.62

  - Debian GNU/Linux, NetBSD make (from pmake-1.111 [sic]), bash 3.0,
SunStudio C compiler (Sun C 5.10 Linux_i386 2009/06/03),
libtool 2.2.8, autoconf 2.63

  - Solaris 10, GNU make 3.82, Solaris /bin/sh, libtool 2.4, autoconf 2.68
SunStudio C compiler (Sun C 5.9 SunOS_i386 Patch 124868-15 2010/08/11),
some GNU tools (sed, expr, awk) available

Tested with some errors:

  - Solaris 10, CCS make, Solaris /bin/ksh, libtool 2.4, autoconf 2.68,
SunStudio C compiler (Sun C 5.9 SunOS_i386 Patch 124868-15 2010/08/11)
only Solaris utilities available

  - Solaris 10, XPG4 make, Solaris /bin/sh, libtool 2.4, autoconf 2.68,
gcc 4.4.4, only Solaris utilities available

  - Solaris 10, dmake, Solaris /bin/sh, libtool 2.4, autoconf 2.68,
gcc 4.4.4, some GNU tools (sed, expr, awk) available

  - Debian GNU/Linux, Heirloom make and shell (2007), gcc 4.2.4,
libtool 2.2.8, autoconf 2.67, and with Heirloom utilities (sed,
grep, etc.) preferred over system (GNU) utilities

In all the setups indicated as with some errors above, the tests
`suffix3.test', `suffix8.test', `suffix10.test' and `suffix11.test'
failed due to the inability of those make implementations to chain
implicit rules.

I would have worked around those failures by either declaring hidden
dependencies explicitly in the Makefile.am files, or by skipping the
affected tests if the make implementation is found unable to chain
implicit dependencies.  But since there's no consensus on this yet,
I'd rather leave the tests fail FTM.  They can be adjusted in a
follow-up patch anyway (or better, Automake could be improved to
work around the limitations of those make implementations).

OK to apply to a temporary branch off of maint, and merge to master?
I will push in 72 hours (Sunday evening) if there is no objection
or review by then.

Regards,
   Stefano
From 497eb149e4b470f00afe477ce8b5e2da63c61252 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Fri, 15 Oct 2010 17:37:38 +0200
Subject: [PATCH] Improve and extend tests `suffix*.test'.

* tests/suffix.test: Check that suffix rules for C compilation are
only included once.  Try also with a static library.
* tests/suffix2.test: Add a new grep to help potential debugging.
Do not run automake with the `--add-missing' options, since we
already create all the needed auxiliary files.  Try also *without*
the `no-dependencies' automake option.
* tests/suffix4.test: Make grepping of Makefile.in stricter.
* tests/suffix3.test: Rewritten to run also autoconf, ./configure
and make.
* tests/suffix5.test: Likewise.
* tests/suffix6.test: Fix botched recipe indentation (eight spaces
were used instead of a tabulation character).  Extend to check
that `.obj' is handled like `.$(OBJEXT)' (as is done for `.o').
Improved parsing  grepping of generated Makefile.in.  Other minor
fixes and improvements.
* tests/suffix10.test: Move some checks in Makefile.am.  Also run
make all.
* tests/suffix12.test: Likewise, and account for VPATH issues in
weaker make implementations.
* tests/suffix11.test: Likewise.  Also, run make distcheck, for
completeness, and related changes.
* tests/suffix8.test: Likewise.  Also, do not put `gcc' anymore
in $required.
* tests/suffix13.test: Do not use the `--force-missing' automake
option unnecessarily.
* tests/suffix6b.test: New test, semantic sister of `suffix6.test'.
* tests/suffix6c.test: Likewise.
* tests/Makefile.am (TESTS): Updated.
---
 ChangeLog   |   32 ++
 tests/Makefile.am   |2 +
 tests/Makefile.in   |2 +
 tests/suffix.test   |   14 ++--
 tests/suffix10.test |   31 -
 tests/suffix11.test |   37 +---
 tests/suffix12.test |   18 ++
 tests/suffix13.test |2 +-
 tests/suffix2.test  |   13 +--
 tests/suffix3.test  |   37 +++--
 tests/suffix4.test  |4 ++-
 tests/suffix5.test  |   24 +++--
 tests/suffix6.test  |   37 +++-
 tests/suffix6b.test |   77 ++
 tests/suffix6c.test |   92 +++
 tests/suffix8.test  |   67 

Avoid false positive in sc_tests_plain_make maintainer-check.

2010-12-16 Thread Ralf Wildenhues
I'm seeing this in master now:

../automake/tests/remake11.test:makefiles_am_list=`find . -name Makefile.am | 
LC_ALL=C sort`
../automake/tests/remake11.test:makefiles_list=`echo $makefiles_am_list | sed 
's/\.am$//'`
Do not run make in the above tests.  Use $MAKE instead.
Makefile:1090: recipe for target `sc_tests_plain_make' failed
make: *** [sc_tests_plain_make] Error 1

Merging the patch below in the tests-remake-extend branch to avoid it.

Thanks,
Ralf

Avoid false positive in sc_tests_plain_make maintainer-check.

* Makefile.am (sc_tests_plain_make): Ensure to only match full
`make' words.  Avoid false positive with remake11.test.

diff --git a/Makefile.am b/Makefile.am
index ae096cb..15aa3b3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -342,7 +342,7 @@ sc_tests_make_without_am_makeflags:
 
 ## Tests should never call make directly.
 sc_tests_plain_make:
-   @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[  ]*make'; then \
+   @if grep -v '^#' $(srcdir)/tests/*.test | $(EGREP) ':[  ]*make( |$$)'; 
then \
  echo 'Do not run make in the above tests.  Use $$MAKE instead.' 
12; \
  exit 1; \
fi
diff --git a/Makefile.in b/Makefile.in
index 04d7067..1c1bf81 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1087,7 +1087,7 @@ sc_tests_make_without_am_makeflags:
fi
 
 sc_tests_plain_make:
-   @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[  ]*make'; then \
+   @if grep -v '^#' $(srcdir)/tests/*.test | $(EGREP) ':[  ]*make( |$$)'; 
then \
  echo 'Do not run make in the above tests.  Use $$MAKE instead.' 
12; \
  exit 1; \
fi



Re: [PATCH] Improvements and extend tests on canonicalization.

2010-12-16 Thread Stefano Lattarini
On Thursday 16 December 2010, Ralf Wildenhues wrote:
  --- /dev/null
  +++ b/tests/canon7.test
  @@ -0,0 +1,93 @@
 
  +# Stress test on canonicalization.
 [...]
  +cat  Makefile.am  'END'
  +noinst_PROGRAMS = dummy_static dummy_dynamic ,foo-bar
  +noinst_LIBRARIES = libb.az+baz.a
  +noinst_LTLIBRARIES = lib~zardoz,,.la
  +
  +dummy_static_SOURCES = dummy.c lib.h
  +dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
  +
  +dummy_static_LDADD = $(noinst_LIBRARIES)
  +dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)
  +
  +_foo_bar_SOURCES = libs.c
  +libb_az_baz_a_SOURCES = libs.c
  +lib_zardoz___la_SOURCES = libd.c
  +
  +check-local:
  +   ls -l
  +   ./,foo-bar
  +   ./dummy_static
  +   ./dummy_dynamic
  +   ./,foo-bar | grep 'Hello, FooBar!'
  +   ./dummy_static | grep 'Hello from Static!'
  +   ./dummy_dynamic | grep 'Hello from Dynamic!'
  +END
  +
  +cat  foobar.c  'END'
  +#include stdio.h
  +int main(void)
  +{
  +  printf(Hello, FooBar!\n);
  +  return 0;
  +}
  +END
  +
  +cat  dummy.c  'END'
  +#include stdio.h
  +#include lib.h
  +int main(void)
  +{
  +  printf(Hello from %s!\n, dummy_func());
  +  return 0;
  +}
  +END
  +
  +echo 'char *dummy_func(void);'  lib.h
  +echo 'char *dummy_func(void) { return Dynamic; }'  libd.c
  +echo 'char *dummy_func(void) { return Static; }'  libs.c
 
 I'm not sure how you tested this, but ',foo-bar' fails to link for me on
 GNU/Linux (and supposedly everywhere else, lacking a 'main' symbol).

Yes, and I cant' understand how I could have missed that.  I mean, I
clearly remember running the tests after the squashed-in amendaments...
(asking to self) What kind of blunder did I do?

Anyway, sorry for the noise and thanks for fixing this.  Just a
nit below (inlined in your patch) ...

 Also, the code passes pointers to read-only memory (literal strings) in
 non-const pointers (literal strings may not be modifiable).

Sorry again. My C-fu is not very strong :-(

 I'm pushing the fix below.
 
 Note that you're lucky using convenience archives here.  With shared
 libraries, the const data would need special handling in order to work
 on w32.  (See the Libtool documentation, git version, for details.)
 
 Thanks,
 Ralf
 
 Fix canon7.test failure.
 
 * tests/canon7.test (_foo_bar_SOURCES): Add foobar.c.
 (lib.h, libd.c, libs.c): Use const for constant strings.
 
 diff --git a/tests/canon7.test b/tests/canon7.test
 index 9b3d8d0..3f25d6f 100755
 --- a/tests/canon7.test
 +++ b/tests/canon7.test
 @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
  dummy_static_LDADD = $(noinst_LIBRARIES)
  dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)
  
 -_foo_bar_SOURCES = libs.c
 +_foo_bar_SOURCES = libs.c foobar.c

_foo_bar_SOURCES = foobar.c should suffice here.

  libb_az_baz_a_SOURCES = libs.c
  lib_zardoz___la_SOURCES = libd.c
  
 @@ -76,9 +76,9 @@ int main(void)
  }
  END
  
 -echo 'char *dummy_func(void);'  lib.h
 -echo 'char *dummy_func(void) { return Dynamic; }'  libd.c
 -echo 'char *dummy_func(void) { return Static; }'  libs.c
 +echo 'const char *dummy_func(void);'  lib.h
 +echo 'const char *dummy_func(void) { return Dynamic; }'  libd.c
 +echo 'const char *dummy_func(void) { return Static; }'  libs.c
  
  libtoolize
  $ACLOCAL
 

Thanks,
   Stefano



Re: [PATCH] Improvements and extend tests on canonicalization.

2010-12-16 Thread Ralf Wildenhues
* Stefano Lattarini wrote on Sat, Dec 11, 2010 at 06:41:46PM CET:
 Ping on this? Reference:
  http://lists.gnu.org/archive/html/automake-patches/2010-09/msg00106.html
 
 The updated patch is attached.  I will push it in 72 hours (by tuesday
 evening) unless there are objections.

 --- /dev/null
 +++ b/tests/canon7.test
 @@ -0,0 +1,93 @@

 +# Stress test on canonicalization.
[...]
 +cat  Makefile.am  'END'
 +noinst_PROGRAMS = dummy_static dummy_dynamic ,foo-bar
 +noinst_LIBRARIES = libb.az+baz.a
 +noinst_LTLIBRARIES = lib~zardoz,,.la
 +
 +dummy_static_SOURCES = dummy.c lib.h
 +dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
 +
 +dummy_static_LDADD = $(noinst_LIBRARIES)
 +dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)
 +
 +_foo_bar_SOURCES = libs.c
 +libb_az_baz_a_SOURCES = libs.c
 +lib_zardoz___la_SOURCES = libd.c
 +
 +check-local:
 + ls -l
 + ./,foo-bar
 + ./dummy_static
 + ./dummy_dynamic
 + ./,foo-bar | grep 'Hello, FooBar!'
 + ./dummy_static | grep 'Hello from Static!'
 + ./dummy_dynamic | grep 'Hello from Dynamic!'
 +END
 +
 +cat  foobar.c  'END'
 +#include stdio.h
 +int main(void)
 +{
 +  printf(Hello, FooBar!\n);
 +  return 0;
 +}
 +END
 +
 +cat  dummy.c  'END'
 +#include stdio.h
 +#include lib.h
 +int main(void)
 +{
 +  printf(Hello from %s!\n, dummy_func());
 +  return 0;
 +}
 +END
 +
 +echo 'char *dummy_func(void);'  lib.h
 +echo 'char *dummy_func(void) { return Dynamic; }'  libd.c
 +echo 'char *dummy_func(void) { return Static; }'  libs.c

I'm not sure how you tested this, but ',foo-bar' fails to link for me on
GNU/Linux (and supposedly everywhere else, lacking a 'main' symbol).
Also, the code passes pointers to read-only memory (literal strings) in
non-const pointers (literal strings may not be modifiable).  I'm pushing
the fix below.

Note that you're lucky using convenience archives here.  With shared
libraries, the const data would need special handling in order to work
on w32.  (See the Libtool documentation, git version, for details.)

Thanks,
Ralf

Fix canon7.test failure.

* tests/canon7.test (_foo_bar_SOURCES): Add foobar.c.
(lib.h, libd.c, libs.c): Use const for constant strings.

diff --git a/tests/canon7.test b/tests/canon7.test
index 9b3d8d0..3f25d6f 100755
--- a/tests/canon7.test
+++ b/tests/canon7.test
@@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
 dummy_static_LDADD = $(noinst_LIBRARIES)
 dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)
 
-_foo_bar_SOURCES = libs.c
+_foo_bar_SOURCES = libs.c foobar.c
 libb_az_baz_a_SOURCES = libs.c
 lib_zardoz___la_SOURCES = libd.c
 
@@ -76,9 +76,9 @@ int main(void)
 }
 END
 
-echo 'char *dummy_func(void);'  lib.h
-echo 'char *dummy_func(void) { return Dynamic; }'  libd.c
-echo 'char *dummy_func(void) { return Static; }'  libs.c
+echo 'const char *dummy_func(void);'  lib.h
+echo 'const char *dummy_func(void) { return Dynamic; }'  libd.c
+echo 'const char *dummy_func(void) { return Static; }'  libs.c
 
 libtoolize
 $ACLOCAL



Re: [PATCH] Improvements and extend tests on canonicalization.

2010-12-16 Thread Ralf Wildenhues
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 08:16:25PM CET:
 On Thursday 16 December 2010, Ralf Wildenhues wrote:
  --- a/tests/canon7.test
  +++ b/tests/canon7.test
  @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
   dummy_static_LDADD = $(noinst_LIBRARIES)
   dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)
   
  -_foo_bar_SOURCES = libs.c
  +_foo_bar_SOURCES = libs.c foobar.c
 
 _foo_bar_SOURCES = foobar.c should suffice here.

Feel free to fix that.

Thanks!
Ralf



Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.

2010-12-16 Thread Ralf Wildenhues
* Stefano Lattarini wrote on Mon, Dec 13, 2010 at 07:54:05PM CET:
 OK to apply to a temporary branch off of maint, and merge to master?

The patch is ok with nits addressed.

 BTW, notice that I'm planning to further extend the Lex/Yacc tests
 and make them more semantic, but that should be better done in a
 follow-up patch IMVHO.

If there is any way I can get you to not do any more of such
conversions: Please don't work even more on *these* tests.  They are
Good Enough[tm].  The incremental gain from more change is not worth
the additional work from you nor review from me.


Actually, lex and yacc support has *several* *real* issues, with maybe
more than a dozen reported bugs in the last few years, many of them
unfixed.  See the list archives.  It would be really nice if somebody
who knows their ways around bison/yacc and flex/lex well (or is willing
to learn) could look into some of the issues.  I should warn that it's
not easy though to come up with coherent/consistent and portable
improvements.


 Subject: [PATCH] Extend, fix and improve tests on Lex and Yacc support.
 
 * tests/lexcpp.test: New test script, on support for Lex + C++.
 * tests/lexvpath.test: New test script, test build and rebuild
 rules for lexers in VPATH setup.
 * tests/yacc-basic.test: New test script, run simple semantic
 checks on basic Yacc support (similarly to what lex3.test does
 for Lex support).
 * tests/lex.test: Don't create useless dummy source file joe.l.
 Remove extra blank lines.
 (Makefile.am): Remove useless definition of $(LDADD).
 * tests/lex4.test: Add trailing `:' command.  Do not create dummy
 useless lex source file.
 * tests/lex2.test: Likewise.  Call automake with the `-a' option,
 so that it doesn't fail for the absence of `ylwrap' script.  Make
 grepping of automake stderr stricter.
 * tests/yacc7.test: Add trailing `:' command.  Enable `errexit'
 shell flag earlier (just after having sourced ./defs).
 * tests/yacc4.test: Likewise.  Also ...
 (configure.in): Use pre-populated skeleton set up by ./defs,
 instead of writing one from scratch.
 Other minor cosmetic changes.
 * tests/yacc5.test: Likewise.
 * tests/yaccvpath.test: Likewise. Also ...
 ($distdir): New variable.
 Use it throughout.
 * tests/lex5.test: Likewise.
 * tests/lex3.test: Likewise.  Check the distdir, rather than
 grepping the distribution tarball.  Extend the test on the
 created binary, and be sure to avoid hangs.  Add some comments.
 * tests/yacc.test: Use stricter grepping.  Add trailing `:'.
 * tests/yacc6.test: Likewise.
 * tests/yacc3.test: Likewise.  Prefer `cp -f' over plain `cp'.
 Do not create unused file `Makefile.sed'.  Remove useless rules
 from Makefile.am.  Other minor cosmetic changes.
 * tests/yacc2.test: Make grepping of generated `Makefile.in' and
 of automake error messages stricter.  Do not redirect output of
 grep to /dev/null.  Prefer `cp -f' over plain `cp'.  Move call to
 aclocal earlier.  Reduce the number of empty blank lines.  Fix a
 typo in comments.
 * tests/yacc8.test: Fixed bugs that reduced the completeness of
 the tests.  Added trailing `:' command.
 (configure.in): Use pre-populated skeleton set up by ./defs,
 instead of writing one from scratch.
 * tests/yaccpp.test: Test also extensions `.y++', `.ypp', and
 `.yxx', rather than only `.yy'.
 * tests/defs.in ($required): Better recognition of requirement
 flex.
 * tests/Makefile.am (TESTS): Update.


 --- a/tests/defs.in
 +++ b/tests/defs.in
 @@ -113,6 +113,13 @@ do
echo $me: running etags --version -o /dev/null
( etags --version -o /dev/null ) || exit 77
;;
 +flex)
 +  # Since flex is required, we pick LEX for ./configure.
 +  LEX=flex
 +  export LEX
 +  echo $me: running flex --version
 +  ( flex --version ) || exit 77
 +  ;;

I don't understand why the new 'required=flex' entry should be needed in
defs.in.  Any tests that fail without the defs.in change?

 +++ b/tests/lex.test
 @@ -26,22 +26,16 @@ END
  cat  Makefile.am  'END'
  bin_PROGRAMS = zot
  zot_SOURCES = joe.l
 -LDADD = @LEXLIB@

Please don't remove that.  It is documented to be required.

  END


 --- a/tests/lex3.test
 +++ b/tests/lex3.test

 +# Basic semantic checks on Lex support.
  # Test associated with PR 19.
  # From Matthew D. Langston.

  cat  Makefile.am  'END'
 @@ -46,38 +44,44 @@ END
  
  cat  foo.l  'END'
  %%
 -END   return EOF;
 +GOOD   return EOF;
  .
  %%
  int
  main ()
  {
 -  while (yylex () != EOF)
 -;
 -
 -  return 0;
 +  if (yylex () == EOF)
 +return 0;
 +  else
 +return 1;

That's not semantic either.  One wouldn't write a lexer like that.

  }
  END

 +# Program should build and run.
  $MAKE
 -echo 'This is the END' | ./foo
 -$MAKE distcheck
 +echo GOOD | ./foo
 +echo BAD | ./foo  Exit 1

 --- a/tests/lex5.test
 +++ b/tests/lex5.test

 @@ -33,10 +29,9 @@ AC_OUTPUT
  END
  
  cat  Makefile.am  'END'
 -AUTOMAKE_OPTIONS  = subdir-objects
 -LDADD = @LEXLIB@
 -
 -bin_PROGRAMS= foo/foo
 

Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.

2010-12-16 Thread Stefano Lattarini
On Thursday 16 December 2010, Ralf Wildenhues wrote:
 * Stefano Lattarini wrote on Mon, Dec 13, 2010 at 07:54:05PM CET:
  OK to apply to a temporary branch off of maint, and merge to master?
 
 The patch is ok with nits addressed.

I've addressed almost all of them, but I have a doubt about one (see
defs.in below) and I disagree with one (see lex3.test below).

  BTW, notice that I'm planning to further extend the Lex/Yacc tests
  and make them more semantic, but that should be better done in a
  follow-up patch IMVHO.
 
 If there is any way I can get you to not do any more of such
 conversions: Please don't work even more on *these* tests.  They are
 Good Enough[tm].

IMHO they're not.  See just below.

 The incremental gain from more change is not worth the additional
 work from you nor review from me.
 
 Actually, lex and yacc support has *several* *real* issues, with maybe
 more than a dozen reported bugs in the last few years, many of them
 unfixed.  See the list archives.

Yes, but I woldn't dare trying to modify the Lex/Yacc related code with
the limited understanding I have of it, without having a *much* stronger
testsuite than it's currently available.

 It would be really nice if somebody who knows their ways around
 bison/yacc and flex/lex well

I'm not that somebody, unfortunately.

 (or is willing to learn)

Well, maybe I might *try* to be this somebody...  once I feel
backed up by a strong-enough testsuite.

 could look into some of the issues.  I should warn that it's not
 easy though to come up with coherent/consistent and portable
 improvements.

 
  Subject: [PATCH] Extend, fix and improve tests on Lex and Yacc support.
  
  * tests/lexcpp.test: New test script, on support for Lex + C++.
  * tests/lexvpath.test: New test script, test build and rebuild
  rules for lexers in VPATH setup.
  * tests/yacc-basic.test: New test script, run simple semantic
  checks on basic Yacc support (similarly to what lex3.test does
  for Lex support).
  * tests/lex.test: Don't create useless dummy source file joe.l.
  Remove extra blank lines.
  (Makefile.am): Remove useless definition of $(LDADD).
  * tests/lex4.test: Add trailing `:' command.  Do not create dummy
  useless lex source file.
  * tests/lex2.test: Likewise.  Call automake with the `-a' option,
  so that it doesn't fail for the absence of `ylwrap' script.  Make
  grepping of automake stderr stricter.
  * tests/yacc7.test: Add trailing `:' command.  Enable `errexit'
  shell flag earlier (just after having sourced ./defs).
  * tests/yacc4.test: Likewise.  Also ...
  (configure.in): Use pre-populated skeleton set up by ./defs,
  instead of writing one from scratch.
  Other minor cosmetic changes.
  * tests/yacc5.test: Likewise.
  * tests/yaccvpath.test: Likewise. Also ...
  ($distdir): New variable.
  Use it throughout.
  * tests/lex5.test: Likewise.
  * tests/lex3.test: Likewise.  Check the distdir, rather than
  grepping the distribution tarball.  Extend the test on the
  created binary, and be sure to avoid hangs.  Add some comments.
  * tests/yacc.test: Use stricter grepping.  Add trailing `:'.
  * tests/yacc6.test: Likewise.
  * tests/yacc3.test: Likewise.  Prefer `cp -f' over plain `cp'.
  Do not create unused file `Makefile.sed'.  Remove useless rules
  from Makefile.am.  Other minor cosmetic changes.
  * tests/yacc2.test: Make grepping of generated `Makefile.in' and
  of automake error messages stricter.  Do not redirect output of
  grep to /dev/null.  Prefer `cp -f' over plain `cp'.  Move call to
  aclocal earlier.  Reduce the number of empty blank lines.  Fix a
  typo in comments.
  * tests/yacc8.test: Fixed bugs that reduced the completeness of
  the tests.  Added trailing `:' command.
  (configure.in): Use pre-populated skeleton set up by ./defs,
  instead of writing one from scratch.
  * tests/yaccpp.test: Test also extensions `.y++', `.ypp', and
  `.yxx', rather than only `.yy'.
  * tests/defs.in ($required): Better recognition of requirement
  flex.
  * tests/Makefile.am (TESTS): Update.

  --- a/tests/defs.in
  +++ b/tests/defs.in
  @@ -113,6 +113,13 @@ do
 echo $me: running etags --version -o /dev/null
 ( etags --version -o /dev/null ) || exit 77
 ;;
  +flex)
  +  # Since flex is required, we pick LEX for ./configure.
  +  LEX=flex
  +  export LEX
  +  echo $me: running flex --version
  +  ( flex --version ) || exit 77
  +  ;;
 
 I don't understand why the new 'required=flex' entry should be needed in
 defs.in.  Any tests that fail without the defs.in change?
 
Not for me, but since we do something similar for bison, I was just trying
to be consistent.  Should I remove this hunk?

  +++ b/tests/lex.test
  @@ -26,22 +26,16 @@ END
   cat  Makefile.am  'END'
   bin_PROGRAMS = zot
   zot_SOURCES = joe.l
  -LDADD = @LEXLIB@
 
 Please don't remove that.  It is documented to be required.

OK (even if automake doesn't currently require it at automake-time).

   END
 
 
  --- 

Re: [PATCH] Improvements and extend tests on canonicalization.

2010-12-16 Thread Stefano Lattarini
On Thursday 16 December 2010, Ralf Wildenhues wrote:
 * Stefano Lattarini wrote on Thu, Dec 16, 2010 at 08:16:25PM CET:
  On Thursday 16 December 2010, Ralf Wildenhues wrote:
   --- a/tests/canon7.test
   +++ b/tests/canon7.test
   @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
dummy_static_LDADD = $(noinst_LIBRARIES)
dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)

   -_foo_bar_SOURCES = libs.c
   +_foo_bar_SOURCES = libs.c foobar.c
  
  _foo_bar_SOURCES = foobar.c should suffice here.
 
 Feel free to fix that.

Below's what I pushed.  I tested it with both GCC and Sun Studio
C compiler, both before and after merging to master.  I hope this
will avoid me another embarassing blunder.

Regards,
   Stefano

-*-*-*-

Minor cleanups in canon7.test.

* tests/canon7.test (_foo_bar_SOURCES): Remove libs.c.
(configure.in): Remove AC_PROG_CXX.
---
 ChangeLog |6 ++
 tests/canon7.test |3 +--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9386d80..cb8c207 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2010-12-17  Stefano Lattarini  stefano.lattar...@gmail.com
+
+   Minor cleanups in canon7.test.
+   * tests/canon7.test (_foo_bar_SOURCES): Remove libs.c.
+   (configure.in): Remove AC_PROG_CXX.
+
 2010-12-16  Ralf Wildenhues  ralf.wildenh...@gmx.de
 
Fix canon7.test failure.
diff --git a/tests/canon7.test b/tests/canon7.test
index 3f25d6f..b85a835 100755
--- a/tests/canon7.test
+++ b/tests/canon7.test
@@ -23,7 +23,6 @@ set -e
 
 cat  configure.in  'END'
 AC_PROG_CC
-AC_PROG_CXX
 AC_PROG_RANLIB  dnl: for static libraries
 AC_PROG_LIBTOOL dnl: for libtool libraries
 AC_OUTPUT
@@ -43,7 +42,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES)
 dummy_static_LDADD = $(noinst_LIBRARIES)
 dummy_dynamic_LDADD = $(noinst_LTLIBRARIES)
 
-_foo_bar_SOURCES = libs.c foobar.c
+_foo_bar_SOURCES = foobar.c
 libb_az_baz_a_SOURCES = libs.c
 lib_zardoz___la_SOURCES = libd.c
 
-- 
1.7.1




Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.

2010-12-16 Thread Ralf Wildenhues
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 10:10:29PM CET:
 On Thursday 16 December 2010, Ralf Wildenhues wrote:
 
   BTW, notice that I'm planning to further extend the Lex/Yacc tests
   and make them more semantic, but that should be better done in a
   follow-up patch IMVHO.
  
  If there is any way I can get you to not do any more of such
  conversions: Please don't work even more on *these* tests.  They are
  Good Enough[tm].
 
 IMHO they're not.  See just below.
 
  The incremental gain from more change is not worth the additional
  work from you nor review from me.
  
  Actually, lex and yacc support has *several* *real* issues, with maybe
  more than a dozen reported bugs in the last few years, many of them
  unfixed.  See the list archives.
 
 Yes, but I woldn't dare trying to modify the Lex/Yacc related code with
 the limited understanding I have of it, without having a *much* stronger
 testsuite than it's currently available.

The *current* tests are good enough for the current code.  How many bugs
have we found due to the testsuite frenzy in the last few months with
its 200+ patches?  I'd guess less than 5.  That's an awful signal to
noise ratio.  How much have you gotten to know the Automake code base
(outside of tests/) as a result of that?  I don't know, I cannot guess,
but normally one needs to work on code to get to know it really.

Why not enhance the testsuite as you go along fixing bugs or otherwise
improving code outside the testsuite?  That ensures that we progress.
Also, while working on the code, it is often clearer which semantics you
are possibly changing; then, we can expose them in the testsuite as we
stumble over them.

  It would be really nice if somebody who knows their ways around
  bison/yacc and flex/lex well
 
 I'm not that somebody, unfortunately.

Don't give yourself up before starting.

  (or is willing to learn)
 
 Well, maybe I might *try* to be this somebody...  once I feel
 backed up by a strong-enough testsuite.

I don't believe this argumentation, for reasons stated above.

Of course good testsuite coverage doesn't ever fully make up for knowing
portability issues, semantics of tools, and searching history and
archives regularly.  That just comes with it.

   --- a/tests/defs.in
   +++ b/tests/defs.in
   @@ -113,6 +113,13 @@ do
  echo $me: running etags --version -o /dev/null
  ( etags --version -o /dev/null ) || exit 77
  ;;
   +flex)
   +  # Since flex is required, we pick LEX for ./configure.
   +  LEX=flex
   +  export LEX
   +  echo $me: running flex --version
   +  ( flex --version ) || exit 77
   +  ;;
  
  I don't understand why the new 'required=flex' entry should be needed in
  defs.in.  Any tests that fail without the defs.in change?
  
 Not for me, but since we do something similar for bison, I was just trying
 to be consistent.  Should I remove this hunk?

Yes, please.  We don't need unneeded code.

cat  foo.l  'END'
%%
   -END   return EOF;
   +GOOD   return EOF;
.
%%
int
main ()
{
   -  while (yylex () != EOF)
   -;
   -
   -  return 0;
   +  if (yylex () == EOF)
   +return 0;
   +  else
   +return 1;
  
  That's not semantic either.  One wouldn't write a lexer like that.
 
 Yes, but this will have IMVHO a lower risk of hanging due to possible
 errors/blunders in other parts of the testcase.  Hanging which, BTW,
 would happen ...
 
}
END
  
   +# Program should build and run.
$MAKE
   -echo 'This is the END' | ./foo
   -$MAKE distcheck
   +echo GOOD | ./foo
   +echo BAD | ./foo  Exit 1
  
 ... here.
 
 I think we should keep the lexer my stupid but safer way.

Maybe; but *now*, that is a good time for adding a comment in main
above, because that is one bit of unobvious code.  (Unobvious is the
reason why the code does not look nor work like a normal lexer.)
OK with that fixed.

# Don't redefine several times the same variable.
   -cp Makefile.am Makefile.src
   +cp -f Makefile.am Makefile.src
  
  Why should you need this change?  Generally, I don't see why you ever
  need 'cp -f'.
 
 I dimly remember that I used to think there were cp implementations which
 might prompt if stdout/stderr is a tty and the dest file exists, unless
 the `-f' option is used..

That is true, but when you run a test, there is no tty involved.
What am I missing?

 But I might be very mistaken here, and since
 you tell me the `-f' is useless ...

No.  It is useless *here*.

Thanks,
Ralf