Re: New snprintfv on CVS

[Alexandre Duret-Lutz]

Hi Bruce,

Happy new year.

>>> "Bruce" == Bruce Korb <address@hidden> writes:

[...]

 >> /usr/local/share/automake-1.8/am/texibuild.am: INFO_DEPS was set with `=' 
 >> and is now set with `+='

[...]

Thanks for the report.  

Automake has an internal check that ensures every internal
variable is defined consistently with the same sign (=, :=, or
+=).  However this check assumes that if automake tries to touch
a variable, then that variable is owned by Automake.  This is
wrong: in your case you have set INFO_DEPS so it has become a
user variables and Automake should not try to do anything with
it.

You hit this bug because the way INFO_DEPS is defined has
changed in 1.8, but in fact it's an old bug that dates from
Automake 1.5 and that nobody noticed so far.  I'm glad you did.
(You can reproduce it in 1.5 if you try to override DIST_COMMON.)

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

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

        * 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.  Ignore Automake attempts to override a variable for
        += assignments too, not only := and =.
        * tests/txinfo29.test: New file.
        * tests/Makefile.am (TESTS): Add txinfo29.test.
        Reported by Bruce Korb.

Index: lib/Automake/Variable.pm
===================================================================
RCS file: /cvs/automake/automake/lib/Automake/Variable.pm,v
retrieving revision 1.27
diff -u -r1.27 Variable.pm
--- lib/Automake/Variable.pm    8 Dec 2003 18:00:02 -0000       1.27
+++ lib/Automake/Variable.pm    3 Jan 2004 12:35:40 -0000
@@ -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 @@
                               || $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 @@
   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 @@
   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,64 +956,30 @@
                       $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
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.541
diff -u -r1.541 Makefile.am
--- tests/Makefile.am   2 Jan 2004 15:55:17 -0000       1.541
+++ tests/Makefile.am   3 Jan 2004 12:35:41 -0000
@@ -495,6 +495,7 @@
 txinfo26.test \
 txinfo27.test \
 txinfo28.test \
+txinfo29.test \
 transform.test \
 unused.test \
 vars.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.702
diff -u -r1.702 Makefile.in
--- tests/Makefile.in   2 Jan 2004 15:55:17 -0000       1.702
+++ tests/Makefile.in   3 Jan 2004 12:35:41 -0000
@@ -609,6 +609,7 @@
 txinfo26.test \
 txinfo27.test \
 txinfo28.test \
+txinfo29.test \
 transform.test \
 unused.test \
 vars.test \
Index: tests/txinfo29.test
===================================================================
RCS file: tests/txinfo29.test
diff -N tests/txinfo29.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/txinfo29.test 3 Jan 2004 12:35:41 -0000
@@ -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 = @address@hidden' Makefile.in
-- 
Alexandre Duret-Lutz