bug#7800: [PATCHES] yacc: support variable expansions in *YFLAGS definition (was: Re: bug#7800: automake fails to honor `-d' in AM_YFLAGS when variable expansions are involved)

[Stefano Lattarini]

Reference:
 <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7800>

On Friday 07 January 2011, Ralf Wildenhues wrote:
> Hi Stefano,
> 
> thanks for the report.
> 
> * Stefano Lattarini wrote on Fri, Jan 07, 2011 at 03:36:43PM CET:
> > Currently, automake is not smart enough to resolve variable expansions
> > in AM_YFLAGS (or foo_YFLAGS) when scanning them for the `-d' flag.
> 
> value_as_list_recursive can usually help here.
> 
> > Not sure if this bug is worth fixing, but having it reported in the bug
> > database won't hurt, either (and I might anyway attempt a fix soonish).
> 
> The usual complication is what to do if
> - the variable is overridden at 'make' time (automake usually punts in
>   that case),
> - the variable is conditionally set (this usually requires making the
>   automake code more complex to solve).
> 
> Punting also for the latter is fine if the feature is expected to be
> little-used.
> 
> Cheers,
> Ralf
> 

The attached two-patch series should fix the bug (the first patch
is a testsuite enhancement, the second one really fixes the bug).

OK to apply to the temporary branch 'yacc-clean' (recently merged into
master with commit v1.11-575-ga297a16) and merge to master again?

Regards,
   Stefano

From 6dc4c48587164bbf6784acddbfe5065b283fe190 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Fri, 7 Jan 2011 20:52:17 +0100
Subject: [PATCH 1/2] tests: more on *YFLAGS support

* tests/yflags-var-expand.test: New test, still xfailing.  It
exposes automake bug#7800 -- "automake fails to honor `-d' in
AM_YFLAGS when variable expansions are involved".
* tests/yflags-d-false-positive.test: New test, checking that
automake do not spuriously see `-d' in *YFLAGS when that isn't
really there.
* tests/yflags-force-override.test: New test, checking that
automake can cope with definition of the YFLAGS variable in
Makefile.am (even if that is an extremely bad practice, as that
variable is user-reserved).
* tests/yflags-cmdline-override.test: New test, checking that
automake can cope with user-redefinition of YFLAGS at configure
time and/or at make time.
* tests/yflags-conditional.test: New test, checks that automake
warns on conditionally-defined *YFLAGS variables.
* tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
---
 ChangeLog                           |   20 ++++++++
 tests/Makefile.am                   |    6 ++
 tests/Makefile.in                   |    6 ++
 tests/yflags-cmdline-override.test  |   88 +++++++++++++++++++++++++++++++++++
 tests/yflags-conditional.test       |   46 ++++++++++++++++++
 tests/yflags-d-false-positives.test |   42 +++++++++++++++++
 tests/yflags-force-override.test    |   64 +++++++++++++++++++++++++
 tests/yflags-var-expand.test        |   63 +++++++++++++++++++++++++
 8 files changed, 335 insertions(+), 0 deletions(-)
 create mode 100755 tests/yflags-cmdline-override.test
 create mode 100755 tests/yflags-conditional.test
 create mode 100755 tests/yflags-d-false-positives.test
 create mode 100755 tests/yflags-force-override.test
 create mode 100755 tests/yflags-var-expand.test

diff --git a/ChangeLog b/ChangeLog
index 2482bbf..143109b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,25 @@
 2011-01-07   Stefano Lattarini  <address@hidden>
 
+       tests: more on *YFLAGS support
+       * tests/yflags-var-expand.test: New test, still xfailing.  It
+       exposes automake bug#7800 -- "automake fails to honor `-d' in
+       AM_YFLAGS when variable expansions are involved".
+       * tests/yflags-d-false-positive.test: New test, checking that
+       automake do not spuriously see `-d' in *YFLAGS when that isn't
+       really there.
+       * tests/yflags-force-override.test: New test, checking that
+       automake can cope with definition of the YFLAGS variable in
+       Makefile.am (even if that is an extremely bad practice, as that
+       variable is user-reserved).
+       * tests/yflags-cmdline-override.test: New test, checking that
+       automake can cope with user-redefinition of YFLAGS at configure
+       time and/or at make time.
+       * tests/yflags-conditional.test: New test, checks that automake
+       warns on conditionally-defined *YFLAGS variables.
+       * tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
+
+2011-01-07   Stefano Lattarini  <address@hidden>
+
        yacc: "make clean" removes .c and .h files from non-distributed .y
        Previously, while automake did *not* distribute C source and header
        files derived from non-distributed Yacc sources, it still caused
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bed5e41..0caa8d2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,6 +21,7 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
+yflags-var-expand.test \
 txinfo5.test
 
 include $(srcdir)/parallel-tests.am
@@ -807,6 +808,11 @@ yaccpp.test \
 yaccvpath.test \
 yflags.test \
 yflags2.test \
+yflags-cmdline-override.test \
+yflags-conditional.test \
+yflags-d-false-positives.test \
+yflags-force-override.test \
+yflags-var-expand.test \
 $(parallel_tests)
 
 EXTRA_DIST = ChangeLog-old gen-parallel-tests $(TESTS)
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 77e9a38..94fd7b2 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -277,6 +277,7 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
+yflags-var-expand.test \
 txinfo5.test
 
 parallel_tests = \
@@ -1074,6 +1075,11 @@ yaccpp.test \
 yaccvpath.test \
 yflags.test \
 yflags2.test \
+yflags-cmdline-override.test \
+yflags-conditional.test \
+yflags-d-false-positives.test \
+yflags-force-override.test \
+yflags-var-expand.test \
 $(parallel_tests)
 
 EXTRA_DIST = ChangeLog-old gen-parallel-tests $(TESTS)
diff --git a/tests/yflags-cmdline-override.test 
b/tests/yflags-cmdline-override.test
new file mode 100755
index 0000000..baedf68
--- /dev/null
+++ b/tests/yflags-cmdline-override.test
@@ -0,0 +1,88 @@
+#! /bin/sh
+# Copyright (C) 2011 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 automake can cope with user-redefinition of $(YFLAGS)
+# at configure time and/or at make time.
+
+. ./defs || Exit 1
+
+set -e
+
+unset YFLAGS || :
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_YACC
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+# A minor automake wart: automake doesn't generate the code to clean
+# `*.output' files generated by yacc (it's not even clear if that would
+# be useful in general, so it's probably better to be conservative).
+CLEANFILES = foo.output
+# Another automake wart: `-d' flag won't be given at automake time, so
+# automake won't be able to generate the code to clean `foo.h' :-(
+MAINTAINERCLEANFILES = foo.h
+END
+
+cat > foo.y << 'END'
+%{
+int yylex () { return 0; }
+void yyerror (char *s) { return; }
+int main () { return 0; }
+%}
+%%
+foobar : 'f' 'o' 'o' 'b' 'a' 'r' {};
+END
+
+$ACLOCAL
+$AUTOMAKE -a
+$AUTOCONF
+
+./configure YFLAGS='-d -v'
+$MAKE
+ls -l
+test -f foo.c
+test -f foo.h
+test -f foo.output
+
+$MAKE maintainer-clean
+ls -l
+
+./configure YFLAGS='-v'
+$MAKE
+ls -l
+test -f foo.c
+test ! -r foo.h
+test -f foo.output
+
+$MAKE maintainer-clean
+ls -l
+
+./configure YFLAGS='-v'
+YFLAGS=-d $MAKE -e
+ls -l
+test -f foo.c
+test -f foo.h
+test ! -r foo.output
+
+$MAKE maintainer-clean
+ls -l
+
+:
diff --git a/tests/yflags-conditional.test b/tests/yflags-conditional.test
new file mode 100755
index 0000000..8c673b1
--- /dev/null
+++ b/tests/yflags-conditional.test
@@ -0,0 +1,46 @@
+#! /bin/sh
+# Copyright (C) 2011 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 automake complains about conditionally-defined *_YFLAGS.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_YACC
+AM_CONDITIONAL([COND], [:])
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo bar
+foo_SOURCES = foo.y
+bar_SOURCES = bar.y
+if COND
+AM_YFLAGS = $(YFLAGS)
+bar_YFLAGS = -v
+endif COND
+END
+
+: > ylwrap
+
+$ACLOCAL
+AUTOMAKE_fails
+grep "Makefile\.am:5:.*AM_YFLAGS.* defined conditionally" stderr
+grep "Makefile\.am:6:.*bar_YFLAGS.* defined conditionally" stderr
+
+:
diff --git a/tests/yflags-d-false-positives.test 
b/tests/yflags-d-false-positives.test
new file mode 100755
index 0000000..c3d5ecb
--- /dev/null
+++ b/tests/yflags-d-false-positives.test
@@ -0,0 +1,42 @@
+#! /bin/sh
+# Copyright (C) 2011 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 for false positives in automake recognition of `-d' in YFLAGS.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_YACC
+END
+
+$ACLOCAL
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = zardoz foobar
+zardoz_SOURCES = zardoz.y
+foobar_SOURCES = foobar.y
+AM_YFLAGS = -xd --d - d --output=d
+foobar_YFLAGS = - d $(foovar)-d -dd
+END
+
+$AUTOMAKE -a
+$EGREP '(foobar|zardoz)\.h.*:' Makefile.in && Exit 1
+$EGREP '(foobar|zardoz)\.h' Makefile.in | $FGREP -v '$(YLWRAP) ' && Exit 1
+
+:
diff --git a/tests/yflags-force-override.test b/tests/yflags-force-override.test
new file mode 100755
index 0000000..0b0133e
--- /dev/null
+++ b/tests/yflags-force-override.test
@@ -0,0 +1,64 @@
+#! /bin/sh
+# Copyright (C) 2011 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 automake can cope with definition of the $(YFLAGS) variable
+# in Makefile.am (even if that is an extremely bad practice, because that
+# variable is user-reserved).
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_YACC
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+YFLAGS = -d -v
+END
+
+cat > foo.y << 'END'
+%{
+int yylex () { return 0; }
+void yyerror (char *s) { return; }
+int main () { return 0; }
+%}
+%%
+foobar : 'f' 'o' 'o' 'b' 'a' 'r' {};
+END
+
+$ACLOCAL
+$AUTOMAKE -a -Wno-gnu
+
+$EGREP '(foo|YFLAGS)' Makefile.in # for debugging
+grep '^foo.h *:' Makefile.in
+
+$AUTOCONF
+./configure
+
+$MAKE
+
+test -f foo.c
+test -f foo.h
+test -f foo.output
+
+$MAKE distcheck
+
+:
diff --git a/tests/yflags-var-expand.test b/tests/yflags-var-expand.test
new file mode 100755
index 0000000..1967be3
--- /dev/null
+++ b/tests/yflags-var-expand.test
@@ -0,0 +1,63 @@
+#! /bin/sh
+# Copyright (C) 2011 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 automake expand variables when looking for `-d' in YFLAGS;
+# for example, the following is supposed to work:
+#  foo_flags = -d
+#  AM_YFLAGS = $(foo_flags)
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_YACC
+END
+
+$ACLOCAL
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo bar
+foo_SOURCES = foo.y
+bar_SOURCES = bar.y
+my_YFLAGS = -x
+AM_YFLAGS = $(my_YFLAGS:x=d)
+bar_YFLAGS = $(AM_YFLAGS)
+END
+
+$AUTOMAKE -a
+
+$EGREP '(foo|bar|YFLAGS)' Makefile.in # for debugging
+grep '^foo.h *:' Makefile.in
+grep '^bar-bar.h *:' Makefile.in
+
+cat > Makefile.am <<'END'
+AUTOMAKE_OPTIONS = -Wno-gnu
+bin_PROGRAMS = zardoz
+zardoz_SOURCES = parser.y
+my_YFLAGS = $(my_YFLAGS_1)
+my_YFLAGS += $(my_YFLAGS_2)
+my_YFLAGS_2 = -d
+YFLAGS = $(my_YFLAGS)
+END
+
+$AUTOMAKE
+
+$EGREP 'parser|YFLAGS' Makefile.in # for debugging
+grep '^parser.h *:' Makefile.in
+
+:
-- 
1.7.2.3

