Subject: bug#7804: closed (Re: [PATCH] yacc: warn about conditional content in *YFLAGS variables)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 11 Jan 2011 00:00:04 +0000

Your bug report

#7804: Automake does not warn if AM_YFLAGS is conditionally extended

which was filed against the automake package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
7804: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7804
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [PATCH] yacc: warn about conditional content in *YFLAGS variables
From: Stefano Lattarini <[email-masked]>
To: [email-masked]
Cc: [email-masked]
Date: Tue, 11 Jan 2011 00:59:42 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On Monday 10 January 2011, Stefano Lattarini wrote:
> On Monday 10 January 2011, Ralf Wildenhues wrote:
> > [ dropping bug-automake ]
> > 
> > * Stefano Lattarini wrote on Mon, Jan 10, 2011 at 03:59:10PM CET:
> > > The attached patch should fix the bug.  OK for the temporary branch
> > > 'yacc-work', to be merged to master afterwards?
> > 
> > OK, thanks.  Please add a short NEWS blurb (or merge with other NEWS
> > about yacc changes).
> >
> Oops, sorry!  I don't know I could forget to do so right away ... :-(
> 
> > How about ensuring in the test that the user can override
> > automake by setting warning flags suitably (preapproved)?
> >
> Good idea (I'll assume that he won't try to make `-d' conditional, as
> the behaviour should remain undefined in this case IMHO).  I'll amend
> the patch with a new testcase (tonight or tomorrow).
> 
Attached is what I pushed.  This should fix automake bug#7804.

Regards,
  Stefano
From 834dc3a98272e7eb1869e2a1972ac14ea45e0ec0 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <[email-masked]>
Date: Mon, 10 Jan 2011 15:50:35 +0100
Subject: [PATCH] yacc: warn about conditional content in *YFLAGS variables

This commit fixes automake bug#7804.

* automake.in (lang_yacc_target_hook): Warn if any of the relevant
*YFLAGS variables has conditional contents (not only a conditional
definition).  Related refactoring.
* NEWS: Updated.
* tests/yflags-conditional.test: Updated and extended.
* tests/yflags-conditional-force.test: New test.
* tests/Makefile.am (TESTS): Updated.
---
 ChangeLog                           |   12 ++++
 NEWS                                |    3 +
 automake.in                         |   34 +++++++---
 tests/Makefile.am                   |    1 +
 tests/Makefile.in                   |    1 +
 tests/yflags-conditional.test       |  117 ++++++++++++++++++++++++++++++++---
 tests/yflags-force-conditional.test |   95 ++++++++++++++++++++++++++++
 7 files changed, 243 insertions(+), 20 deletions(-)
 create mode 100755 tests/yflags-force-conditional.test

diff --git a/ChangeLog b/ChangeLog
index 06f9b84..90eb69e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2011-01-10   Stefano Lattarini  <[email-masked]>
+
+	yacc: warn about conditional content in *YFLAGS variables
+	This change fixes automake bug#7804.
+	* automake.in (lang_yacc_target_hook): Warn if any of the relevant
+	*YFLAGS variables has conditional contents (not only a conditional
+	definition).  Related refactoring.
+	* NEWS: Updated.
+	* tests/yflags-conditional.test: Updated and extended.
+	* tests/yflags-conditional-force.test: New test.
+	* tests/Makefile.am (TESTS): Updated.
+
 2011-01-08   Stefano Lattarini  <[email-masked]>
 
 	yacc: support variable expansions in *YFLAGS definition.
diff --git a/NEWS b/NEWS
index 79860ad..a947af9 100644
--- a/NEWS
+++ b/NEWS
@@ -62,6 +62,9 @@ Bugs fixed in 1.11.0a:
     through other variables, such as in:
       foo_opts = -d
       AM_YFLAGS = $(foo_opts)
+
+  - Automake now complains if a `*YFLAGS' variable has any conditional
+    content, not only a conditional definition.
 
 New in 1.11:
 
diff --git a/automake.in b/automake.in
index 2bffe48..fa458d6 100755
--- a/automake.in
+++ b/automake.in
@@ -6061,15 +6061,29 @@ sub lang_yacc_target_hook
 {
     my ($self, $aggregate, $output, $input, %transform) = @_;
 
-    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))
-    {
+    # If some relevant *YFLAGS variable contains the `-d' flag, we'll
+    # have to to generate special code.
+    my $yflags_contains_minus_d = 0;
+
+    foreach my $pfx ("", "${aggregate}_")
+      {
+	my $yflagsvar = var ("${pfx}YFLAGS");
+	next unless $yflagsvar;
+	# We cannot work reliably with conditionally-defined YFLAGS.
+	if ($yflagsvar->has_conditional_contents)
+	  {
+	    msg_var ('unsupported', $yflagsvar,
+	             "`${pfx}YFLAGS' cannot have conditional contents");
+	  }
+	else
+	  {
+	    $yflags_contains_minus_d = 1
+	      if grep (/^-d$/, $yflagsvar->value_as_list_recursive);
+	  }
+      }
+
+    if ($yflags_contains_minus_d)
+      {
 	(my $output_base = $output) =~ s/$KNOWN_EXTENSIONS_PATTERN$//;
 	my $header = $output_base . '.h';
 
@@ -6098,7 +6112,7 @@ sub lang_yacc_target_hook
 	# then we want to remove them with "make clean"; otherwise,
 	# "make distcheck" will fail.
 	$clean_files{$header} = $transform{'DIST_SOURCE'} ? MAINTAINER_CLEAN : CLEAN;
-    }
+      }
     # See the comment above for $HEADER.
     $clean_files{$output} = $transform{'DIST_SOURCE'} ? MAINTAINER_CLEAN : CLEAN;
 }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bb1d786..39f1a39 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -817,6 +817,7 @@ yflags-cmdline-override.test \
 yflags-conditional.test \
 yflags-d-false-positives.test \
 yflags-force-override.test \
