Re: [GNU Autoconf 2.61b] testsuite: 156 158 failed

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> 
> 
> According to Ralf Wildenhues on 9/30/2007 11:53 AM:
> > Hello,
> 
> Hi Ralf,
> 
> > 
> > one of the recent commits broke a couple of tests in the testsuite,
> > namely the AC_F77_WRAPPERS and AC_FC_WRAPPERS tests:
> > 
> > | 156. acfortran.at:14: testing ...
> > | +/usr/bin/m4:configure.ac:5: ERROR: end of file in string
> 
> Thanks for the heads-up. I'll look into it.

I'm posting the following as a tutorial in how git can be useful, with my 
comments following #.  My first suspect was commit b2dda0a, since it touches so 
much low-level stuff:
 http://git.sv.gnu.org/gitweb/?p=autoconf.git;a=commit;h=b2dda0a

So to test the theory:
# create my temporary regression branch just before the suspect commit
git checkout -b fortran-regression b2dda0a^
# check the tests
make check TESTSUITEFLAGS=155-158
... # the tests pass

# update the branch to include the suspect commit
git reset --hard b2dda0a
make check TESTSUITEFLAGS=155-158
... # bingo, but the patch touched so much, it would be nice to break it up
# undo the commit, leaving all the commit's edits in my directory
git reset HEAD^
# start the interactive adder, which lets me subdivide pending edits
git add -i
           staged     unstaged path
  1:    unchanged       +15/-0 ChangeLog
  2:    unchanged       +12/-7 lib/autoconf/general.m4
  3:    unchanged       +21/-7 lib/m4sugar/m4sh.m4
  4:    unchanged      +32/-12 lib/m4sugar/m4sugar.m4

*** Commands ***
  1: status       2: update       3: revert       4: add untracked
  5: patch        6: diff         7: quit         8: help
What now> 5     # review each hunk of uncommitted change
           staged     unstaged path
  1:    unchanged       +15/-0 ChangeLog
  2:    unchanged       +12/-7 lib/autoconf/general.m4
  3:    unchanged       +21/-7 lib/m4sugar/m4sh.m4
  4:    unchanged      +32/-12 lib/m4sugar/m4sugar.m4


Patch update> 2    # I'll start with just the autoconf changes
@@ -1950,14 +1950,15 @@ rm -f confcache[]dnl
...
Stage this hunk [y/n/a/d/j/J/s/?]? a   # add all hunks from this file

*** Commands ***
  1: status       2: update       3: revert       4: add untracked
  5: patch        6: diff         7: quit         8: help
What now> 7   # enough for now
Bye.


# commit the intermediate status (ie. I've now split commit b2dda0a)
git commit -m 'split part 1 - autoconf changes'
# save off the unapplied parts of the commit
git stash save fortran
# test the partial patch in isolation
make check TESTSUITEFLAGS=155-158
... # the tests failed, so we've narrowed it down!


# let's repeat the splitting process; undo the last commit
git reset HEAD^
git add -i
           staged     unstaged path
  1:    unchanged       +12/-7 lib/autoconf/general.m4

*** Commands ***
  1: status       2: update       3: revert       4: add untracked
  5: patch        6: diff         7: quit         8: help
What now> 5
           staged     unstaged path
  1:    unchanged       +12/-7 lib/autoconf/general.m4