From 312acec17badbf428abf3961e081004c721b5323 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Fri, 7 Jan 2011 21:52:56 +0100
Subject: [PATCH 2/2] yacc: support variable expansions in *YFLAGS definition.

This commit fixes automake bug#7800.

* automake.in (lang_yacc_target_hook): Use 'value_as_list_recursive'
instead of 'variable_value' to get the value of *YFLAGS variables.
Related changes.
($DASH_D_PATTERN): Removed as obsolete.
* tests/Makefile.am (XFAIL_TESTS): Remove yflags-var-expand.test.
* tests/yacc-clean.test: Remove a now-useless workaround.
* NEWS: Update.
---
 ChangeLog             |   12 ++++++++++++
 NEWS                  |    6 ++++++
 automake.in           |   15 ++++++++-------
 tests/Makefile.am     |    1 -
 tests/Makefile.in     |    1 -
 tests/yacc-clean.test |    4 ----
 6 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 143109b..316ea61 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2011-01-07   Stefano Lattarini  <address@hidden>
 
+       yacc: support variable expansions in *YFLAGS definition.
+       This change fixes automake bug#7800.
+       * automake.in (lang_yacc_target_hook): Use 'value_as_list_recursive'
+       instead of 'variable_value' to get the value of *YFLAGS variables.
+       Related changes.
+       ($DASH_D_PATTERN): Removed as obsolete.
+       * tests/Makefile.am (XFAIL_TESTS): Remove yflags-var-expand.test.
+       * tests/yacc-clean.test: Remove a now-useless workaround.
+       * NEWS: Update.
+
+2011-01-07   Stefano Lattarini  <address@hidden>
+
        tests: more on *YFLAGS support
        * tests/yflags-var-expand.test: New test, still xfailing.  It
        exposes automake bug#7800 -- "automake fails to honor `-d' in
