FYI: reduce the am__XXX* shadowing clutter (Was: Re: $(EXEEXT) in TESTS required?)

[Alexandre Duret-Lutz]

| Note to self: need to tune transform_variable_recursively so it doesn't
| create a temporary variable when no transformation has occurred.

I'm checking this patch in.  It has two effects :

  - automake no longer systematically creates a "shadow" am__XXX variable
    for each subvariable of a recursive transformation: there is no need
    to do so if the variable hasn't been altered by the transformation.
    This is visible in exeext4.test where automake used to shadow
    `BAZE = baz$(EXEEXT)' with `am__EXEEXT_3 = baz$(EXEEXT)'.

  - automake does not systematically redefine such "shadow" variables
    or even the top-most variable been transformed.  Not redefining
    shadow variable is a very small speed improvement I didn't bother to
    quantify.  Not redefining the top-level helps in projects like Automake
    itself:  since the patch to add $(EXEEXT) where needed in TESTS,
    automake's redefinition of TESTS was causing

             TESTS = \
               a.test \
               b.test \
               c.test

    to be output as

             TESTS = a.test b.test c.test

    (since all automake variables are "prettified"), causing unnecessary
    long Makefile.in diffs anytime a new test was inserted.  Now the TESTS
    definition in Makefile.in is again the same as it is in Makefile.am.


2006-04-09  Alexandre Duret-Lutz  <address@hidden>

        * lib/Automake/Variable.pm (_hash_varname, _hash_values): New functions.
        (_gen_varname): Use _hash_values, and return a flag indicating whether
        the variable name was generated or reused.
        (transform_variable_recursively): Do not redefine variables that
        are reused, and try to reuse the variable being transformed.
        * tests/check2.test: Make sure TESTS hasn't been redefined.
        * tests/check5.test, tests/exeext4.test: Make sure variables have
        been reused.
        * tests/subst2.test: Make sure bin_PROGRAMS gets rewritten.

Index: lib/Automake/Variable.pm
===================================================================
RCS file: /cvs/automake/automake/lib/Automake/Variable.pm,v
retrieving revision 1.42
diff -u -r1.42 Variable.pm
--- lib/Automake/Variable.pm    20 Mar 2006 20:31:28 -0000      1.42
+++ lib/Automake/Variable.pm    9 Apr 2006 13:25:12 -0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2003, 2004, 2005  Free Software Foundation, Inc.
+# Copyright (C) 2003, 2004, 2005, 2006  Free Software Foundation, Inc.

 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -142,6 +142,11 @@
 # Keys have the form "(COND1)VAL1(COND2)VAL2..." where VAL1 and VAL2
 # are the values of the variable for condition COND1 and COND2.
 my %_gen_varname = ();
+# $_gen_varname_n{$base} is the number of variables generated by
+# _gen_varname() for $base.  This is not the same as keys
+# %{$_gen_varname{$base}} because %_gen_varname may also contain
+# variables not generated by _gen_varname.
+my %_gen_varname_n = ();

 # Declare the macros that define known variables, so we can
 # hint the user if she try to use one of these variables.
@@ -338,6 +343,7 @@
   $_appendvar = 0;
   @_var_order = ();
   %_gen_varname = ();
+  %_gen_varname_n = ();
   $_traversal = 0;
 }

@@ -1463,14 +1469,48 @@
   return &$fun_collect ($var, $parent_cond, @allresults);
 }

-# $VARNAME
+# _hash_varname ($VAR)
+# --------------------
+# Compute the key associated $VAR in %_gen_varname.
+# See _gen_varname() below.
+sub _hash_varname ($)
+{
+  my ($var) = @_;
+  my $key = '';
+  foreach my $cond ($var->conditions->conds)
+    {
+      my @values = $var->value_as_list ($cond);
+      $key .= "($cond)@values";
+    }
+  return $key;
+}
+
+# _hash_values (@VALUES)
+# ----------------------
+# Hash @VALUES for %_gen_varname.  @VALUES shoud be a list
+# of pairs: ([$cond, @values], [$cond, @values], ...).
+# See _gen_varname() below.
+sub _hash_values (@)
+{
+  my $key = '';
+  foreach my $pair (@_)
+    {
+      my ($cond, @values) = @$pair;
+      $key .= "($cond)@values";
+    }
+  return $key;
+}
+# ($VARNAME, $GENERATED)
 # _gen_varname ($BASE, @DEFINITIONS)
 # ---------------------------------
 # Return a variable name starting with $BASE, that will be
 # used to store definitions @DEFINITIONS.
 # @DEFINITIONS is a list of pair [$COND, @OBJECTS].
 #
