Re: aclocal-1.8/m4_include behaving oddly

[Alexandre Duret-Lutz]

>>> "pme" == Phil Edwards <address@hidden> writes:

 pme> One of the GCC runtime libraries (libstdc++-v3) has for years contained
 pme> the following lines in acinclude.m4:

 pme> m4_include([../libtool.m4])
 pme> dnl The lines below arrange for aclocal not to bring an installed
 pme> dnl libtool.m4 into aclocal.m4, while still arranging for automake to
 pme> dnl add a definition of LIBTOOL to Makefile.in.
 pme> ifelse(,,,[AC_SUBST(LIBTOOL)
 pme> AC_DEFUN([AM_PROG_LIBTOOL])
 pme> AC_DEFUN([AC_LIBTOOL_DLOPEN])
 pme> AC_DEFUN([AC_PROG_LD])
 pme> ])

I agree with Andreas Schwab that if you switch to aclocal 1.8,
all these lines can be removed if `-I ..' is passed to aclocal.
In that case aclocal 1.8 should add the m4_include itself.  It's
cleaner.

However what you see is definitely a bug in aclocal 1.8.
aclocal 1.8 does scan m4_included files (1.7 did not), but it
only scan them for required macros without looking at macro
definitions.  So it sees that ../libtool.m4 requires macros like
_AC_LIBTOOL_CXX but does not find them.

I'm installing the following fix on HEAD and branch-1-8.

2004-01-02  Alexandre Duret-Lutz  <address@hidden>

        * aclocal.in (%file_includes): New variable.
        (scan_configure_dep): Compile $m4_include_rx and $ac_require_rx once.
        (scan_file): Scan for included files, and process these files
        recursively.  Fill %file_includes and %file_contents.  Return the
        list of included files, not the contents.
        (scan_m4_files): Adjust calls to scan_files.
        (strip_redundant_includes): New function.
        (trace_used_macros): Call it.
        (write_aclocal): Likewise.  Also check the mtime of included files.
        * tests/Makfile.am (TESTS): Add acloca14.test.
        * tests/acloca14.test: New file.
        Report from Phil Edwards.

Index: THANKS
===================================================================
RCS file: /cvs/automake/automake/THANKS,v
retrieving revision 1.233
diff -u -r1.233 THANKS
--- THANKS      1 Jan 2004 18:54:20 -0000       1.233
+++ THANKS      2 Jan 2004 14:41:00 -0000
@@ -185,6 +185,7 @@
 Peter Mattis           address@hidden
 Peter Muir             address@hidden
 Petter Reinholdtsen    address@hidden
+Phil Edwards           address@hidden
 Phil Nelson            address@hidden
 Philip Fong            address@hidden
 Philip S Tellis                address@hidden
Index: aclocal.in
===================================================================
RCS file: /cvs/automake/automake/aclocal.in,v
retrieving revision 1.97
diff -u -r1.97 aclocal.in
--- aclocal.in  1 Jan 2004 17:34:17 -0000       1.97
+++ aclocal.in  2 Jan 2004 14:41:00 -0000
@@ -74,6 +74,8 @@
 
 # Remember the order into which we scanned the files.
 # It's important to output the contents of aclocal.m4 in the opposite order.
+# (Definitions in first files we have scanned should override those from
+# later files.  So they must appear last in the output.)
 @file_order = ();
 
 # Map macro names to file names.
@@ -82,6 +84,9 @@
 # Map file names to file contents.
 %file_contents = ();
 
+# Map file names to included files (transitively closed).
+%file_includes = ();
+
 # How much to say.
 $verbose = 0;
 
@@ -125,7 +130,7 @@
     # First, scan acinclude.m4 if it exists.
     if (-f 'acinclude.m4')
     {
-       $file_contents{'acinclude.m4'} = &scan_file ('acinclude.m4');
+       &scan_file ('acinclude.m4');
     }
 
     local ($m4dir);
@@ -149,7 +154,7 @@
            next if $file eq 'aclocal.m4';
 
            $fullfile = $m4dir . '/' . $file;
-           $file_contents{$fullfile} = &scan_file ($fullfile);
+           &scan_file ($fullfile);
        }
        closedir (DIR);
     }
@@ -219,12 +224,12 @@
       s/\bdnl\b.*$//;
       s/\#.*$//;
 
-      while (/$m4_include_rx/g)
+      while (/$m4_include_rx/go)
        {
          push (@ilist, $1 || $2);
        }
 
-      while (/$ac_require_rx/g)
+      while (/$ac_require_rx/go)
        {
          push (@rlist, $1 || $2);
        }
@@ -261,15 +266,23 @@
 # Point to the documentation for underquoted AC_DEFUN only once.
 my $underquoted_manual_once = 0;
 
-# Scan a single M4 file.  Return contents.
+# Scan a single M4 file, and all files it includes.
+# Return the list of included files.
 sub scan_file ($)
 {
-  local ($file) = @_;
+  my ($file) = @_;
+  my $base = dirname $file;
+
+  # Do not scan the same file twice.
+  return @$file_includes{$file} if exists $file_includes{$file};
+  # Prevent potential infinite recursion (if two files include each other).
+  return () if exists $file_contents{$file};
 
   unshift @file_order, $file;
 
   my $fh = new Automake::XFile $file;
   my $contents = '';
+  my @inc_files = ();
   while ($_ = $fh->getline)
     {
       # Ignore `##' lines.