diff --git a/NEWS b/NEWS
index 0575935..1c489fc 100644
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,12 @@ Bugs fixed in 1.11.0a:
   - The code for automatic dependency tracking works around a Solaris
     make bug triggered by sources containing repeated slashes when the
     `subdir-objects' option was used.
+
+  - Automake is now smart enough to detect the presence of the `-d' flag
+    in the various `*YFLAGS' variables even when their definitions involve
+    indirections through other variables, such as in:
+      foo_opts = -d
+      AM_YFLAGS = $(foo_opts)
 
 New in 1.11:
 
diff --git a/automake.in b/automake.in
index b97d4a6..2bffe48 100755
--- a/automake.in
+++ b/automake.in
@@ -208,8 +208,6 @@ my $INCLUDE_PATTERN = ('^include\s+'
                       . '|(\$\(srcdir\)/' . $PATH_PATTERN . ')'
                       . '|([^/\$]' . $PATH_PATTERN . '))\s*(#.*)?' . "\$");
 
-# Match `-d' as a command-line argument in a string.
-my $DASH_D_PATTERN = "(^|\\s)-d(\\s|\$)";
 # Directories installed during 'install-exec' phase.
 my $EXEC_DIR_PATTERN =
   '^(?:bin|sbin|libexec|sysconf|localstate|lib|pkglib|.*exec.*)' . "\$";
