Re: [PATCH 1/3] Refactor and extend tests on cscope functionality.

[Ralf Wildenhues]

Hi Stefano,

* Stefano Lattarini wrote on Sat, Jun 19, 2010 at 05:04:22PM CEST:
> * tests/cscope.test: Improve and normalize generation and names
> of source files.
> Do not use hackish, belated SKIPs anymore.
> Slighty extend existing tests.
> (configure.in): Do not call macro `AM_PROG_GCJ' anymore.
> ($required): Set properly.
> ($EMACS): Intialize and export, for use by configure.
> * tests/cscope2.test: Add 'cscope' to $required.  Check that
> the 'cscope' target do not generate any file if there are no
> source files.
> * tests/cscope3.test: Add 'cscope' to $required.  Check that
> the recursive 'cscope' target do not create a 'cscope.out' file
> if there no source files.  Add a proper 'FIXME' comment.
> * tests/cscope4.test: New test.
> * tests/cscope5.test: Likewise.
> * tests/cscope6.test: Likewise.
> * tests/cscope7.test: Likewise.
> * tests/cscopec.test: Likewise.
> * tests/cscopecxx.test: Likewise.
> * tests/cscope8.test: New test, similar to cscope.test, but adds
> java, lex, yacc and fortran77 sources to the mix.
> * tests/Makefile.am (TESTS): Updated.

> --- a/tests/cscope3.test
> +++ b/tests/cscope3.test

> @@ -27,9 +27,10 @@ cat >> configure.in << 'END'
>  AC_CONFIG_FILES([sub/Makefile])
>  AC_OUTPUT
>  END
> +
> +echo 'SUBDIRS = sub' > Makefile.am
>  mkdir sub
> -echo 'SUBDIRS = sub' >Makefile.am
> -: >sub/Makefile.am
> +: > sub/Makefile.am
> 
>  $ACLOCAL
>  $AUTOCONF
> @@ -37,5 +38,10 @@ $AUTOMAKE
> 
>  ./configure
>  $MAKE cscope
> +# FIXME (in automake.in): We'd like cscope.files not to be created, as
> +# there are no source files in `.' nor in `sub'.

Why is that?  Why would we not want the file cscope.files to be created?

>    Fow nor, we are
> +# content with ensuring that it's empty.
> +test ! -s cscope.files
> +test ! -f cscope.out



> --- /dev/null
> +++ b/tests/cscope6.test

> +# Basic checks on cscope functionality.
> +# This test does *not* require the cscope program, nor any compiler.
> +# Please keep this test in sync with the sister test `cscope7.test'.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cscope=false # used below, when postprocessing Makefile.in
> +
> +cat >> configure.in << 'END'
> +AC_CONFIG_FILES([sub/Makefile])
> +AC_PROG_CC
> +AC_PROG_CXX
> +AC_PROG_LEX
> +AC_PROG_YACC
> +AC_PROG_F77
> +AC_PROG_FC
> +AM_PROG_GCJ
> +AM_PATH_LISPDIR
> +AC_OUTPUT
> +END
> +
> +mkdir sub sub/subsub
> +
> +cat > Makefile.am <<'END'
> +SUBDIRS = sub
> +dist_lisp_LISP = zardoz.el
> +bin_PROGRAMS = foo
> +foo_SOURCES = ## will be extended later
> +END
> +
> +cat > sub/Makefile.am <<'END'
> +bin_PROGRAMS = bar
> +bar_SOURCES = subsub/dist.c
> +nodist_bar_SOURCES = subsub/gen.c
> +subsub/gen.c:; echo '/*GeNeRaTeD*/' >$@
> +END
> +
> +: > fileslist-exp
> +
> +for f in \
> +  foo01.c \
> +  foo02.cc \
> +  foo03.cpp \
> +  foo04.cxx \
> +  foo05.c++ \
> +  foo06.y \
> +  foo07.ypp \
> +  foo08.yy \
> +  foo09.l \
> +  foo10.l++ \
> +  foo11.lxx \
> +  foo12.f \
> +  foo13.f90 \
> +  quux.java \
> +; do
> +  touch $f # cannot use ': >', as Solaris Sh "optimize" it away
> +  echo ./$f >> fileslist-exp
> +  echo foo_SOURCES += $f >> Makefile.am
> +done
> +
> +cat >> fileslist-exp <<'END'
> +./zardoz.el
> +sub/subsub/dist.c
> +sub/subsub/gen.c
> +END
> +
> +: > zardoz.el
> +: > sub/subsub/dist.c
> +
> +$ACLOCAL
> +$AUTOMAKE --add-missing
> +
> +## We go to great lengths to avoid running configure.  This is not
> +## for a mistaken desire of "optimization".  Rather, we want to be
> +## able to run this test even if some (or all!) of configure-time
> +## requirements (various compilers, an emacs installation, etc.)
> +## are not present on the system.  We think that this possibility
> +## is well worth the additional complications below.
> +
> +# Required to prevent make to try to rerun autotools and configure.
> +touch configure config.status
> +chmod a+x configure config.status
> +
> +for d in . sub; do
> +  LC_ALL=C sed \
> +      -e '/^ address@hidden@ *$/d' \