Patch update> 1
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index a0f473a..7835f24 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -1950,14 +1950,15 @@ rm -f confcache[]dnl
 # The name of shell var CACHE-ID must contain `_cv_' in order to get saved.
 # Should be dnl'ed.  Try to catch common mistakes.
 m4_defun([AC_CACHE_VAL],
-[AS_LITERAL_IF([$1], [m4_bmatch(m4_quote($1), [_cv_], [],
-                               [AC_DIAGNOSE([syntax],
+[AS_LITERAL_IF([$1], [m4_if(m4_index(m4_quote($1), [_cv_]), [-1],
+                           [AC_DIAGNOSE([syntax],
 [$0($1, ...): suspicious cache-id, must contain _cv_ to be cached])])])dnl
-m4_bmatch([$2], [AC_DEFINE],
-          [AC_DIAGNOSE([syntax],
+m4_if(m4_index([$2], [AC_DEFINE]), [-1], [],
+      [AC_DIAGNOSE([syntax],
 [$0($1, ...): suspicious presence of an AC_DEFINE in the second argument, ]dnl
-[where no actions should be taken])],
-          [AC_SUBST], [AC_DIAGNOSE([syntax],
+[where no actions should be taken])])dnl
+m4_if(m4_index([$2], [AC_SUBST]), [-1], [],
+      [AC_DIAGNOSE([syntax],
 [$0($1, ...): suspicious presence of an AC_SUBST in the second argument, ]dnl
 [where no actions should be taken])])dnl
 AS_VAR_SET_IF([$1],
Stage this hunk [y/n/a/d/j/J/s/?]? y   # keep hunk 1...


@@ -2006,8 +2007,12 @@ m4_bmatch([$1], ^m4_defn([m4_re_word])$, [],
 # ---------------------------
 # This macro is a wrapper around AC_DEFINE_TRACE_LITERAL which filters
 # out non literal symbols.
+#
+# m4_index is roughly 5 to 8 times faster than m4_bpatsubst.
 m4_define([AC_DEFINE_TRACE],
-[AS_LITERAL_IF([$1], [AC_DEFINE_TRACE_LITERAL(m4_bpatsubst([[$1]], [(.*)]))])])
+[AS_LITERAL_IF([$1], [AC_DEFINE_TRACE_LITERAL(
+  m4_if(m4_index([[$1]], [(]), [-1], [[$1]],
+       [m4_substr([[$1]], [0], m4_index([[$1]], [(]))]))])])
 
 
 # AC_DEFINE(VARIABLE, [VALUE], [DESCRIPTION])
Stage this hunk [y/n/a/d/K/s/?]? n   # but not hunk 2...

*** Commands ***
  1: status       2: update       3: revert       4: add untracked
  5: patch        6: diff         7: quit         8: help
What now> 7
Bye.

# save off the partial patch again
git commit -m 'split part 2'
git stash
# retry the testsuite
make check TESTSUITEFLAGS=155-158
# tests successful; hunk 1 was not the problem, so it must be hunk 2
# reapply hunk 2, to further debug it
git stash apply address@hidden
git diff
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 2a31904..7835f24 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -2007,8 +2007,12 @@ m4_bmatch([$1], ^m4_defn([m4_re_word])$, [],
 # ---------------------------
 # This macro is a wrapper around AC_DEFINE_TRACE_LITERAL which filters
 # out non literal symbols.
+#
+# m4_index is roughly 5 to 8 times faster than m4_bpatsubst.
 m4_define([AC_DEFINE_TRACE],
-[AS_LITERAL_IF([$1], [AC_DEFINE_TRACE_LITERAL(m4_bpatsubst([[$1]], [(.*)]))])])
+[AS_LITERAL_IF([$1], [AC_DEFINE_TRACE_LITERAL(
+  m4_if(m4_index([[$1]], [(]), [-1], [[$1]],
+     [m4_substr([[$1]], [0], m4_index([[$1]], [(]))]))])])
 
Aha - now I see the bug.  m4_substr chops off the tail end of the string, 
INCLUDING the closing quote.  In other words, my attempt to speed up 
m4_bpatsubst([[$1]], [(.*)]) is broken if you do
  AC_DEFINE([MACRO(with, args)])
I still think it is possible to elide the arguments portion of a macro without 
regex, which was the (failed) intent of the above change.  So patch coming up 
soon...

Meanwhile, now that I identified the problem spot, I can throw away all my 
searching:
# no longer need what I stashed
git stash clear
# no longer need the fortran-regression branch
git checkout -f master
git branch -D fortran-regression

-- 
Eric Blake