* lib/Automake/Variable.pm (define): Rework the way we check

Automake variable definition.  Ensure consistent :=/+=/=
definitions only for variables that have been and are defined by
Automake, and make it an internal error.  Ignore Automake
attempts to touch a user variable, even with += assignments.
* tests/txinfo29.test: New file.
* tests/Makefile.am (TESTS): Add txinfo29.test.
Reported by Bruce Korb.

diff --git a/ChangeLog b/ChangeLog
index c2cb450..12697d8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2004-01-03  Alexandre Duret-Lutz  <adl@gnu.org>
+
+       * lib/Automake/Variable.pm (define): Rework the way we check
+       Automake variable definition.  Ensure consistent :=/+=/=
+       definitions only for variables that have been and are defined by
+       Automake, and make it an internal error.  Ignore Automake
+       attempts to touch a user variable, even with += assignments.
+       * tests/txinfo29.test: New file.
+       * tests/Makefile.am (TESTS): Add txinfo29.test.
+       Reported by Bruce Korb.
+
 2004-01-02  Tom Tromey  <tromey@redhat.com>
 
        * automake.in (handle_source_transform): Don't generate dist

diff --git a/lib/Automake/Variable.pm b/lib/Automake/Variable.pm
index 7285902..cec6b56 100644 (file)
--- a/lib/Automake/Variable.pm
+++ b/lib/Automake/Variable.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2003  Free Software Foundation, Inc.
+# Copyright (C) 2003, 2004  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
@@ -799,9 +799,6 @@ sub define ($$$$$$$$)
                               || $pretty == VAR_SILENT
                               || $pretty == VAR_SORTED);
 
-  # We will adjust the owner of this variable unless told otherwise.
-  my $adjust_owner = 1;
-
   error $where, "bad characters in variable name `$var'"
     if $var !~ /$_VARIABLE_PATTERN/o;
 
@@ -830,12 +827,40 @@ sub define ($$$$$$$$)
   my $def = $self->def ($cond);
   my $new_var = $def ? 0 : 1;
 
-  # An Automake variable must be consistently defined with the same
-  # sign by Automake.
-  error ($where, "$var was set with `". $def->type .
-        "=' and is now set with `$type='")
-    if $owner == VAR_AUTOMAKE && ! $new_var && $def->type ne $type;
+  # Additional checks for Automake definitions.
+  if ($owner == VAR_AUTOMAKE && ! $new_var)
+    {
+      # An Automake variable must be consistently defined with the same
+      # sign by Automake.
+      if ($def->type ne $type && $def->owner == VAR_AUTOMAKE)
+       {
+         error ($def->location,
+                "Automake variable `$var' was set with `"
+                . $def->type . "=' here...", partial => 1);
+         error ($where, "... and is now set with `$type=' here.");
+         prog_error ("Automake variable assignments should be consistently\n"
+                     . "defined with the same sign.");
+       }
 
+      # If Automake tries to override a value specified by the user,
+      # just don't let it do.
+      if ($def->owner != VAR_AUTOMAKE)
+       {
+         if (! exists $_silent_variable_override{$var})
+           {
+             my $condmsg = ($cond == TRUE
+                            ? '' : (" in condition `" . $cond->human . "'"));
+             msg_cond_var ('override', $cond, $var,
+                           "user variable `$var' defined here$condmsg...",
+                           partial => 1);
+             msg ('override', $where,
+                  "... overrides Automake variable `$var' defined here");
+           }
+         verb ("refusing to override the user definition of:\n"
+               . $self->dump ."with `" . $cond->human . "' => `$value'");
+         return;
+       }
+    }
 
   # Differentiate assignment types.
 
@@ -843,6 +868,11 @@ sub define ($$$$$$$$)
   if ($type eq '+' && ! $new_var)
     {
       $def->append ($value, $comment);
+
+      # Only increase owners.  A VAR_CONFIGURE variable augmented in a
+      # Makefile.am becomes a VAR_MAKEFILE variable.
+      $def->set_owner ($owner, $where->clone)
+       if $owner > $def->owner;
     }
   # 2. append (+=) to a variable defined for *another* condition
   elsif ($type eq '+' && ! $self->conditions->false)
@@ -926,65 +956,31 @@ sub define ($$$$$$$$)
                       $where, $pretty);
            }
        }