Removing SET_MAKE is not going to work with a package using recursive
makefiles and some ancient makes.  I don't know whether any of them are
relevant any more though.

> +      -e '/address@hidden@/d' \
> +      -e '/address@hidden@/d' \
> +      -e "s|^CSCOPE *=.*|CSCOPE = $cscope|" \
> +      -e 's|^srcdir *=.*|srcdir = .|' \
> +      -e 's|^builddir *=.*|builddir = .|' \
> +      -e 's|^VPATH *=.*|VPATH = .|' \
> +      -e 's|^SHELL *=.*|SHELL = /bin/sh|' \

This might not work everywhere, /bin/sh might not exist (nevermind
the #!/bin/sh lines, for them to work it needs system support, not
necessarily the presence of a /bin/sh file).

> +      -e 's|@PATH_SEPARATOR@|:|g' \

This is not going to work portably.

> +    <$d/Makefile.in >$d/Makefile.tmp
> +      # FIXME above: we should use the $PATH_SEPARATOR found
> +      # at configure time...

How about, instead, running configure with all the required tools
pre-set?  That way, all you need is a C compiler.

> +done
> +sed -e 's|^top_srcdir *=.*$|top_srcdir = .|' \
> +    -e 's|^top_builddir *=.*$|top_builddir = .|' \
> +  <Makefile.tmp >Makefile
> +sed -e 's|^top_srcdir *=.*$|top_srcdir = ..|' \
> +    -e 's|^top_builddir *=.*$|top_builddir = ..|' \
> +  <sub/Makefile.tmp >sub/Makefile
> +rm -f Makefile.tmp sub/Makefile.tmp
> +
> +## Go with the tests.
> +
> +$MAKE -n cscope
> +$MAKE cscopelist
> +ls -l . sub sub/subsub # useful for debugging
> +cat cscope.files # must exist
> +test ! -f cscope.out
> +test ! -f sub/cscope.files
> +test ! -f sub/cscope.out
> +test -f sub/subsub/gen.c
> +$FGREP '/*GeNeRaTeD*/' sub/subsub/gen.c
> +
> +LC_ALL=C sort cscope.files >fileslist-got
> +
> +cat fileslist-exp
> +cat fileslist-got
> +diff fileslist-exp fileslist-got
> +
> +# Check that making 'cscope' twice do not inappropriately
> +# append extra text to 'cscope.files'.
> +cp -f cscope.files prevlist
> +CSCOPE=: $MAKE -e cscope
> +diff prevlist cscope.files
> +
> +$MAKE clean
> +test -f cscope.files
> +
> +$MAKE distclean
> +ls > lst
> +$FGREP 'cscope' lst && Exit 1
> +
> +:

