Re: aclocal-1.8/m4_include behaving oddly

[Alexandre Duret-Lutz <adl at src dot lip6 dot fr>]

>>> "pme" == Phil Edwards <phil@jaj.com> 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  <adl@gnu.org>

	* 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		petm@scam.XCF.Berkeley.EDU
 Peter Muir		iyhi@yahoo.com
 Petter Reinholdtsen	pere@hungry.com
+Phil Edwards		phil@jaj.com
 Phil Nelson		phil@cs.wwu.edu
 Philip Fong		pwlfong@users.sourceforge.net
 Philip S Tellis		philip@ncst.ernet.in
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} = \@all_inc_files;
+  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