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> Cc: tracker <at> debbugs.gnu.org Subject: bug#7804: closed (Automake does not warn if AM_YFLAGS is conditionally extended) Date: Tue, 11 Jan 2011 00:00:04 +0000
[Message part 1 (text/plain, inline)]
Your message dated Tue, 11 Jan 2011 00:59:42 +0100 with message-id <201101110059.43709.stefano.lattarini <at> gmail.com> and subject line Re: [PATCH] yacc: warn about conditional content in *YFLAGS variables has caused the GNU bug report #7804, regarding Automake does not warn if AM_YFLAGS is conditionally extended to be marked as done. (If you believe you have received this mail in error, please contact help-debbugs <at> 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: 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
[Message part 3 (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 4 (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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.