+yflags-force-conditional.test \
 yflags-var-expand.test \
 $(parallel_tests)
 
diff --git a/tests/Makefile.in b/tests/Makefile.in
index e83cf33..0ea9825 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -1084,6 +1084,7 @@ yflags-cmdline-override.test \
 yflags-conditional.test \
 yflags-d-false-positives.test \
 yflags-force-override.test \
+yflags-force-conditional.test \
 yflags-var-expand.test \
 $(parallel_tests)
 
diff --git a/tests/yflags-conditional.test b/tests/yflags-conditional.test
index 8c673b1..91e3da4 100755
--- a/tests/yflags-conditional.test
+++ b/tests/yflags-conditional.test
@@ -14,7 +14,8 @@
 # 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.
+# Check that automake complains about *_YFLAGS variables which have
+# conditional content.
 
 . ./defs || Exit 1
 
@@ -22,25 +23,121 @@ set -e
 
 cat >> configure.in <<'END'
 AC_PROG_CC
-AC_PROG_YACC
+
+# `YFLAGS' is AC_SUBST'd by AC_PROG_YACC by default, but we
+# don't want this, since it might confuse our error messages.
+# Also, AM_SUBST_NOTMAKE seems not to help about this.
+# So we simply define $(YACC) by hand.
+AC_SUBST([YACC], [yacc])
+
 AM_CONDITIONAL([COND], [:])
 END
 
+$ACLOCAL
+
 cat > Makefile.am <<'END'
-bin_PROGRAMS = foo bar
+bin_PROGRAMS = foo zardoz
 foo_SOURCES = foo.y
-bar_SOURCES = bar.y
+zardoz_SOURCES = zardoz.y
 if COND
-AM_YFLAGS = $(YFLAGS)
-bar_YFLAGS = -v
+AM_YFLAGS = -v
+zardoz_YFLAGS = -v
 endif COND
 END
 