-# If we already have a $BASE-variable containing @DEFINITIONS, reuse it.
+# If we already have a $BASE-variable containing @DEFINITIONS, reuse
+# it and set $GENERATED to 0.  Otherwise construct a new name and set
+# $GENERATED to 1.
+#
 # This way, we avoid combinatorial explosion of the generated
 # variables.  Especially, in a Makefile such as:
 #
@@ -1501,19 +1541,17 @@
 sub _gen_varname ($@)
 {
   my $base = shift;
-  my $key = '';
-  foreach my $pair (@_)
-    {
-      my ($cond, @values) = @$pair;
-      $key .= "($cond)@values";
-    }
+  my $key = _hash_values @_;

-  return $_gen_varname{$base}{$key} if exists $_gen_varname{$base}{$key};
+  return ($_gen_varname{$base}{$key}, 0)
+    if exists $_gen_varname{$base}{$key};

-  my $num = 1 + keys (%{$_gen_varname{$base}});
+  my $num = 1 + ($_gen_varname_n{$base} || 0);
+  $_gen_varname_n{$base} = $num;
   my $name = "${base}_${num}";
   $_gen_varname{$base}{$key} = $name;
-  return $name;
+
+  return ($name, 1);
 }

 =item C<$resvar = transform_variable_recursively ($var, $resvar, $base, 
$nodefine, $where, &fun_item, [%options])>
@@ -1557,12 +1595,24 @@
      # of the recursive transformation of a subvariable.
      sub {
        my ($subvar, $parent_cond, @allresults) = @_;
+       # If no definition is required, return anything: the result is
+       # not expected to be used, only the side effect of $fun_item
+       # should matter.
+       return 'report-me' if $nodefine;
+       # Cache $subvar, so that we reuse it if @allresults is the same.
+       my $key = _hash_varname $subvar;
+       $_gen_varname{$base}{$key} = $subvar->name;
+
        # Find a name for the variable, unless this is the top-variable
        # for which we want to use $resvar.
-       my $varname =
-        ($var != $subvar) ? _gen_varname ($base, @allresults) : $resvar;
-       # Define the variable if required.
-       unless ($nodefine)
+       my ($varname, $generated) =
+        ($var != $subvar) ? _gen_varname ($base, @allresults) : ($resvar, 1);
+
+       # Define the variable if we are not reusing a previously
+       # defined variable.  At the top-level, we can also avoid redefining
+       # the variable if it already contains the same values.
+       if ($generated
+          && !($varname eq $var->name && $key eq _hash_values @allresults))
         {
           # If the new variable is the source variable, we assume
           # we are trying to override a user variable.  Delete
@@ -1587,8 +1637,8 @@
                           '', $where, VAR_PRETTY);
                 }
             }
-          set_seen $varname;
         }
+       set_seen $varname;
        return "\$($varname)";
      },
      %options);
Index: tests/check2.test
===================================================================
RCS file: /cvs/automake/automake/tests/check2.test,v
retrieving revision 1.3
diff -u -r1.3 check2.test
--- tests/check2.test   14 May 2005 20:28:54 -0000      1.3
+++ tests/check2.test   9 Apr 2006 13:25:12 -0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2002  Free Software Foundation, Inc.
+# Copyright (C) 2002, 2006  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -33,7 +33,8 @@

 cat > Makefile.am << 'END'
 SUBDIRS = dir
-TESTS = subrun.sh
+TESTS = \
+  subrun.sh
 subrun.sh:
        (echo '#! /bin/sh'; echo 'dir/echo.sh') > $@
        chmod +x $@
