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: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Stefano Lattarini <stefano.lattarini <at> gmail.com> Subject: bug#7804: closed (Re: [PATCH] yacc: warn about conditional content in *YFLAGS variables) Date: Tue, 11 Jan 2011 00:00:04 +0000
[Message part 1 (text/plain, inline)]
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 7804 <at> debbugs.gnu.org. -- 7804: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7804 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Stefano Lattarini <stefano.lattarini <at> gmail.com> To: automake-patches <at> gnu.org Cc: 7804-done <at> debbugs.gnu.org Subject: Re: [PATCH] yacc: warn about conditional content in *YFLAGS variables Date: Tue, 11 Jan 2011 00:59:42 +0100[Message part 3 (text/plain, inline)]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[0001-yacc-warn-about-conditional-content-in-YFLAGS-variab.patch (text/x-patch, inline)]From 834dc3a98272e7eb1869e2a1972ac14ea45e0ec0 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 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 <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. + * 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 <stefano.lattarini <at> gmail.com> 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
[Message part 5 (message/rfc822, inline)]
From: Stefano Lattarini <stefano.lattarini <at> gmail.com> To: bug-automake <at> gnu.org Subject: Automake does not warn if AM_YFLAGS is conditionally extended Date: Fri, 7 Jan 2011 23:31:59 +0100Hello 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.