Package: automake;
Reported by: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Date: Fri, 7 Jan 2011 22:26:01 UTC
Severity: normal
Tags: patch
Done: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Stefano Lattarini <stefano.lattarini <at> gmail.com> To: 7804 <at> debbugs.gnu.org Cc: automake-patches <at> gnu.org Subject: bug#7804: [PATCH] yacc: warn about conditional content in *YFLAGS variables (was: bug#7804: Automake does not warn if AM_YFLAGS is conditionally extended) Date: Mon, 10 Jan 2011 15:59:10 +0100
[Message part 1 (text/plain, inline)]
On Friday 07 January 2011, Stefano Lattarini wrote: > 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 > The attached patch should fix the bug. OK for the temporary branch 'yacc-work', to be merged to master afterwards? Regards, Stefano
[0001-yacc-warn-about-conditional-content-in-YFLAGS-variab.patch (text/x-patch, inline)]
From 2ffb8db20b74a319e63fe2d8eda12a075117272b Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Date: Mon, 10 Jan 2011 15:50:35 +0100
Subject: [PATCH] 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.
* tests/yflags-conditional.test: Updated and extended.
---
ChangeLog | 9 +++
automake.in | 34 ++++++++----
tests/yflags-conditional.test | 117 +++++++++++++++++++++++++++++++++++++----
3 files changed, 140 insertions(+), 20 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 06f9b84..e40f608 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-01-10 Stefano Lattarini <stefano.lattarini <at> gmail.com>
+
+ 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.
+ * tests/yflags-conditional.test: Updated and extended.
+
2011-01-08 Stefano Lattarini <stefano.lattarini <at> gmail.com>
yacc: support variable expansions in *YFLAGS definition.
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/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
:
--
1.7.2.3
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.