Allow variable override of, rather than by, silent variables. (was: bfdlibdir and bfdincludedir change)

[Ralf Wildenhues <Ralf dot Wildenhues at gmx dot de>]

<http://thread.gmane.org/gmane.comp.gnu.binutils/43160/focus=43207>

* Ralf Wildenhues wrote on Wed, Aug 26, 2009 at 09:04:42PM CEST:
>
[ binutils/bfd/Makefile.am ]
> However, the Makefile.am files
> as they are now will currently cause automake to warn unconditionally:
> 
> $ automake
> Makefile.am:18: bfdlibdir was already defined in condition TRUE, which includes condition INSTALL_LIBBFD ...
> configure.in:137: ... `bfdlibdir' previously defined here
> Makefile.am:19: bfdincludedir was already defined in condition TRUE, which includes condition INSTALL_LIBBFD ...
> configure.in:137: ... `bfdincludedir' previously defined here
> Makefile.am:24: bfdlibdir was already defined in condition TRUE, which includes condition !INSTALL_LIBBFD ...
> Makefile.am:18: ... `bfdlibdir' previously defined here
> Makefile.am:25: bfdincludedir was already defined in condition TRUE, which includes condition !INSTALL_LIBBFD ...
> Makefile.am:19: ... `bfdincludedir' previously defined here
> 
> (-Wno-override won't help here).  I can look into fixing the warning in
> a future automake sometime, but for the moment, you'd have to live with
> such warnings in opcodes/ and bfd/.  (The warning is wrong when
> AM_SUBST_NOTMAKE is used on these variables, as that prevents the
> 'foo = @foo@' line in Makefile.in from being produced by automake.)

Fixing this bug in Automake branch-1.11 and master as follows.

Cheers,
Ralf

    Allow variable override of, rather than by, silent variables.
    
    When overriding variables, check the VAR_SILENT status of the
    old condition, not the new one: we allow non-silent overrides
    of silent variables (these do not hit ordering bugs in the
    automake output), but there is no use case for doing so the
    other way round.  Fixes conditional overrides of variables
    annotated AM_SUBST_NOTMAKE in configure.ac.
    
    * lib/Automake/Variable.pm (_check_ambiguous_condition): Check
    for VAR_SILENT in $ambig_cond not $cond.
    * tests/cond44.test, tests/cond45.test: New tests, analogous to
    cond23.test and cond24.test.
    * tests/Makefile.am: Update.
    Spotted originally in binutils/bfd.

diff --git a/lib/Automake/Variable.pm b/lib/Automake/Variable.pm
index 94c0880..e7fa6bb 100644
--- a/lib/Automake/Variable.pm
+++ b/lib/Automake/Variable.pm
@@ -472,8 +472,9 @@ sub _check_ambiguous_condition ($$$)
   my $var = $self->name;
   my ($message, $ambig_cond) = $self->conditions->ambiguous_p ($var, $cond);
 
-  # We allow silent variables to be overridden silently.
-  my $def = $self->def ($cond);
+  # We allow silent variables to be overridden silently,
+  # by either silent or non-silent variables.
+  my $def = $self->def ($ambig_cond);
   if ($message && !($def && $def->pretty == VAR_SILENT))
     {
       msg 'syntax', $where, "$message ...", partial => 1;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bc8dbb2..15325e4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -185,6 +185,8 @@ cond40.test \
 cond41.test \
 cond42.test \
 cond43.test \
+cond44.test \
+cond45.test \
 condd.test \
 condhook.test \
 condinc.test \
diff --git a/tests/cond44.test b/tests/cond44.test
new file mode 100755
index 0000000..ac907f1
--- /dev/null
+++ b/tests/cond44.test
@@ -0,0 +1,40 @@
+#!/bin/sh
+# Copyright (C) 2002, 2003, 2009 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program 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 this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that redefinitions of AC_SUBST'ed AM_SUBST_NOTMAKE'd variables
+# are not diagnosed.  See cond23.test.
+
+. ./defs
+
+set -e
+
+cat >>configure.in <<EOF
+AM_CONDITIONAL([COND], [true])
+AM_SUBST_NOTMAKE([libdir])
+AC_OUTPUT
+EOF
+
+cat >Makefile.am <<EOF
+if COND
+libdir = mumble
+endif
+EOF
+
+$ACLOCAL
+AUTOMAKE_run 0
+grep 'libdir was already defined' stderr && Exit 1
+grep '^libdir = ' Makefile.in && Exit 1
+Exit 0
diff --git a/tests/cond45.test b/tests/cond45.test
new file mode 100755
index 0000000..57dd339
--- /dev/null
+++ b/tests/cond45.test
@@ -0,0 +1,41 @@
+#!/bin/sh
+# Copyright (C) 2002, 2003, 2009 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program 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 this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that redefinitions of AC_SUBST'ed AM_SUBST_NOTMAKE'd variables
+# are not diagnosed.  See cond24.test.
+
+. ./defs
+
+set -e
+
+cat >>configure.in <<EOF
+AC_SUBST([foo], [bar])
+AM_SUBST_NOTMAKE([foo])
+AM_CONDITIONAL([COND], [true])
+AC_OUTPUT
+EOF
+
+cat >Makefile.am <<EOF
+if COND
+foo = baz
+endif
+EOF
+
+$ACLOCAL
+AUTOMAKE_run 0
+grep 'foo was already defined' stderr && Exit 1
+grep '^foo =' Makefile.in && Exit 1
+Exit 0