Re: Fix AC_CONFIG_LINKS brokenness when source is variable

[Alexandre Duret-Lutz]

Hi Paolo,
 
Thanks for the report and the patch.

>>> "Paolo" == Paolo Bonzini <address@hidden> writes:
 
 Paolo> -      # Require all input files.
 Paolo> -      require_file ($where, FOREIGN, $local);
 Paolo> +      # Require all input files, unless they are expanded from a
 Paolo> +      # shell variable.
 Paolo> +      require_file ($where, FOREIGN, $local)
 Paolo> +       unless $local =~ /\$/;
 Paolo> }

Opening a can of worms...

This was not enough because there are code before require_file
that should not be run either (it setups rules to create the
directory for the source in $(distdir), but since the source is
variable it's a bad idea).  Similarly, the destination of link
should not be cleaned when it contains a $.

Also although such a setup does not work presently in Autoconf
(I've just sent a bug report about it), it makes sense not to
distribute the source if it is an output of another AC_CONFIG_*
macro: that makes the code more homogeneous since we already do
this for AC_CONFIG_FILES.  (I realize we should do this for
AC_CONFIG_HEADERS too; I'll work on it tomorrow.)

Here is what I'm installing.

2003-12-03  Paolo Bonzini  <address@hidden>
            Alexandre Duret-Lutz  <address@hidden>

        * automake.in (handle_configure): Do not require link sources if
        they contain a dollar, or if they were built.  Likewise, do not
        clean link destination if they contain a dollar.
        (scan_autoconf_traces) <AC_CONFIG_LINKS>: Populate
        %ac_config_files_location with link destinations.  Do not
        store locations in @config_links, now that %ac_config_files_location
        have them.
        * tests/conflnk3.test: New file.
        * tests/Makefile.am (TESTS): Add conflnk3.test.

Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1523
diff -u -r1.1523 automake.in
--- automake.in 30 Nov 2003 17:00:36 -0000      1.1523
+++ automake.in 3 Dec 2003 22:54:05 -0000
@@ -316,7 +316,8 @@
 # List of files in AC_CONFIG_FILES/AC_OUTPUT without Makefile.am's,
 # and their outputs.
 my @other_input_files = ();
-# Where the last AC_CONFIG_FILES/AC_OUTPUT appears.
+# Where each AC_CONFIG_FILES/AC_OUTPUT/AC_CONFIG_LINK appears.
+# The keys are the files created by these macros.
 my %ac_config_files_location = ();
 
 # List of directories to search for configure-required files.  This
@@ -3766,59 +3767,60 @@
       push (@actual_other_files, $local);
     }
 
-  foreach my $struct (@config_links)
+  # For links we should clean destinations and distribute sources.
+  foreach my $spec (@config_links)
     {
-      my ($spec, $where) = @$struct;
       my ($link, $file) = split /:/, $spec;
+      my $where = $ac_config_files_location{$link};
 
-      # We skip links that aren't in this directory.  However, if
-      # the link's directory does not have a Makefile, and we are
-      # currently doing `.', then we add the link to CONFIG_CLEAN_FILES
-      # in `.'s Makefile.in.
-      my $local = basename ($link);
-      my $fd = dirname ($link);
-      if ($fd ne $relative_dir)
+      # Skip destinations that contain shell variables.
+      if ($link !~ /\$/)
        {
-         if ($relative_dir eq '.' && ! &is_make_dir ($fd))
+         # We skip links that aren't in this directory.  However, if
+         # the link's directory does not have a Makefile, and we are
+         # currently doing `.', then we add the link to CONFIG_CLEAN_FILES
+         # in `.'s Makefile.in.
+         my $local = basename ($link);
+         my $fd = dirname ($link);
+         if ($fd ne $relative_dir)
            {
-             $local = $link;
-           }
-         else
-           {
-             $local = undef;
+             if ($relative_dir eq '.' && ! &is_make_dir ($fd))
+               {
+                 $local = $link;
+               }
+             else
+               {
+                 $local = undef;
+               }
            }
+         push @actual_other_files, $local if $local;
        }
 
-      push @actual_other_files, $local if $local;
-
-      $local = basename ($file);
-      $fd = dirname ($file);
-
-      # Make sure the dist directory for each input file is created.
-      # We only have to do this at the topmost level though.
-      if ($relative_dir eq '.')
+      # Do not process sources that contain shell variables.
+      if ($file !~ /\$/)
        {
-         $dist_dirs{$fd} = 1;
-       }
+         my $fd = dirname ($file);
 
-      # We skip files that aren't in this directory.  However, if
-      # the files's directory does not have a Makefile, and we are
-      # currently doing `.', then we require the file from `.'.
-      if ($fd ne $relative_dir)
-       {
-         if ($relative_dir eq '.' && ! &is_make_dir ($fd))
+         # Make sure the dist directory for each input file is created.
+         # We only have to do this at the topmost level though.
+         if ($relative_dir eq '.')
            {
-             $local = $file;
+             $dist_dirs{$fd} = 1;
            }
-         else
+
+         # We distribute files that are in this directory.
+         # At the top-level (`.') we also distribute files whose
+         # directory does not have a Makefile.
+         if (($fd eq $relative_dir)
+             || ($relative_dir eq '.' && ! &is_make_dir ($fd)))
            {
-             next;
+             # The following will distribute $file as a side-effect when
+             # it is appropriate (i.e., when $file is not already an output).
+             # We do not need the result, just the side-effect.
+             rewrite_inputs_into_dependencies ($link, $file);
            }
        }
-
-      # Require all input files.
-      require_file ($where, FOREIGN, $local);
-  }
+    }
 
   # These files get removed by "make distclean".
   define_pretty_variable ('CONFIG_CLEAN_FILES', TRUE, INTERNAL,
@@ -4498,7 +4500,12 @@
        }
       elsif ($macro eq 'AC_CONFIG_LINKS')
        {
-         push @config_links, map { [$_, $where] } split (' ', $args[1]);
+         foreach my $spec (split (' ', $args[1]))
+           {
+             my ($dest, $src) = split (':', $spec);
+             $ac_config_files_location{$dest} = $where;
+             push @config_links, $spec;
+           }
        }
       elsif ($macro eq 'AC_INIT')
         {
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.537
diff -u -r1.537 Makefile.am
--- tests/Makefile.am   30 Nov 2003 13:35:29 -0000      1.537
+++ tests/Makefile.am   3 Dec 2003 22:54:05 -0000
@@ -133,6 +133,7 @@
 confincl.test \
 conflnk.test \
 conflnk2.test \
+conflnk3.test \
 confsub.test \
 confvar.test \
 confvar2.test \
Index: tests/conflnk3.test
===================================================================
RCS file: tests/conflnk3.test
diff -N tests/conflnk3.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/conflnk3.test 3 Dec 2003 22:54:05 -0000
@@ -0,0 +1,86 @@
+#! /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.
+
+# Test to make sure that AC_CONFIG_LINKS using a variable source
+# is not broken.
+
+. ./defs || exit 1
+
+set -e
+
+cat > Makefile.am << 'END'
+SUBDIRS = sdir
+test: distdir
+       test ! -e $(distdir)/sdir/dest3
+       test ! -e $(distdir)/sdir/dest2
+       test ! -e $(distdir)/dest3
+       test ! -e $(distdir)/dest2
+       test -f $(distdir)/src2
+## src3 cannot be distributed, Automake knows nothing about it
+       test ! -e $(distdir)/sdir/src3
+       test ! -e $(distdir)/src3
+END
+
+: > src
+: > src2
+mkdir sdir
+: > sdir/Makefile.am
+: > sdir/src3
+
+cat >>configure.in << 'EOF'
+AC_CONFIG_FILES(sdir/Makefile)
+my_src_dir=sdir
+my_dest=dest
+AC_CONFIG_LINKS(sdir/dest2:src2 sdir/dest3:$my_src_dir/src3)
+AC_CONFIG_LINKS($my_dest:src)
+# the following is a link whose source is itself a link
+AC_CONFIG_LINKS(dest4:sdir/dest2)
+AC_OUTPUT
+EOF
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+# $my_src_dir and $my_dest are variables local to configure, they should
+# not appear in Makefile.
+grep my_src_dir Makefile.in && exit 1
+grep my_dest Makefile.in && exit 1
+
+./configure
+test -e sdir/dest2
+test -e sdir/dest3
+test -e dest
+test -e dest4
+$MAKE test
+
+$MAKE distclean
+test ! -e sdir/dest2
+test ! -e sdir/dest3
+test -e dest  # Should still exist, Automake knows nothing about it.
+rm -f dest
+test ! -e dest4
+
+## Cannot do the following, because at the time of writing Autoconf
+## (2.59) does not support AC_CONFIG_LINKS source in the build tree.
+# mkdir build
+# cd build
+# ../configure
+# $MAKE test

-- 
Alexandre Duret-Lutz