the cost of due diligence

[Jim Meyering]

I've wanted to make a grep snapshot for a while, but each time I'd
find some time, I'd encounter some new blocker. A few days ago it
was leaks. Thanks to Paul Eggert for fixing those promptly. Most
recently, I noticed that grep's autoconf-head-built configure script
had a syntax error. That led me to discover that autoconf had numerous
failing tests.

There were a couple issues, including one that was introduced
nearly ten years ago. It was initially harmless due to a chance
implementation detail. The very recent implementation change, from
if/else/fi to case...esac, evoked failure in any configure script
that uses AC_FUNC_ACLOCAL, but there was no test coverage of that
macro. One could argue that this lack of testing was another issue.

I've pushed these:

>From 615e34cbe3e74dad5aef4b4e0525dea5cf72220d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@fb.com>
Date: Sun, 26 Jun 2022 15:37:51 -0700
Subject: [PATCH 1/2] tests: avoid test failures due to new EGREP_TRADITIONAL

Some tests require that filtered pre- and post-test
environments be identical. The filtering removes assignments
to known (set-by-test) envvar names, like GREP, SED, YACC.
* tests/local.at (_AT_CHECK_ENV): Also filter out assignments to
EGREP_TRADITIONAL, added in v2.72a-35-g0c762675.
---
 tests/local.at | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/local.at b/tests/local.at
index 14824145..4c2c9903 100644
--- a/tests/local.at
+++ b/tests/local.at
@@ -400,6 +400,7 @@ if test -f state-env.before && test -f state-env.after; then
       [FC(_DUMMY_MAIN|FLAGS|LIBS|FLAGS_[fF]|_MODEXT|_MODINC|_MODOUT|_DEFINE)?],
       [ALLOCA|GETLOADAVG_LIBS|KMEM_GROUP|NEED_SETGID|POW_LIB],
       [AWK|LEX|LEXLIB|LEX_OUTPUT_ROOT|LN_S|M4|MKDIR_P|AR|RANLIB|SET_MAKE|YACC],
+      [EGREP_TRADITIONAL],
       [GREP|[EF]GREP|SED],
       [[_@]|.[*@%:@?$].],
       [argv|ARGC|LINENO|BASH_ARGC|BASH_ARGV|OLDPWD|PIPESTATUS|RANDOM],
-- 
2.35.1.677.gabf474a5dd


>From 8ed183db6d99a33f5c6943b1d2d7fba5d260290b Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@fb.com>
Date: Sun, 26 Jun 2022 22:38:04 -0700
Subject: [PATCH 2/2] AC_FUNC_ALLOCA: fix a misplaced (now fatal) closing "fi"

"autoconf quoting is a pain"
* lib/autoconf/functions.m4 (AC_FUNC_ALLOCA): Its AC_CACHE_CHECK
contains an if/else block, but the closing "fi" lay just after its ")".
Before, this error didn't trigger any failure because the if/else code was
in the "else" block of AC_CACHE_CHECK's AS_IF invocation and AS_IF was also
implemented using an if..fi block. So the ostensibly-"outer" "fi" provided
by AS_IF matched the inner "if/else", and that stray-after-end "fi" served
to close the AS_IF block. However, when AS_IF switched from if..fi to
case..esac, this became a nesting error: no matching "fi".
Initially-harmless error introduced by v2.69-52-gfd29dbd7 in 2012.
Error exposed by v2.72a-30-gc8d6d6eb.
* tests/mktests.pl (scan_m4_files): Do not elide direct test
of AC_FUNC_ALLOCA.

FTR, here's the list of macros whose direct tests were being suppressed:
AC_CANONICAL_BUILD AC_CANONICAL_HOST AC_CANONICAL_TARGET
AC_CHECK_INCLUDES_DEFAULT AC_DISABLE_OPTION_CHECKING
AC_ERLANG_NEED_ERL AC_ERLANG_NEED_ERLC AC_ERLANG_SUBST_ERTS_VER
AC_ERLANG_SUBST_INSTALL_LIB_DIR AC_ERLANG_SUBST_LIB_DIR AC_F77_DUMMY_MAIN
AC_F77_LIBRARY_LDFLAGS AC_FC_DUMMY_MAIN AC_FC_LIBRARY_LDFLAGS
AC_FUNC_ALLOCA AC_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK AC_GNU_SOURCE
AC_HEADER_DIRENT AC_HEADER_SYS_WAIT AC_PATH_X AC_PROG_CPP AC_PROG_CXX
AC_PROG_CXXCPP AC_PROG_F77 AC_PROG_FC AC_PROG_GO AC_PROG_GREP
AC_PROG_OBJC AC_PROG_OBJCPP AC_PROG_OBJCXX AC_PROG_OBJCXXCPP
AC_STRUCT_TM AC_TYPE_GETGROUPS AC_TYPE_LONG_LONG_INT AC_TYPE_MBSTATE_T
AC_TYPE_PID_T AC_TYPE_SIZE_T AC_TYPE_UID_T AC_TYPE_UNSIGNED_LONG_LONG_INT
AC_USE_SYSTEM_EXTENSIONS
---
 lib/autoconf/functions.m4 | 4 +++-
 tests/mktests.pl          | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index e7c54846..51aeb0b9 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -448,8 +448,10 @@ void *alloca (size_t);
 ]],                               [[char *p = (char *) alloca (1);
                                    if (p) return 0;]])],
                [ac_cv_func_alloca_works=yes],
-               [ac_cv_func_alloca_works=no])])
+               [ac_cv_func_alloca_works=no]
+               )
 fi
+])

 if test $ac_cv_func_alloca_works = yes; then
   AC_DEFINE(HAVE_ALLOCA, 1,
diff --git a/tests/mktests.pl b/tests/mktests.pl
index ab47eaf1..b6842682 100644
--- a/tests/mktests.pl
+++ b/tests/mktests.pl
@@ -290,6 +290,14 @@ sub scan_m4_files
       push @macros_to_test, [ $file, \@ac_macros, \@au_macros ];
     }

+  # Do **NOT** filter out AC_FUNC_ALLOCA. Filtering it out
+  # ended up eliding a direct test of AC_FUNC_ALLOCA which
+  # would have exposed a bug, while no required use does so.
+  # Clearing this hash entirely would currently enable direct tests
+  # of 38 macros, but would require designating each that must be
+  # skipped when cross-compiling.
+  delete $required_macros{AC_FUNC_ALLOCA};
+
   # Filter out macros that are AC_REQUIREd by some other macro;
   # it's not necessary to test them directly.
   my @pruned_macros_to_test;
-- 
2.35.1.677.gabf474a5dd