+cat > Makefile1.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+## dummy comment to keep line count right
+if COND
+YFLAGS = foo
+endif COND
+END
+
+cat > Makefile2.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+AM_YFLAGS = am_yflags
+if COND
+YFLAGS = yflags
+endif COND
+END
+
+cat > Makefile3.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+foo_YFLAGS = foo_yflags
+if COND
+YFLAGS = yflags
+endif COND
+END
+
+cat > Makefile4.am <<'END'
+bin_PROGRAMS = foo zardoz
+
+foo_SOURCES = foo.y
+zardoz_SOURCES = $(foo_SOURCES)
+
+YFLAGS =
+AM_YFLAGS = $(COND_VAR1)
+zardoz_YFLAGS = $(COND_VAR2:z=r)
+
+COND_VAR2 = foo
+if COND
+YFLAGS += -v
+COND_VAR2 += bar
+else !COND
+COND_VAR1 = -d
+endif !COND
+END
+
+cat > Makefile5.am <<'END'
+bin_PROGRAMS = foo zardoz
+foo_SOURCES = foo.y
+zardoz_SOURCES = zardoz.y
+YFLAGS = -v
+AM_YFLAGS = -v
+if COND
+zardoz_YFLAGS = -v
+endif
+END
+
+cat > Makefile6.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = foo.y
+foo_YFLAGS = -v
+if COND
+quux_YFLAGS = -v
+AM_YFLAGS = -v
+endif
+END
+
 : > ylwrap
 
-$ACLOCAL
-AUTOMAKE_fails
-grep "Makefile\.am:5:.*AM_YFLAGS.* defined conditionally" stderr
-grep "Makefile\.am:6:.*bar_YFLAGS.* defined conditionally" stderr
+LC_ALL=C; export LC_ALL  # For grep regexes below.
+
+AUTOMAKE_fails -Wnone -Wunsupported Makefile
+grep '^Makefile\.am:5:.*AM_YFLAGS.* conditional contents' stderr
+grep '^Makefile\.am:6:.*zardoz_YFLAGS.* conditional contents' stderr
+
+for i in 1 2 3; do
+  AUTOMAKE_fails -Wnone -Wunsupported Makefile$i
+  grep "^Makefile$i\\.am:5:.*[^a-zA-Z0-9_]YFLAGS.* conditional contents" \
+       stderr
+done
+
+AUTOMAKE_fails -Wnone -Wunsupported Makefile4
+grep '^Makefile4\.am:6:.*[^a-zA-Z0-9_]YFLAGS.* conditional contents' stderr
+grep '^Makefile4\.am:7:.*AM_YFLAGS.* conditional contents' stderr
+grep '^Makefile4\.am:8:.*zardoz_YFLAGS.* conditional contents' stderr
+
+# Now let's check we avoid false positives.
+
+# Disable `gnu' warnings because we override the user variable `YFLAGS'.
+AUTOMAKE_fails -Wno-gnu Makefile5
+grep -v '^Makefile5\.am:.*zardoz_YFLAGS' stderr | grep . && Exit 1
+
+# Disable `gnu' warnings because we override the user variable `YFLAGS'.
+$AUTOMAKE -Wno-gnu Makefile6
 
 :
