Re: Avoid certain spurious `testsuite' rebuilds

[Noah Misch]

Hi Ralf,

On Sat, Apr 01, 2006 at 03:04:00PM +0200, Ralf Wildenhues wrote:
> * Noah Misch wrote on Fri, Mar 31, 2006 at 06:09:06PM CEST:

> > +stamp_tgat = $(srcdir)/stamp-tgat

> > +## Rebuild TESTSUITE_GENERATED_AT when their sources change.
> > +$(stamp_tgat): $(stamp_acm4)
> > +   test -f updated && mv -f updated $@; :
> 
> `updated' and `$(stamp_tgat)' aka `$(srcdir)/stamp-tgat' may not be on
> the same mount point.

Yes; good catch.  I moved `updated' into the source directory.

> > +$(stamp_acm4): mktests.sh $(AUTOCONF_FILES)
> > +   touch updated
> 
> If you want to be compatible to old BSDs, avoid touch for creating files
> or updating time stamps, see autoconf.texi.  I don't know if we can
> ignore this in practice, though.

>From that description in the manual, I take that one should not use `touch' to
update the mtime of an existing, empty file.  Using `touch' to update the mtime
of an existing file >0 bytes long is portable, as is using `touch' to create a
new, empty file with mtime = time().  Do you agree?

> > +   { ($(do_update)) && \
> > +       if test -f $(stamp_tgat); then rm -f updated; fi; } \
> > +     || test $$? -eq 55
> > +   touch $@
> 
> Likewise.

In light of the above analysis, I changed this.  The other use in this Makefile
should be safe, but I did change it, lest a user create `updated'.

> [ mktests ]
> > -    rm -f ac$base.tat ac$base.at
> > -    touch ac$base.at
> > +    rm -f ac$base.tat
> > +    if test -s ac$base.at; then
> > +   rm -f ac$base.at
> > +   touch ac$base.at
> 
> I guess the same applies here.

I left this as-is, since we just deleted the file we touch.


Thanks for the review.  Here is an updated patch (same ChangeLog):

diff -urp -X dontdiff ac-clean/tests/.cvsignore 
ac-test_ifchanged/tests/.cvsignore
--- ac-clean/tests/.cvsignore   2004-12-10 02:08:52.000000000 -0500
+++ ac-test_ifchanged/tests/.cvsignore  2006-04-01 12:52:10.000000000 -0500
@@ -14,4 +14,6 @@ autoreconf
 autom4te
 autoheader
 autoconf
+stamp-tgat
+stamp-acm4
 wrapper.in
diff -urp -X dontdiff ac-clean/tests/Makefile.am 
ac-test_ifchanged/tests/Makefile.am
--- ac-clean/tests/Makefile.am  2006-03-06 16:13:22.000000000 -0500
+++ ac-test_ifchanged/tests/Makefile.am 2006-04-01 13:00:59.000000000 -0500
@@ -1,7 +1,7 @@
 ## Process this file with automake to create Makefile.in. -*-Makefile-*-
 
 ## Makefile for Autoconf testsuite.
-## Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005
+## Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006
 ## Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
@@ -79,6 +79,10 @@ $(wrappers): wrapper.in
 ## Test suite.  ##
 ## ------------ ##
 
+stamp_tgat = $(srcdir)/stamp-tgat
+stamp_acm4 = $(srcdir)/stamp-acm4
+EXTRA_DIST += $(stamp_tgat) $(stamp_acm4)
+
 TESTSUITE_GENERATED_AT = \
        $(srcdir)/aclang.at \
        $(srcdir)/acc.at \
@@ -112,7 +116,8 @@ TESTSUITE = ./testsuite
 AUTOTEST = ./autom4te --language=autotest
 $(TESTSUITE): $(srcdir)/package.m4 \
              local.at \
-             $(TESTSUITE_AT) \
+             $(TESTSUITE_HAND_AT) \
+             $(stamp_tgat) $(TESTSUITE_GENERATED_AT) \
              $(autotest_m4f_dependencies)
        cd $(top_builddir)/lib/autotest && $(MAKE) $(AM_MAKEFLAGS) autotest.m4f
        $(AUTOTEST) -I $(srcdir) suite.at -o address@hidden
@@ -161,9 +166,54 @@ AUTOCONF_FILES = $(autoconfdir)/general.
                 $(autoconfdir)/types.m4 \
                 $(autoconfdir)/programs.m4
 
