RFC: fixing AC_CONFIG_HEADERS rules (PR/355)

[Alexandre Duret-Lutz]

This patch attempts to improve the rebuild rules for
AC_CONFIG_HEADERS, but there is one aspect of the patch I don't
like and I hope you can help me.

To Akim (with whom I've discussed another aspect of these
rules): this mail is completely unrelated to the stamp-h handling.

That patch does four things:

 - duplicate rebuild rules for AC_CONFIG_HEADERS in all 
   directories (this is for PR/355)
 - call autoheader only for the first AC_CONFIG_HEADERS
   (this matches the autoheader behavior)
 - have config.h.in dependent on all configure dependencies
   (i.e., all m4_includes)
 - ensure that all AC_CONFIG_HEADERS *inputs* are up to
   date before running a complete config.status.

The last point is the one that worries me.  Without it
you can get the following scenario:

 1. the user updates some configure dependency (let's say foo.m4)
    and run `make'
 2. our new rebuild rules detect that a foo.m4 has changed, 
    run aclocal, autoconf, automake, and then run ./config.status.  
    (Note that autoheader hasn't been called yet, so ./config.status
    outputs a config.h from an obsolete config.h.in.)
 3. Once Makefile has been regenerated, make continues, and
    discover that config.h is a dependency of the `all' rule.
    Because config.h depends on stamp-h1, stamp-h1 depends on
    config.hin, and config.h.in depends on aclocal.m4, make runs
    autoheader to rebuild config.h.in.

Now make ought to call ./config.status once again
to rebuild config.h from the new config.h.in, but if you have a
sufficiently fast box, steps 2 and 3 will occur within the
same second: the config.h generated from the outdated
config.h.in will have the same mtime as the new config.h.in.
Hence make will think that config.h is up to date. 

I thought of two solution for this:
  a. sleep before running autoheader
  b. ensure that config.h.in is up to date before running 
     ./config.status at step 2.

The problem with a. is that it's slow and the minimum sleep time
depends on the file system (the test suite need to sleep for as
much as 5 seconds on DJGPP).

Solution b. can be generalized to all AC_CONFIG_HEADERS inputs
(i.e., not limited to the autoheader-generated config.h.in).
The problem is how it can be implemented.  We are in the
rebuild rule for `Makefile', right before the
plain `./config.status' invocation, and we want all 
AC_CONFIG_HEADERS inputs (let's call them %CONFIG_H_DEPS%)
to be up to date.  We can't run
    $(MAKE) %CONFIG_H_DEPS%
because Makefile hasn't been updated yet and running $(MAKE)
recursively would just trigger this very same Makefile
rebuild rule we are in, before even considering %CONFIG_H_DEPS%.
Call this an infinite loop.

The only way I've found to update %CONFIG_H_DEPS% from
the Makefile rule is with the GNU Make `-W' flag:
    $(MAKE) -W Makefile %CONFIG_H_DEPS%
so that make doesn't try to update Makefile.

Can anybody think of another way of doing this.
I'm mildly annoyed by the use of `-W'.  Of course I don't
think it matters very much because these rebuild
rules are only triggered by GNU Make, but nonetheless
I'd prefer if there were a portable way to do it.

PS: with the two new tests in this patch, the test suite
    tops 500 tests.  (Any volunteer to run the garbage
    collector?  tests/install.test is a good starting point.)

2003-10-01  Alexandre Duret-Lutz  <address@hidden>

        For PR automake/355:
        * automake.in (handle_configure): Define %CONFIG_H_DEPS% while
        processing configure.am.  Define rebuild rules for headers
        in all Makefiles.
        * lib/am/configure.am (%MAKEFILE%): Ensure %CONFIG_H_DEPS%
        are up to date before running config.status.
        * lib/am/remake-hdr.am (%STAMP%): Use %CONFIG_H_DEPS% instead
        of (srcdir)/%CONFIG_HIN%.  Output the $(srcdir)/%CONFIG_HIN%
        rule only for the first header.
        * tests/autohdr3.test, tests/autohdr4.test: New files.
        * tests/Makefile.am (TESTS): Add autohdr3.test and autohdr4.test.
        * tests/config.test: Do not grep, run things to see if they work.

Index: NEWS
===================================================================
RCS file: /cvs/automake/automake/NEWS,v
retrieving revision 1.231
diff -u -r1.231 NEWS
--- NEWS        30 Sep 2003 19:05:53 -0000      1.231
+++ NEWS        1 Oct 2003 21:11:19 -0000
@@ -170,6 +170,8 @@
 
   - Do not assume that make files are called Makefile in cleaning rules.
 
+  - More accurate update of AC_CONFIG_HEADERS files (PR/355).
+
 * Miscellaneous
 
   - The Automake manual is now distributed under the terms of the GNU FDL.
Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1507
diff -u -r1.1507 automake.in
--- automake.in 30 Sep 2003 19:05:53 -0000      1.1507
+++ automake.in 1 Oct 2003 21:11:23 -0000
@@ -3375,6 +3375,16 @@
   define_pretty_variable ('am__configure_deps', TRUE, INTERNAL,
                          @configure_deps);
 
+
+  # The first config_header input is assumed to be config.h.in.
+  # (This is what autoheader assumes.)
+  my @config_h_deps = ();
+  for my $spec (@config_headers)
+    {
+      my ($dest, @ins) = split_config_file_spec ($spec);
+      push @config_h_deps, rewrite_inputs_into_dependencies (1, @ins);
+    }
+
   $output_rules .= file_contents
     ('configure',
      new Automake::Location,
@@ -3384,6 +3394,7 @@
      'MAKEFILE-IN'         => $rel_makefile_in,
      'MAKEFILE-IN-DEPS'    => "@include_stack",
      'MAKEFILE-AM'         => $rel_makefile_am,
+     'CONFIG_H_DEPS'      => "@config_h_deps",
      STRICTNESS            => global_option 'cygnus'
                                 ? 'cygnus' : $strictness_name,
      'USE-DEPS'            => global_option 'no-dependencies'
@@ -3408,78 +3419,70 @@
       my ($config_h_path, @ins) = split_config_file_spec ($spec);
       my $config_h_dir = dirname ($config_h_path);
 
-      # If the header is in the current directory we want to build
-      # the header here.  Otherwise, if we're at the topmost
-      # directory and the header's directory doesn't have a
-      # Makefile, then we also want to build the header.
-      if ($relative_dir eq $config_h_dir
-         || ($relative_dir eq '.' && ! &is_make_dir ($config_h_dir)))
+      for my $in (@ins)
        {
-         my ($cn_sans_dir, $stamp_dir);
-         if ($relative_dir eq $config_h_dir)
-           {
-             $cn_sans_dir = basename ($config_h_path);
-             $stamp_dir = '';
-           }
-         else
-           {
-             $cn_sans_dir = $config_h_path;
-             if ($config_h_dir eq '.')
-               {
-                 $stamp_dir = '';
-               }
-             else
-               {
-                 $stamp_dir = $config_h_dir . '/';
-               }
+         # If the input is in the current directory we want to distribute
+         # the input here.  Otherwise, if we're at the topmost
+         # directory and the input's directory doesn't have a
+         # Makefile, then we also want to distribute it.
+         my $in_dir = dirname $in;
+         if ($relative_dir eq $config_h_dir
+             || ($relative_dir eq '.' && ! &is_make_dir ($config_h_dir)))
+           {
+             error $config_header_location, "required file `$in' not found"
+               unless -f $in;
+             push_dist_common (rewrite_inputs_into_dependencies (1, $in));
            }
+       }
 
-         # Compute relative path from directory holding output
-         # header to directory holding input header.  FIXME:
-         # doesn't handle case where we have multiple inputs.
-         my $in0_sans_dir;
-         if (dirname ($ins[0]) eq $relative_dir)
-           {
-             $in0_sans_dir = basename ($ins[0]);
-           }
-         else
-           {
-             $in0_sans_dir = backname ($relative_dir) . '/' . $ins[0];
-           }
+      @ins = rewrite_inputs_into_dependencies (1, @ins);
 
-         require_file ($config_header_location, FOREIGN, $in0_sans_dir);
+      my ($cn_sans_dir, $stamp_dir);
+      if ($relative_dir eq $config_h_dir)
+       {
+         $cn_sans_dir = basename ($config_h_path);
+         $stamp_dir = '';
+       }
+      else
+       {
+         $cn_sans_dir = '$(top_builddir)/' . $config_h_path;
+         $stamp_dir = '$(top_builddir)/';
+         $stamp_dir .= "$config_h_dir/"
+           unless $config_h_dir eq '.';
+       }
 
-         # Header defined and in this directory.
-         my @files;
-         if (-f $config_h_path . '.top')
-           {
-             push (@files, "$cn_sans_dir.top");
-           }
-         if (-f $config_h_path . '.bot')
-           {
-             push (@files, "$cn_sans_dir.bot");
-           }
+      # Header defined and in this directory.
+      my @files;
+      if (-f $config_h_path . '.top')
+       {
+         push (@files, "$cn_sans_dir.top");
+       }
+      if (-f $config_h_path . '.bot')
+       {
+         push (@files, "$cn_sans_dir.bot");
+       }
 
-         push_dist_common (@files);
+      push_dist_common (@files);
 
-         # For now, acconfig.h can only appear in the top srcdir.
-         if (-f 'acconfig.h')
-           {
-             push (@files, '$(top_srcdir)/acconfig.h');
-           }
+      # For now, acconfig.h can only appear in the top srcdir.
+      if (-f 'acconfig.h')
+       {
+         push (@files, '$(top_srcdir)/acconfig.h');
+       }
 
-         my $stamp = "${stamp_dir}stamp-h${hdr_index}";
-         $output_rules .=
-           file_contents ('remake-hdr',
-                          new Automake::Location,
-                          FILES         => "@files",
-                          CONFIG_H      => $cn_sans_dir,
-                          CONFIG_HIN    => $in0_sans_dir,
-                          CONFIG_H_PATH => $config_h_path,
-                          STAMP         => "$stamp");
+      my $stamp = "${stamp_dir}stamp-h${hdr_index}";
+      $output_rules .=
+       file_contents ('remake-hdr',
+                      new Automake::Location,
+                      FILES            => "@files",
+                      CONFIG_H         => $cn_sans_dir,
+                      CONFIG_HIN       => $ins[0],
+                      CONFIG_H_DEPS    => "@ins",
+                      CONFIG_H_PATH    => $config_h_path,
+                      FIRST_CONFIG_HIN => ($hdr_index == 1),
+                      STAMP            => "$stamp");
 
-         push @distclean_config, $cn_sans_dir, $stamp;
-       }
+      push @distclean_config, $cn_sans_dir, $stamp;
     }
 
   $output_rules .= file_contents ('clean-hdr',
Index: lib/am/configure.am
===================================================================
RCS file: /cvs/automake/automake/lib/am/configure.am,v
retrieving revision 1.25
diff -u -r1.25 configure.am
--- lib/am/configure.am 23 Sep 2003 08:14:35 -0000      1.25
+++ lib/am/configure.am 1 Oct 2003 21:11:25 -0000
@@ -62,9 +62,13 @@
 ## all the files, instead of once per file (iii) generate Makefiles
 ## in newly added directories.
        @case '$?' in \
+         *config.status*) \
+## If AC_CONFIG_HEADERS is used, make sure all input are up-to-date before
+## running config.status.  -W %MAKEFILE% is a GNU Make flag that tells Make
+## not to try rebuilding %MAKEFILE% (otherwise we'd have an infinite loop).
+?TOPDIR_P??CONFIG_H_DEPS?      $(MAKE) $(AM_MAKEFLAGS) -W %MAKEFILE% 
%CONFIG_H_DEPS%; \
 ## Don't prefix $(top_builddir), because GNU make will strip it out
 ## when it's `.'.
-         *config.status*) \
 ?TOPDIR_P?         echo ' $(SHELL) ./config.status'; \
 ?TOPDIR_P?         $(SHELL) ./config.status;; \
 ?!TOPDIR_P?        cd $(top_builddir) && $(MAKE) $(AM_MAKEFLAGS) am--refresh;; 
\
Index: lib/am/remake-hdr.am
===================================================================
RCS file: /cvs/automake/automake/lib/am/remake-hdr.am,v
retrieving revision 1.38
diff -u -r1.38 remake-hdr.am
--- lib/am/remake-hdr.am        6 Sep 2003 05:36:57 -0000       1.38
+++ lib/am/remake-hdr.am        1 Oct 2003 21:11:25 -0000
@@ -28,11 +28,14 @@
        else :; fi
 
 
-%STAMP%: $(srcdir)/%CONFIG_HIN% $(top_builddir)/config.status
+%STAMP%: %CONFIG_H_DEPS% $(top_builddir)/config.status
        @rm -f %STAMP%
        cd $(top_builddir) && $(SHELL) ./config.status %CONFIG_H_PATH%
 
 
+## Only the first of the AC_CONFIG_HEADERS is assumed to be generated
+## by autoheader.
+if %?FIRST_CONFIG_HIN%
 $(srcdir)/%CONFIG_HIN%: %MAINTAINER-MODE% $(top_srcdir)/%CONFIGURE-AC% 
$(ACLOCAL_M4) %FILES%
        cd $(top_srcdir) && $(AUTOHEADER)
 ## Autoheader has the bad habit of not changing the time stamp if
@@ -41,3 +44,4 @@
 ## by config.status, there is no reason to make things complex for
 ## config.hin.
        touch $(srcdir)/%CONFIG_HIN%
+endif %?FIRST_CONFIG_HIN%
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.519
diff -u -r1.519 Makefile.am
--- tests/Makefile.am   30 Sep 2003 19:05:54 -0000      1.519
+++ tests/Makefile.am   1 Oct 2003 21:11:25 -0000
@@ -46,6 +46,8 @@
 asm.test \
 autohdr.test \
 autohdr2.test \
+autohdr3.test \
+autohdr4.test \
 auxdir.test \
 auxdir2.test \
 backsl.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.667
diff -u -r1.667 Makefile.in
--- tests/Makefile.in   30 Sep 2003 19:05:54 -0000      1.667
+++ tests/Makefile.in   1 Oct 2003 21:11:25 -0000
@@ -156,6 +156,8 @@
 asm.test \
 autohdr.test \
 autohdr2.test \
+autohdr3.test \
+autohdr4.test \
 auxdir.test \
 auxdir2.test \
 backsl.test \
Index: tests/autohdr3.test
===================================================================
RCS file: tests/autohdr3.test
diff -N tests/autohdr3.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/autohdr3.test 1 Oct 2003 21:11:25 -0000
@@ -0,0 +1,52 @@
+#!/bin/sh
+# Copyright (C) 2003  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# GNU Automake is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Automake; see the file COPYING.  If not, write to
+# the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+# Check rebuild rules for autoheader.
+
+required=GNUmake
+. ./defs
+
+set -e
+
+cat >>configure.in <<EOF
+m4_include([foo.m4])
+AC_CONFIG_HEADERS([config.h:config.hin])
+AC_OUTPUT
+EOF
+
+: > Makefile.am
+: > foo.m4
+
+$ACLOCAL
+$AUTOCONF
+$AUTOHEADER
+$AUTOMAKE
+
+./configure
+$MAKE
+
+$sleep
+echo 'AC_DEFINE([GREPME], 1, [Doc for GREPME])' > foo.m4
+
+$MAKE
+grep GREPME config.hin
+grep GREPME config.h
+
+$MAKE distcheck
Index: tests/autohdr4.test
===================================================================
RCS file: tests/autohdr4.test
diff -N tests/autohdr4.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/autohdr4.test 1 Oct 2003 21:11:25 -0000
@@ -0,0 +1,71 @@
+#!/bin/sh
+# Copyright (C) 2003  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# GNU Automake is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Automake; see the file COPYING.  If not, write to
+# the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+# Check rebuild rules for AC_CONFIG_HEADERS
+# PR/355.
+
+required='GNUmake gcc'
+. ./defs
+
+set -e
+
+cat >>configure.in <<EOF
+AC_PROG_CC
+AC_CONFIG_HEADERS([defs.h config.h:sub1/config.top:sub2/config.bot])
+AC_CONFIG_FILES([sub3/Makefile])
+AC_OUTPUT
+EOF
+
+mkdir sub1 sub2 sub3
+
+: > sub1/config.top
+echo '#define NAME "grepme1"' >sub2/config.bot
+echo SUBDIRS = sub3 >Makefile.am
+echo noinst_PROGRAMS = run >sub3/Makefile.am
+
+cat >sub3/run.c <<'EOF'
+#include <defs.h>
+#include <config.h>
+#include <stdio.h>
+
+int
+main ()
+{
+  puts (NAME); /* from config.h */
+  puts (PACKAGE); /* from defs.h */
+}
+EOF
+
+
+$ACLOCAL
+$AUTOCONF
+$AUTOHEADER
+$AUTOMAKE
+
+./configure
+$MAKE
+sub3/run | grep grepme1
+
+$sleep
+echo '#define NAME "grepme2"' > sub2/config.bot
+$MAKE
+sub3/run | grep grepme2
+
+$MAKE distcheck
Index: tests/config.test
===================================================================
RCS file: /cvs/automake/automake/tests/config.test,v
retrieving revision 1.8
diff -u -r1.8 config.test
--- tests/config.test   8 Sep 2002 13:07:55 -0000       1.8
+++ tests/config.test   1 Oct 2003 21:11:25 -0000
@@ -22,21 +22,29 @@
 # idea is that if config.h is in a subdir, and there is no Makefile in
 # that subdir, then we want to build config.h as the top level.
 
+required=GNUmake
 . ./defs || exit 1
 
-cat > configure.in << 'END'
-AC_INIT
-AM_INIT_AUTOMAKE(nonesuch, nonesuch)
+set -e
+
+cat >> configure.in << 'END'
 AM_CONFIG_HEADER(subdir/config.h)
-AC_PROG_CC
-AC_OUTPUT(Makefile)
+AC_OUTPUT
 END
 
 : > Makefile.am
 mkdir subdir
 : > subdir/config.h.in
 
-$ACLOCAL || exit 1
-$AUTOMAKE || exit 1
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+./configure
+$MAKE
+
+$sleep
+echo '#define FOO' > subdir/config.h.in
+$MAKE subdir/config.h
+grep FOO subdir/config.h
 
-grep '^subdir/config.h:' Makefile.in
+$MAKE distcheck

-- 
Alexandre Duret-Lutz