@@ -6063,11 +6061,14 @@ sub lang_yacc_target_hook
 {
     my ($self, $aggregate, $output, $input, %transform) = @_;
 
-    my $flag = $aggregate . "_YFLAGS";
-    my $flagvar = var $flag;
-    my $YFLAGSvar = var 'YFLAGS';
-    if (($flagvar && $flagvar->variable_value =~ /$DASH_D_PATTERN/o)
-       || ($YFLAGSvar && $YFLAGSvar->variable_value =~ /$DASH_D_PATTERN/o))
+    my $flagvar = var ($aggregate . "_YFLAGS");
+    my $YFLAGSvar = var ('YFLAGS');
+    # We cannot work reliably with conditionally-defined YFLAGS.
+    $flagvar->check_defined_unconditionally if $flagvar;
+    $YFLAGSvar->check_defined_unconditionally if $YFLAGSvar;
+    my @flags = $flagvar ? $flagvar->value_as_list_recursive : ();
+    my @YFLAGS = $YFLAGSvar ? $YFLAGSvar->value_as_list_recursive : ();
+    if (grep (/^-d$/, @flags) || grep (/^-d$/, @YFLAGS))
     {
        (my $output_base = $output) =~ s/$KNOWN_EXTENSIONS_PATTERN$//;
        my $header = $output_base . '.h';
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0caa8d2..b4b2576 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,7 +21,6 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
-yflags-var-expand.test \
 txinfo5.test
 
 include $(srcdir)/parallel-tests.am
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 94fd7b2..586a2a0 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -277,7 +277,6 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
-yflags-var-expand.test \
 txinfo5.test
 
 parallel_tests = \
diff --git a/tests/yacc-clean.test b/tests/yacc-clean.test
index a8df065..009bf97 100755
--- a/tests/yacc-clean.test
+++ b/tests/yacc-clean.test
@@ -62,10 +62,6 @@ END
 cat > sub2/Makefile.am << 'END'
 include $(top_srcdir)/sub1/Makefile.am
 AM_YFLAGS = -d
-## FIXME: these apparently redundant definitions are required to
-## work around automake bug#7800.
-bar_YFLAGS += -d
-qux_YFLAGS += -d
 END
 
 cat > sub1/parse.y << 'END'
-- 
1.7.2.3