@@ -277,7 +290,7 @@
 
       $contents .= $_;
 
-      if (/$ac_defun_rx/)
+      while (/$ac_defun_rx/go)
        {
          if (! defined $1)
            {
@@ -288,11 +301,12 @@
                unless $underquoted_manual_once;
              $underquoted_manual_once = 1;
            }
-         if (! defined $map{$1 || $2})
+         my $macro = $1 || $2;
+         if (! defined $map{$macro})
            {
-             print STDERR "aclocal: found macro $1 in $file: $.\n"
+             print STDERR "aclocal: found macro $macro in $file: $.\n"
                if $verbose;
-             $map{$1 || $2} = $file;
+             $map{$macro} = $file;
            }
          else
            {
@@ -301,18 +315,69 @@
              # extremely unpopular.  It causes actual problems which
              # are hard to work around, especially when you must
              # mix-and-match tool versions.
-             print STDERR "aclocal: ignoring macro $1 in $file: $.\n"
+             print STDERR "aclocal: ignoring macro $macro in $file: $.\n"
                if $verbose;
            }
        }
+
+      while (/$m4_include_rx/go)
+       {
+         my $ifile = $1 || $2;
+         # m4_include is relative to the directory of the file which
+         # perform the include, but we want paths relative to the
+         # directory where aclocal is run.  Do not use
+         # File::Spec->rel2abs, because we want to store relative
+         # paths (they might be used later of aclocal outputs an
+         # m4_include for this file, or if the user itself includes
+         # this file).
+         $ifile = "$base/$ifile"
+           unless $base eq '.' || File::Spec->file_name_is_absolute ($ifile);
+         push (@inc_files, $ifile);
+       }
     }
+  $file_contents{$file} = $contents;
 