@@ -60,3 +61,8 @@
 # in check.test and check3.test).
 grep 'check: check-recursive' Makefile.in
 grep 'check: check-am' dir/Makefile.in
+
+# Make sure subrun.sh is still on its line as above.  This means Automake
+# hasn't rewritten the TESTS line unnecessarily (we can tell, because all
+# Automake variables are reformatted by VAR_PRETTY).
+grep '  subrun.sh' Makefile.in
Index: tests/check5.test
===================================================================
RCS file: /cvs/automake/automake/tests/check5.test,v
retrieving revision 1.2
diff -u -r1.2 check5.test
--- tests/check5.test   18 Mar 2006 06:32:36 -0000      1.2
+++ tests/check5.test   9 Apr 2006 13:25:12 -0000
@@ -54,4 +54,7 @@
 test -f ok
 EXEEXT=.bin $MAKE -e print-tests >output
 cat output
+# No am__EXEEXT_* variable is needed.
+grep '_EXEEXT' Makefile.in && exit 1
 grep 'BEG: one.bin two.bin :END' output
+$FGREP 'TESTS = $(check_PROGRAMS)' Makefile.in
Index: tests/exeext4.test
===================================================================
RCS file: /cvs/automake/automake/tests/exeext4.test,v
retrieving revision 1.3
diff -u -r1.3 exeext4.test
--- tests/exeext4.test  29 Jan 2006 17:35:12 -0000      1.3
+++ tests/exeext4.test  9 Apr 2006 13:25:12 -0000
@@ -78,3 +78,8 @@
 $MAKE print-barbaz > output
 cat output
 grep 'bar baz bar' output
+
+# Only two am__EXEEXT_* variables are needed here: one for BAR, and one
+# BAZ.  The latter must use the former.
+test 2 = `grep '__EXEEXT_. =' Makefile.in | wc -l`
+grep 'am__EXEEXT_2 = .*am__EXEEXT_1' Makefile.in
Index: tests/subst2.test
===================================================================
RCS file: /cvs/automake/automake/tests/subst2.test,v
retrieving revision 1.3
diff -u -r1.3 subst2.test
--- tests/subst2.test   14 May 2005 20:28:56 -0000      1.3
+++ tests/subst2.test   9 Apr 2006 13:25:12 -0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2003  Free Software Foundation, Inc.
+# Copyright (C) 2003, 2006  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -25,6 +25,7 @@
 set -e

 cat >> configure.in << 'END'
+AC_PROG_CC
 AC_SUBST([ABCDEFGHIJKLMNOPQRSTUVWX])
 AC_SUBST([ABCDEFGHIJKLMNOPQRSTUVWXY])
 AC_SUBST([ABCDEFGHIJKLMNOPQRSTUVWXYZ])
@@ -32,7 +33,7 @@
 END

 cat >Makefile.am <<'END'
-bin_PROGRAMS = @ABCDEFGHIJKLMNOPQRSTUVWX@ @ABCDEFGHIJKLMNOPQRSTUVWXY@ 
@ABCDEFGHIJKLMNOPQRSTUVWXYZ@
+bin_PROGRAMS = x @ABCDEFGHIJKLMNOPQRSTUVWX@ @ABCDEFGHIJKLMNOPQRSTUVWXY@ 
@ABCDEFGHIJKLMNOPQRSTUVWXYZ@
 EXTRA_PROGRAMS =

 EXEEXT = .bin
@@ -45,12 +46,12 @@
 $AUTOCONF
 $AUTOMAKE
 ./configure
-$MAKE print-programs >foo
+EXEEXT=.bin $MAKE print-programs >foo
 cat foo
-grep 'BEG: :END' foo
-am__empty=X $MAKE -e print-programs >foo
+grep 'BEG: x.bin :END' foo
+EXEEXT=.bin am__empty=X $MAKE -e print-programs >foo
 cat foo
-grep 'BEG: X :END' foo
+grep 'BEG: x.bin X :END' foo

 # Test for another bug, where EXTRA_PROGRAMS was removed because it was empty.
 grep EXTRA_PROGRAMS Makefile.in
--
Alexandre Duret-Lutz

Shared books are happy books.     http://www.bookcrossing.com/friend/gadl