Re: [PATCH] {yacc-work} configure: look for a lex program to be used by the testsuite

[Ralf Wildenhues]

* Stefano Lattarini wrote on Fri, Jan 28, 2011 at 11:19:12PM CET:
> This will allow the testcases requiring a 'lex' program to run also
> with vendor/legacy lex implementations, not only with 'flex'.

> * configure.ac: Look for a lex program, using AC_CHECK_PROGS.
> * tests/defs.in: New required entry 'lex'.
> ($LEX): Let the user override the lex program to be used by the
> testsuite.
> * tests/cond35.test ($required): Require 'lex', not 'flex'.
> * tests/cond36.test: Likewise.
> * tests/lexv3.test: Likewise.
> * tests/lexv3.test: Likewise.
> * tests/silent-lex-gcc.test: Likewise.
> * tests/silent-lex-generic.test: Likewise.
> * tests/silent-many-gcc.test: Likewise.
> * tests/silent-many-generic.test:likewise.
> * tests/lexvpath.test: Likewise, and fix typo in comments.

> OK for the 'yacc-work' branch?  (Branch which, at this point should
> probably be renamed to 'lex-yacc-work' ...)

OK with nits addressed.  The branch name is fine as it is,
no need to strive for branch name perfection.

> --- a/configure.ac
> +++ b/configure.ac
> @@ -103,6 +103,16 @@ AC_CHECK_PROG([TEX], [tex], [tex])
>  #     configure help screen.
>  AC_CHECK_PROGS([YACC], [yacc byacc 'bison -y'], [false])
>  
> +# The test suite will skip some tests if no yacc program is available.
> +# We don't use AC_PROG_LEX because:
> +#  1. we don't want flex to be preferred to system lex;
> +#  2. we don't want $LEX to be defined to ':' by default;
> +#  3. we prefer not to have the LEXLIB and LEX_OUTPUT_ROOT variables
> +#     to be calculated and/or AC_SUBST'd;
> +#  4. we prefer that the LEX variable is not reported in the
> +#     configure help screen.

You need to withstand copy and paste.  This comment is copied and pasted
from a few lines above.  It is erroneous (search for 'yacc') and immoral
(towards a reader of the code) to do so.  Please find a formulation and
one comment that applies to both AC_CHECK_PROGS lines that can then come
right after one another.

Resist copy and paste!

> +AC_CHECK_PROGS([LEX], [lex flex], [false])
> +
>  # Generate man pages.
>  AM_MISSING_PROG([HELP2MAN], [help2man])

> --- a/tests/defs.in
> +++ b/tests/defs.in
> @@ -63,6 +63,7 @@ export SHELL
>  # User can override various tools used.
>  test -z "$PERL" && PERL='@PERL@'
>  test -z "$YACC" && YACC='@YACC@'
> +test -z "$LEX" && LEX='@LEX@'
>  test -z "$MAKE" && MAKE=make
>  test -z "$AUTOCONF" && AUTOCONF="@am_AUTOCONF@"
>  test -z "$AUTOHEADER" && AUTOHEADER="@am_AUTOHEADER@"
> @@ -210,6 +211,17 @@ do
>        echo "$me: running texi2dvi -o /dev/null --version"
>        ( texi2dvi -o /dev/null --version ) || exit 77
>        ;;
> +    lex)
> +      if test x"$LEX" = x"false"; then
> +        # No lex program was found at configure time, or the user has
> +        # explicitly told he doesn't want a lex program to be used.
> +        echo "$me: \$LEX is \"false\", skipping test" >&2

That error message could be more helpful:
  $me: no working \$LEX found at configure time, or disabled

obviating the need for the comment too.

> +        exit 77
> +      else
> +        # Make LEX available to configure by exporting it.

Superfluous comment (the code already shows what you do).

> +        export LEX
> +      fi
> +      ;;
>      yacc)
>        if test x"$YACC" = x"false"; then
>          # No yacc program was found at configure time, or the user has

I haven't checked the individual tests that you changed, assuming that
you tested them with a non-flex lex program on some system.

Thanks,
Ralf