-  return $contents;
+  # For some reason I don't understand, it does not work
+  # to do `map { scan_file ($_) } @inc_files' below.
+  # With Perl 5.8.2 it undefines @inc_files.
+  my @copy = @inc_files;
+  my @all_inc_files = (@inc_files, map { scan_file ($_) } @copy);
+  $file_includes{$file} = address@hidden;
+  return @all_inc_files;
+}
+
+# strip_redundant_includes (%FILES)
+# ---------------------------------
+# Each key in %FILES is a file that must be present in the output.
+# However some of these files might already include other files in %FILES,
+# so there is no point in including them another time.
+# This removes items of %FILES which are already included by another file.
+sub strip_redundant_includes (%)
+{
+  my %files = @_;
+  # Files at the end of @file_order should override those at the beginning,
+  # so it is important to preserve these trailing files.  We can remove
+  # a file A if it is going to be output before a file B that includes
+  # file A, not the converse.
+  foreach my $file (reverse @file_order)
+    {
+      next unless exists $files{$file};
+      foreach my $ifile (@{$file_includes{$file}})
+       {
+         next unless exists $files{$ifile};
+         delete $files{$ifile};
+         print STDERR "$ifile is already included by $file\n"
+           if $verbose;
+       }
+    }
+  return %files;
 }
 
 sub trace_used_macros ()
 {
   my %files = map { $map{$_} => 1 } keys %macro_seen;
+  $files{'acinclude.m4'} = 1 if -f 'acinclude.m4';
+  %files = strip_redundant_includes %files;
 
   my $traces = ($ENV{AUTOM4TE} || 'autom4te');
   $traces .= " --language Autoconf-without-aclocal-m4 ";
@@ -358,11 +423,16 @@
 
   my %files = map { $map{$_} => 1 } @macros;
   $files{'acinclude.m4'} = 1 if -f 'acinclude.m4';
+  %files = strip_redundant_includes %files;
 
   for $file (grep { exists $files{$_} } @file_order)
     {
-      my $mtime = mtime $file;
-      $greatest_mtime = $mtime if $greatest_mtime < $mtime;
+      # Check the time stamp of this file, and all files it includes.
+      for my $ifile ($file, @{$file_includes{$file}})
+       {
+         my $mtime = mtime $ifile;
+         $greatest_mtime = $mtime if $greatest_mtime < $mtime;
+       }
 
       # If the file to add looks like outside the project, copy it
       # to the output.  The regex catches filenames starting with
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.539
diff -u -r1.539 Makefile.am
--- tests/Makefile.am   4 Dec 2003 18:17:19 -0000       1.539
+++ tests/Makefile.am   2 Jan 2004 14:41:01 -0000
@@ -16,6 +16,7 @@
 acloca11.test \
 acloca12.test \
 acloca13.test \
+acloca14.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.700
diff -u -r1.700 Makefile.in
--- tests/Makefile.in   1 Jan 2004 17:34:18 -0000       1.700
+++ tests/Makefile.in   2 Jan 2004 14:41:01 -0000
@@ -130,6 +130,7 @@
 acloca11.test \
 acloca12.test \
 acloca13.test \
+acloca14.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
Index: tests/acloca14.test
===================================================================
RCS file: tests/acloca14.test
diff -N tests/acloca14.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/acloca14.test 2 Jan 2004 14:41:01 -0000
@@ -0,0 +1,111 @@
+#! /bin/sh
+# Copyright (C) 2004  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.
+
+# Make sure m4_included files are also scanned for definitions.
+# Report from Phil Edwards.
+
+required=GNUmake
+. ./defs || exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AM_PROG_LIBTOOL
+AC_OUTPUT
+END
+
+echo 'm4_include([a.m4])' > acinclude.m4
+echo 'm4_include([b.m4])' > a.m4
+cat >b.m4 <<EOF
+m4_include([c.m4])
+AC_DEFUN([AM_PROG_LIBTOOL],
+[AC_REQUIRE([SOMETHING])dnl
+AC_REQUIRE([SOMETHING_ELSE])dnl
+])
+
+AC_DEFUN([SOMETHING])
+EOF
+echo 'm4_include([d.m4])' > c.m4
+echo 'AC_DEFUN([SOMETHING_ELSE])' >d.m4
+
+mkdir defs
+echo 'AC_DEFUN([SOMETHING_ELSE])' >defs/e.m4
+echo 'AC_DEFUN([ANOTHER_MACRO])' >defs/f.m4
+
+cat >>Makefile.am<<\EOF
+ACLOCAL_AMFLAGS = -I defs
+testdist1: distdir
+       test -f $(distdir)/acinclude.m4
+       test -f $(distdir)/a.m4
+       test -f $(distdir)/b.m4
+       test -f $(distdir)/c.m4
+       test -f $(distdir)/d.m4
+       test ! -d $(distdir)/defs
+testdist2: distdir
+       test -f $(distdir)/acinclude.m4
+       test -f $(distdir)/a.m4
+       test -f $(distdir)/b.m4
+       test -f $(distdir)/c.m4
+       test -f $(distdir)/d.m4
+       test ! -f $(distdir)/defs/e.m4
+       test -f $(distdir)/defs/f.m4
+EOF
+
+$ACLOCAL -I defs
+
+$FGREP acinclude.m4 aclocal.m4
+# None of the following macro should be included.  acinclude.m4
+# includes the first four, and the last two are not needed at all.
+$FGREP a.m4 aclocal.m4 && exit 1
+$FGREP b.m4 aclocal.m4 && exit 1
+$FGREP c.m4 aclocal.m4 && exit 1
+$FGREP d.m4 aclocal.m4 && exit 1
+$FGREP defs/e.m4 aclocal.m4 && exit 1
+$FGREP defs/f.m4 aclocal.m4 && exit 1
+
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+$MAKE testdist1
+
+cp aclocal.m4 stamp
+$sleep
+
+cat >>c.m4 <<\EOF
+AC_DEFUN([FOO], [ANOTHER_MACRO])
+EOF
+$MAKE
+# Because c.m4 has changed, aclocal.m4 must have been rebuilt.
+test `ls -1t aclocal.m4 stamp | sed 1q` = aclocal.m4
+# However, since FOO is not used, f.m4 should not be included
+# and the contents of aclocal.m4 should remain the same
+cmp aclocal.m4 stamp
+
+
+# If FOO where to be used, that would be another story, of course.
+cat >>configure.in <<EOF
+FOO
+EOF
+cp aclocal.m4 stamp
+$sleep
+$MAKE
+grep 'defs/f.m4' aclocal.m4
+$MAKE testdist2

-- 
Alexandre Duret-Lutz