-$(TESTSUITE_GENERATED_AT): mktests.sh $(AUTOCONF_FILES)
-       cd $(srcdir) && ./mktests.sh \
-         `echo " "$(AUTOCONF_FILES) | sed 's, [^ ]*/, ../lib/autoconf/,g'`
+# In this discussion, TGAT is short for $(TESTSUITE_GENERATED_AT) and ACM4 for
+# $(AUTOCONF_FILES).  A colon represents a Make dependency from left to right.
+#
+# testsuite : TGAT : ACM4 is natural, but it needlessly rebuilds `testsuite'
+# much of the time.  Only ACM4 changes that introduce a new public macro
+# actually affect TGAT.
+#
+# As a second try, one can keep the same dependencies but only touch the TGAT
+# that change.  One can then edit and test ACM4 without rebuilding testsuite,
+# but as soon as a TGAT is older than an ACM4, every `make check' will run
+# `mktests.sh' to possibly update TGAT.  That cost adds up.
+#
+# In this implementation, we use testsuite : stamp-tgat : stamp-acm4 : ACM4.
+# stamp-acm4 stands for the greatest mtime among ACM4 and stamp-tgat for the
+# greatest mtime among TGAT.  The stamp-tgat rules always run, but they simply
+# check a sentinel that the stamp-acm4 rules set when a TGAT changes.
+#
+# `testsuite' separately depends on TGAT, and TGAT depends on nothing but has
+# the same rebuild rule as stamp-acm4.  That serves to rebuild members of TGAT
+# if the user deletes one of them.
+#
+# This timing diagram shows the implementation in action.  Natural numbers
+# represent abstract time stamps.
+# COMMAND                    TIMESTAMP OF: testsuite stamp-tgat stamp-acm4 ACM4
+# $ rm testsuite stamp-tgat stamp-acm4  |                                     0
+# $ make check                          |  3          1           2           0
+# $ touch lib/autoconf/c.m4; make check |  3          1           5           4
+# $ rm acc.at; make check               |  8          6           7           4
+
+# Command to update TESTSUITE_GENERATED_AT from AUTOCONF_FILEs.  It should not
+# touch files that need no change.  It should return zero if it changed nothing
+# and 55 if it changed something.  Other exit values signify errors.
+do_update = cd $(srcdir) && ./mktests.sh \
+           `echo " "$(AUTOCONF_FILES) | sed 's, [^ ]*/, ../lib/autoconf/,g'`
+
+## Rebuild TESTSUITE_GENERATED_AT when their sources change.
+$(stamp_tgat): $(stamp_acm4)
+       test -f $(srcdir)/updated && mv -f $(srcdir)/updated $@; :
+$(stamp_acm4): mktests.sh $(AUTOCONF_FILES)
+       echo 'This file preserves a timestamp.' >$(srcdir)/updated
+       { ($(do_update)) && \
+           if test -f $(stamp_tgat); then rm -f $(srcdir)/updated; fi; } \
+         || test $$? -eq 55
+       echo 'This file preserves a timestamp.' >$@
+
+## Rebuild TESTSUITE_GENERATED_AT when the developer deletes one of them.
+$(TESTSUITE_GENERATED_AT):
+       $(do_update) || :
 
 
 ## maintainer-check ##
diff -urp -X dontdiff ac-clean/tests/mktests.sh 
ac-test_ifchanged/tests/mktests.sh
--- ac-clean/tests/mktests.sh   2006-04-01 10:57:02.000000000 -0500
+++ ac-test_ifchanged/tests/mktests.sh  2006-04-01 12:52:10.000000000 -0500
@@ -208,6 +208,7 @@ fi
 ## Creating the test files.  ##
 ## ------------------------- ##
 
+exit_val=0
 for file in $src
 do
   base=`echo "$file" | sed 's,.*[\\/],,;s/\..*//'`
@@ -262,18 +263,27 @@ MK_EOF
   # preserves the old version of the file.  If there is nothing to
   # check, output /rien du tout/[1].
   if grep AT_CHECK ac$base.tat >/dev/null 2>&1; then
-    mv -f ac$base.tat ac$base.at
-    # Help people not to update these files by hand.
-    chmod a-w ac$base.at
+    if diff ac$base.at ac$base.tat >/dev/null 2>&1; then
+       rm -f ac$base.tat
+    else
+       mv -f ac$base.tat ac$base.at
+       # Help people not to update these files by hand.
+       chmod a-w ac$base.at
+       exit_val=55
+    fi
   else
-    rm -f ac$base.tat ac$base.at
-    touch ac$base.at
+    rm -f ac$base.tat
+    if test -s ac$base.at; then
+       rm -f ac$base.at
+       touch ac$base.at
+       exit_val=55
+    fi
   fi
 done
 
 rm -f acdefuns audefuns requires
 
 trap 0
-exit 0
+exit $exit_val
 
 # [1] En franc,ais dans le texte.