> --- /dev/null
> +++ b/tests/cscope7.test

This test has similar issues as cscope6.test

> +# Basic checks on cscope functionality.
> +# This test does *not* require any compiler.
> +# Please keep this test in sync with the sister test `cscope6.test'.


> --- /dev/null
> +++ b/tests/cscopec.test

> +# Test for cscope functionality with C files.
> +# Please keep this in sync with the sister test 'cscopecxx.test'.
> +
> +required='gcc cscope'
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in <<'END'
> +AC_CONFIG_FILES([sub/Makefile])
> +AC_PROG_CC
> +AC_OUTPUT
> +END
> +
> +mkdir sub sub/subsub
> +
> +cat > Makefile.am <<'END'
> +SUBDIRS = sub
> +bin_PROGRAMS = foo
> +foo_SOURCES = foo.c main-alias.h
> +END
> +
> +cat > sub/Makefile.am <<'END'
> +bin_PROGRAMS = bar
> +bar_SOURCES = subsub/dist.c
> +nodist_bar_SOURCES = gen.c
> +gen.c:
> +     echo 'int generated_func () { return 0; }' > $@
> +CLEANFILES = gen.c
> +END
> +
> +cat > main-alias.h <<'END'
> +#define RUN int main (void)
> +END
> +
> +cat > foo.c <<'END'
> +#include "main-alias.h"
> +RUN
> +{
> +  return 0;
> +}
> +END
> +
> +cat > sub/subsub/dist.c <<'END'
> +int generated_func();
> +int main ()
> +{
> +  return generated_func();
> +}
> +END
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE --add-missing
> +
> +ocwd=`pwd`
> +
> +for d in parent abs_parent current; do
> +
> +  case $d in
> +    current)
> +      ./configure
> +      ;;
> +    parent)
> +      mkdir build
> +      cd build
> +      ../configure
> +      ;;
> +    abs_parent)
> +      mkdir build2
> +      cd build2
> +      "$ocwd/configure"
> +      ;;
> +    *)
> +      Exit 99 # sanity check
> +      ;;
> +  esac
> +
> +  $MAKE cscope 2>stderr || { cat stderr >&2; Exit 1; }
> +  cat stderr >&2
> +  grep 'cannot find file' stderr && Exit 1
> +  ls -l . sub
> +  cat cscope.files # must exist
> +  test -f cscope.out
> +  test ! -f sub/cscope.files
> +  test ! -f sub/subsub/cscope.files
> +
> +  case $d in
> +    current)
> +      cat <<'END'
> +./foo.c
> +./main-alias.h
> +sub/gen.c
> +sub/subsub/dist.c
> +END
> +      ;;
> +    parent)
> +      cat <<'END'
> +./../foo.c
> +./../main-alias.h
> +sub/../../sub/subsub/dist.c
> +sub/gen.c
> +END
> +      ;;
> +    abs_parent)
> +      cat <<END
> +$ocwd/foo.c
> +$ocwd/main-alias.h
> +$ocwd/sub/subsub/dist.c
> +sub/gen.c
> +END
> +      ;;
> +  esac | LC_ALL=C sort > fileslist-exp
> +
> +  LC_ALL=C sort cscope.files > fileslist-got
> +  cat fileslist-exp
> +  cat fileslist-got
> +  diff fileslist-exp fileslist-got

How come this doesn't fail with Solaris make expanding VPATH file names
in the makefile?

I think in general, we might expect a bit too much here.  make might
even simplify noncanonical paths such as sub/../../sub/... (see the
section on make issues in the Autoconf manual) e.g. on Tru64/OSF.
(I haven't tested that though.)

Similar with cscopecxx.test.

The rest of the large patch looks fairly good to me, and 2/3 is pretty
obvious too.

Thanks for working on this!
Ralf