diff --git a/tests/yflags-force-conditional.test b/tests/yflags-force-conditional.test
new file mode 100755
index 0000000..65f3197
--- /dev/null
+++ b/tests/yflags-force-conditional.test
@@ -0,0 +1,95 @@
+#! /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 the user can force automake to use *_YFLAGS variables
+# which have conditional content.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_YACC
+AM_CONDITIONAL([COND], [test x"$cond" = x"yes"])
+AC_OUTPUT
+END
+
+mkdir bin
+cat > bin/fake-yacc <<'END'
+#!/bin/sh
+echo "/* $* */" > y.tab.c
+echo 'extern int dummy;' >> y.tab.c
+END
+chmod a+x bin/fake-yacc
+PATH=`pwd`/bin$PATH_SEPARATOR$PATH; export PATH
+YACC=fake-yacc; export YACC
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo bar
+foo_SOURCES = foo.y main.c
+bar_SOURCES = $(foo_SOURCES)
+bar_YFLAGS = $(bar_yflags2)
+if COND
+AM_YFLAGS = __am_cond_yes__
+bar_YFLAGS += __bar_cond_yes__
+else !COND
+AM_YFLAGS = __am_cond_no__
+bar_yflags2 = __bar_cond_no__
+endif !COND
+END
+
+cat > main.c <<'END'
+int main (void)
+{
+  return 0;
+}
+END
+
+: > foo.y
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a -Wno-unsupported
+
+$EGREP '(YFLAGS|yflags|am__append)' Makefile.in # For debugging.
+
+./configure cond=yes
+$MAKE
+
+ls -l
+cat foo.c
+cat bar-foo.c
+
+$FGREP ' __am_cond_yes__ ' foo.c
+$FGREP ' __bar_cond_yes__ ' bar-foo.c
+$FGREP 'cond_no' foo.c bar-foo.c && Exit 1
+
+$MAKE maintainer-clean
+ls -l
+
+./configure cond=no
+$MAKE
+
+ls -l
+cat foo.c
+cat bar-foo.c
+
+$FGREP ' __am_cond_no__ ' foo.c
+$FGREP ' __bar_cond_no__ ' bar-foo.c
+$FGREP 'cond_yes' foo.c bar-foo.c && Exit 1
+
+:
-- 
1.7.2.3

Subject: Automake does not warn if AM_YFLAGS is conditionally extended
From: Stefano Lattarini <[email-masked]>
To: [email-masked]
Date: Fri, 7 Jan 2011 23:31:59 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello automakers.

Due to current implementation details, when dealing with Yacc sources, automake
must know the contents of the `$(AM_YFLAGS)' variable (or similar `$(foo_YFLAGS)'
variables) *statically and unconditionally* in order to always generate proper
code.

This is due to the special handling of the `-d' yacc flag; on this issue, read
at <http://www.gnu.org/software/automake/manual/html_node/Yacc-and-Lex.html>:
 ``AM_YFLAGS is usually used to pass the -d option to yacc. Automake knows what
   this means and will automatically adjust its rules to update and distribute
   the header file built by 'yacc -d'. ''

And while automake correctly warns if AM_YFLAGS is conditionally *defined*:

  $ cat configure.ac
  AC_INIT(x,0)
  AM_INIT_AUTOMAKE(foreign)
  AC_PROG_CC
  AC_PROG_YACC
  AC_CONFIG_FILES(Makefile)
  AM_CONDITIONAL(COND,:)
  $ aclocal
  $ cat > Makefile.am <<'END'
  bin_PROGRAMS = foo bar
  foo_SOURCES = foo.y
  bar_SOURCES = bar.y
  if COND
  AM_YFLAGS = -d
  endif
  END
  $ automake -a -Werror; echo status=$?
  Makefile.am:5: automake does not support AM_YFLAGS being defined conditionally
  status=1

it erronously doesn't warn if AM_YFLAGS is conditionally *extended*:

  $ cat configure.ac
  AC_INIT(x,0)
  AM_INIT_AUTOMAKE(foreign)
  AC_PROG_CC
  AC_PROG_YACC
  AC_CONFIG_FILES(Makefile)
  AM_CONDITIONAL(COND,:)
  $ aclocal
  $ cat > Makefile.am <<'END'
  bin_PROGRAMS = foo bar
  foo_SOURCES = foo.y
  bar_SOURCES = bar.y
  AM_YFLAGS =
  if COND
  AM_YFLAGS += -d
  endif
  END
  $ automake -a -Werror; echo status=$?
  status=0

I think this bug shouldn't be difficult to fix, and I'll attempt a fix
soonish; but as usual, having it in the bug tracker doesn't hurt IMHO.

Regards,
  Stefano