Re: [PATCH] Modernize and improve test scripts `subdir*.test'.

[Ralf Wildenhues]

Hi Stefano,

* Stefano Lattarini wrote on Sat, Jun 19, 2010 at 10:15:58PM CEST:
> Modernize and improve test scripts `subdir*.test'.
> 
> * tests/subdir.test: Enable `errexit' shell flag, and related
> changes.  Use the `configure.in' stub created by `./defs',
> rather than writing one from scratch.
> * tests/subdir2.test: Likewise.
> * tests/subdir4.test: Likewise.
> * tests/subdir3.test: Enable `errexit' shell flag, and related
> changes.  Do not create useless dummy source files.
> * tests/subdir8.test: Prefer fgrep over grep.  Add trailing `:'
> command.  Some minor changes to keep it more in sync with sister
> test `subdir5.test'.
> * tests/subdir5.test: Refactored and modified to minimize diffs
> with sister test `subdir8.test'.
> * tests/subdir6.test: Cosmetic change in spacing.
> * tests/subdir9.test: Define and use new variable `$distdir'.  Add
> trailing `:' command.
> * tests/subdir10.test: Cosmetic consistency-related change.
> * tests/subdirbuiltsources.test: Cosmetic changes in spacings.
> (configure.in): Use stub created by `./defs', rather than writing
> it from scratch.  Do not use obsoleted and/or deprecated forms of
> autoconf/automake macros.

This is ok for maint with nits below addressed.

> diff --git a/tests/subdir5.test b/tests/subdir5.test
> index 2f0113b..0709440 100755
> --- a/tests/subdir5.test
> +++ b/tests/subdir5.test

> @@ -27,14 +27,15 @@ required='GNUmake gcc'
>  
>  set -e
>  
> -cat > configure.in << 'END'
> -AC_INIT(maude, 1.0)
> -AM_INIT_AUTOMAKE
> +cat >> configure.in << 'END'
> +AC_PROG_CC
>  AM_PROG_CC_C_O
> -AC_CONFIG_FILES(Makefile)
> +m4_include([confiles.m4])

I don't understand why this change doesn't change the nature of the
test.  I mean, the first step users typically do when adding a new
subdir is editing configure.ac, not editing a file that is included
by configure.ac.  I think the test really is meant to simulate just
that, and if it is, it should be kept that way.  Also because when
the m4_include is not present in the first version of the file, there
is no dependency on confiles.m4 which makes a difference for the rerun
and this different code path should be kept.

Am I missing something?

>  AC_OUTPUT
>  END
>  
> +: > confiles.m4
> +
>  cat > Makefile.am << 'END'
>  bin_PROGRAMS = wish
>  wish_SOURCES = a.c
> @@ -55,47 +56,49 @@ $AUTOMAKE --include-deps --copy --add-missing
>  ./configure
>  $MAKE
>  
> +# Now add a new directory.
> +#
>  # We shouldn't need to $sleep here: configure ensures that files
>  # generated by it are newer than configure.  Thus, even if
>  # Makefile.in is newer than configure but the updated Makefile.am
>  # below has the same timestamp as Makefile.in, the latter should
>  # be rebuilt due to its dependency on configure.in.
>  
> -# Now add a new directory.
> -cat > configure.in << 'END'
> -AC_INIT(maude, 1.0)
> -AM_INIT_AUTOMAKE
> -AC_PROG_CC
> -AM_PROG_CC_C_O
> -AC_CONFIG_FILES(Makefile maude/Makefile)
> -m4_include([confile.m4])
> -AC_OUTPUT
> -END
> +mkdir maude
>  
> -: > confile.m4
> +# Update confiles.m4 *before* updating sub/Makefile.am.
> +# (subdir8.test do it in the other way: it updates confiles.m4
> +# after Makefile.am)
> +cat >> confiles.m4 << 'END'
> +AC_CONFIG_FILES([maude/Makefile])
> +END
>  
> -mkdir maude
>  cat > maude/Makefile.am << 'END'
>  include_HEADERS = foo.h
>  END
>  
> -: > maude/foo.h
> -
>  echo 'SUBDIRS = maude' >> Makefile.am
>  
> +: > maude/foo.h
> +

Is there a reason for this reordering?

>  # We want a simple rebuild to create maude/Makefile automatically.
>  $MAKE
>  test -f maude/Makefile
>  
>  # Add yet another directory
>  mkdir maude2
> -echo 'AC_CONFIG_FILES([maude2/Makefile])AC_SUBST([GREPME])' > confile.m4
> +cat >> confiles.m4 << 'END'
> +AC_CONFIG_FILES([maude2/Makefile])
> +AC_SUBST([GREPME])
> +END
>  : > maude2/Makefile.am
>  echo 'SUBDIRS += maude2' >> Makefile.am

>  # We want a simple rebuild to create maude2/Makefile and update
>  # all other Makefiles automatically.
>  $MAKE
> -grep GREPME Makefile
> -grep GREPME maude/Makefile
> -grep GREPME maude2/Makefile
> +$FGREP GREPME Makefile
> +$FGREP GREPME maude/Makefile
> +$FGREP GREPME maude2/Makefile

This should not be necessary either.

> +:

> --- a/tests/subdir6.test
> +++ b/tests/subdir6.test
> @@ -1,5 +1,5 @@
>  #! /bin/sh
> -# Copyright (C) 2002  Free Software Foundation, Inc.
> +# Copyright (C) 2002, 2010 Free Software Foundation, Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -38,4 +38,5 @@ $MAKE clean
>  test -f sub/stamp-h1
>  $MAKE distclean
>  test -f sub/stamp-h1 && Exit 1
> +
>  :

tsk.  ;-)

[ subdir8.test ]
> @@ -81,8 +86,8 @@ echo 'SUBDIRS = maude' >> sub/Makefile.am
>  mkdir maude
>  : > maude/Makefile.am
>  
> -# Update confile.m4 *after* updating sub/Makefile.am.
> -# (subdir5.test do it in the other way: it updates configure.in
> +# Update confiles.m4 *after* updating sub/Makefile.am.
> +# (subdir5.test do it in the other way: it updates confiles.m4
>  # before Makefile.am)

This is not needed with above comment addressed.

>  echo 'AC_CONFIG_FILES([maude/Makefile sub/maude/Makefile])' >> confiles.m4
>  
> @@ -98,7 +103,9 @@ test -f sub/maude/Makefile
>  # the top-level directory.
>  echo 'AC_DEFUN([MORE_DEFS], [AC_SUBST([GREPME])])' > m4/moredefs.m4
>  $MAKE
> -grep GREPME Makefile
> -grep GREPME maude/Makefile
> -grep GREPME sub/Makefile
> -grep GREPME sub/maude/Makefile
> +$FGREP GREPME Makefile
> +$FGREP GREPME maude/Makefile
> +$FGREP GREPME sub/Makefile
> +$FGREP GREPME sub/maude/Makefile
> +
> +:

Thanks,
Ralf