-      # Don't adjust the owner.  The above &define did it in the
-      # right conditions.
-      $adjust_owner = 0;
     }
   # 3. first assignment (=, :=, or +=)
   else
     {
-      # If Automake tries to override a value specified by the user,
-      # just don't let it do.
-      if (! $new_var && $def->owner != VAR_AUTOMAKE
-         && $owner == VAR_AUTOMAKE)
-       {
-         if (! exists $_silent_variable_override{$var})
-           {
-             my $condmsg = ($cond == TRUE
-                            ? '' : (" in condition `" . $cond->human . "'"));
-             msg_cond_var ('override', $cond, $var,
-                           "user variable `$var' defined here$condmsg...",
-                           partial => 1);
-             msg ('override', $where,
-                  "... overrides Automake variable `$var' defined here");
-           }
-         verb ("refusing to override the user definition of:\n"
-               . $self->dump
-               ."with `" . $cond->human . "' => `$value'");
-       }
-      else
-       {
-         # There must be no previous value unless the user is redefining
-         # an Automake variable or an AC_SUBST variable for an existing
-         # condition.
-         _check_ambiguous_condition ($self, $cond, $where)
-           unless (!$new_var
-                   && (($def->owner == VAR_AUTOMAKE && $owner != VAR_AUTOMAKE)
-                       || $def->owner == VAR_CONFIGURE));
-
-         # Never decrease an owner.
-         $owner = $def->owner
-           if ! $new_var && $owner < $def->owner;
-
-         # Assignments to a macro set its location.  We don't adjust
-         # locations for `+='.  Ideally I suppose we would associate
-         # line numbers with random bits of text.
-         $def = new Automake::VarDef ($var, $value, $comment, $where->clone,
-                                      $type, $owner, $pretty);
-         $self->set ($cond, $def);
-         push @_var_order, $var;
-
-         # No need to adjust the owner later as we have overridden
-         # the definition.
-         $adjust_owner = 0;
-       }
+      # There must be no previous value unless the user is redefining
+      # an Automake variable or an AC_SUBST variable for an existing
+      # condition.
+      _check_ambiguous_condition ($self, $cond, $where)
+       unless (!$new_var
+               && (($def->owner == VAR_AUTOMAKE && $owner != VAR_AUTOMAKE)
+                   || $def->owner == VAR_CONFIGURE));
+
+      # Never decrease an owner.
+      $owner = $def->owner
+       if ! $new_var && $owner < $def->owner;
+
+      # Assignments to a macro set its location.  We don't adjust
+      # locations for `+='.  Ideally I suppose we would associate
+      # line numbers with random bits of text.
+      $def = new Automake::VarDef ($var, $value, $comment, $where->clone,
+                                  $type, $owner, $pretty);
+      $self->set ($cond, $def);
+      push @_var_order, $var;
     }
 
-  # The owner of a variable can only increase, because an Automake
-  # variable can be given to the user, but not the converse.
-  $def->set_owner ($owner, $where->clone)
-    if $adjust_owner && $owner > $def->owner;
-
   # Call any defined hook.  This helps to update some internal state
   # *while* parsing the file.  For instance the handling of SUFFIXES
   # requires this (see var_SUFFIXES_trigger).

diff --git a/tests/Makefile.am b/tests/Makefile.am
index d82c33b..02ffcd4 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -495,6 +495,7 @@ txinfo25.test \
 txinfo26.test \
 txinfo27.test \
 txinfo28.test \
+txinfo29.test \
 transform.test \
 unused.test \
 vars.test \

diff --git a/tests/Makefile.in b/tests/Makefile.in
index fcab5fc..bfa78d7 100644 (file)
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -609,6 +609,7 @@ txinfo25.test \
 txinfo26.test \
 txinfo27.test \
 txinfo28.test \
+txinfo29.test \
 transform.test \
 unused.test \
 vars.test \

diff --git a/tests/txinfo29.test b/tests/txinfo29.test
new file mode 100755 (executable)
index 0000000..7eae48b
--- /dev/null
+++ b/tests/txinfo29.test
@@ -0,0 +1,64 @@
+#! /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 that INFO_DEPS can be overridden.
+# Report from Bruce Korb.
+
+. ./defs || exit 1
+
+set -e
+
+cat > Makefile.am << 'END'
+INFO_DEPS = foo.info
+info_TEXINFOS = bar.texi
+END
+
+echo '@setfilename baz.info' > bar.texi
+
+$ACLOCAL
+AUTOMAKE_fails --add-missing
+grep '^Makefile.am:1:.*user variable.*INFO_DEPS' stderr
+grep 'overrides Automake variable.*INFO_DEPS' stderr
+
+$AUTOMAKE -Wno-override
+
+# There is only one definition of INFO_DEPS
+test 1 = `grep '^INFO_DEPS.*=' Makefile.in | wc -l`
+# and it is the right one.
+grep '^INFO_DEPS = foo.info$' Makefile.in
+
+
+# Likewise with AC_SUBST.
+
+cat > Makefile.am << 'END'
+info_TEXINFOS = bar.texi
+END
+echo 'AC_SUBST([INFO_DEPS])' >>configure.in
+AUTOMAKE_fails
+
+grep '^configure.in:4:.*user variable.*INFO_DEPS' stderr
+grep 'overrides Automake variable.*INFO_DEPS' stderr
+
+$AUTOMAKE -Wno-override
+
+# There is only one definition of INFO_DEPS
+test 1 = `grep '^INFO_DEPS.*=' Makefile.in | wc -l`
+# and it is the right one.
+grep '^INFO_DEPS = @INFO_DEPS@$' Makefile.in