bug#7333: [PATCH] {maint} Fix a bug in variable concatanation with `+='. (was: Re: bug#7333: bug concatenating CLEANFILES in automake 1.11)
On Friday 05 November 2010, Stefano Lattarini wrote:
I can confirm the bug with latest automake (from git master), with
a much-reduced minimal testcase (see attachment).
I still haven't looked for an explanation or a fix, though.
I've manged to find a very simple fix for the bug (see attached patch).
OK to apply to maint?
Regards,
Stefano
From b8de299295e081909c6d0a8a1cef957b337e3732 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Sat, 6 Nov 2010 12:46:52 +0100
Subject: [PATCH] Fix a bug in variable concatanation with `+='.
* lib/Automake/VarDef.pm (append): Remove extra backslash-escaped
newlines from the end of the variable's content, before appending
to it.
* tests/pluseq11.test: New test, exposing the bug.
* tests/Makefile.am (TESTS): Update.
Reported by Andy Wingo.
---
ChangeLog | 10
lib/Automake/VarDef.pm | 13 +-
tests/Makefile.am |1 +
tests/Makefile.in |1 +
tests/pluseq11.test| 54
5 files changed, 68 insertions(+), 11 deletions(-)
create mode 100755 tests/pluseq11.test
diff --git a/ChangeLog b/ChangeLog
index 6c17cd3..a928363 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2010-11-06 Stefano Lattarini stefano.lattar...@gmail.com
+
+ Fix a bug in variable concatanation with `+='.
+ * lib/Automake/VarDef.pm (append): Remove extra backslash-escaped
+ newlines from the end of the variable's content, before appending
+ to it.
+ * tests/pluseq11.test: New test, exposing the bug.
+ * tests/Makefile.am (TESTS): Update.
+ Reported by Andy Wingo.
+
2010-11-01 Ralf Wildenhues ralf.wildenh...@gmx.de
Fix and document rules to not touch the tree with `make -n'.
diff --git a/lib/Automake/VarDef.pm b/lib/Automake/VarDef.pm
index d7ba155..568c82a 100644
--- a/lib/Automake/VarDef.pm
+++ b/lib/Automake/VarDef.pm
@@ -185,17 +185,8 @@ sub append ($$$)
# Furthermore keeping `#' would not be portable if the variable is
# output on multiple lines.
$val =~ s/ ?#.*//;
-
- if (chomp $val)
-{
- # Insert a backslash before a trailing newline.
- $val .= \\\n;
-}
- elsif ($val)
-{
- # Insert a separator.
- $val .= ' ';
-}
+ # Insert a separator, if required.
+ $val .= ' ' if $val;
$self-{'value'} = $val . $value;
# Turn ASIS appended variables into PRETTY variables. This is to
# cope with `make' implementation that cannot read very long lines.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9c81564..da81c49 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -570,6 +570,7 @@ pluseq7.test \
pluseq8.test \
pluseq9.test \
pluseq10.test \
+pluseq11.test \
postproc.test \
ppf77.test \
pr2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index b568a09..eb461a9 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -837,6 +837,7 @@ pluseq7.test \
pluseq8.test \
pluseq9.test \
pluseq10.test \
+pluseq11.test \
postproc.test \
ppf77.test \
pr2.test \
diff --git a/tests/pluseq11.test b/tests/pluseq11.test
new file mode 100755
index 000..293270f
--- /dev/null
+++ b/tests/pluseq11.test
@@ -0,0 +1,54 @@
+#!/bin/sh
+# Copyright (C) 2010 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 bug in variable concatenation with `+=': an extra backslash
+# is erroneously retained in the final value.
+# See also sister test pluseq11b.test.
+
+. ./defs || Exit 1
+
+set -e
+
+cat configure.in 'END'
+AC_OUTPUT
+END
+
+cat Makefile.am 'END'
+## Use more line continuation to ensure we are robust and can (hopefully)
+## cope any number of them, and not just one
+FOO = \
+\
+\
+bar
+## Both this two variable additions are required to trigger the bug.
+FOO +=
+FOO += baz
+
+.PHONY: test
+test:
+ case '$(FOO)' in *\\*) exit 1;; *) exit 0;; esac
+END
+
+$ACLOCAL
+$AUTOMAKE
+
+grep '^ *FOO *=.*\\.' Makefile.in Exit 1
+
+$AUTOCONF
+./configure
+$MAKE test
+
+:
--
1.7.1
bug#7336: Failure on test parallel-tests5.test
* Lluís Batlle i Rossell wrote on Fri, Nov 05, 2010 at 01:36:28PM CET: On Mon, Nov 01, 2010 at 07:41:12PM +0100, Lluís Batlle i Rossell wrote: On Mon, Nov 01, 2010 at 07:07:48PM +0100, Ralf Wildenhues wrote: [ http://thread.gmane.org/gmane.comp.sysutils.automake.bugs/5061 ] Ralf says that something changed about syscall restarting in linux 2.6.36, so it can be a problem in linux. I hope he will investigate that and keep all of us aware. I'll try linux 2.6.35 meanwhile. Fixed! Ralf and Al Viro found the kernel problem, made evident in the release 2.6.36 for mips only. Fixed here: http://git.linux-mips.org/?p=linux.git;a=commit;h=915bcf007cbf9370fa0f3a57b27cf7fdc45b3964 I reported to automake because it was the very only part of the system failing. Thanks for reporting and having this tracked down, I'm closing the Automake bug. Cheers, Ralf
Re: More problems with `make -n' in automake-generated rules.
Hi Ralf, I've just spotted a bug in the patch ...
On Monday 01 November 2010, Ralf Wildenhues wrote:
diff --git a/automake.in b/automake.in
index cb5fe24..42eff2b 100644
--- a/automake.in
+++ b/automake.in
@@ -6070,11 +6070,11 @@ sub lang_vala_finish_target ($$)
'--vapi', '--internal-vapi', '--gir')))
{
my $headerfile = $flag;
- $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n.
- \...@if test -f \$@; then :; else \\\n.
- \t rm -f \$(srcdir)/${derived}_vala.stamp; \\\n.
- \t \$(am__cd) \$(srcdir) \$(MAKE) \$(AM_MAKEFLAGS)
${derived}_vala.stamp; \\\n.
- \tfi\n;
+ $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n
+ . \...@if test -f \$@; then :; else rm -f
\$(srcdir)/${derived}_vala.stamp; \n
... here (missing fi). It causes a failure in `vala2.test'.
I'll install a fix later if you don't beat me (right now I'm doing other
testing, and prefer not to be sidetracked by this issue).
Regards,
Stefano
Re: More problems with `make -n' in automake-generated rules.
Hi Stefano,
* Stefano Lattarini wrote on Sat, Nov 06, 2010 at 05:52:57PM CET:
Hi Ralf, I've just spotted a bug in the patch ...
- $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n.
- \...@if test -f \$@; then :; else \\\n.
- \t rm -f \$(srcdir)/${derived}_vala.stamp; \\\n.
- \t \$(am__cd) \$(srcdir) \$(MAKE) \$(AM_MAKEFLAGS)
${derived}_vala.stamp; \\\n.
- \tfi\n;
+ $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n
+ . \...@if test -f \$@; then :; else rm -f
\$(srcdir)/${derived}_vala.stamp; \n
... here (missing fi). It causes a failure in `vala2.test'.
I'll install a fix later if you don't beat me (right now I'm doing other
testing, and prefer not to be sidetracked by this issue).
That's what I get for forgetting one testsuite addition. Thanks for
tracking this down, and please push the fix, ideally together with a
new test (but better fixed without than nothing).
Cheers,
Ralf
[PATCH] {maint} Fix a bug in variable concatanation with `+='. (was: Re: bug#7333: bug concatenating CLEANFILES in automake 1.11)
On Friday 05 November 2010, Stefano Lattarini wrote:
I can confirm the bug with latest automake (from git master), with
a much-reduced minimal testcase (see attachment).
I still haven't looked for an explanation or a fix, though.
I've manged to find a very simple fix for the bug (see attached patch).
OK to apply to maint?
Regards,
Stefano
From b8de299295e081909c6d0a8a1cef957b337e3732 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Sat, 6 Nov 2010 12:46:52 +0100
Subject: [PATCH] Fix a bug in variable concatanation with `+='.
* lib/Automake/VarDef.pm (append): Remove extra backslash-escaped
newlines from the end of the variable's content, before appending
to it.
* tests/pluseq11.test: New test, exposing the bug.
* tests/Makefile.am (TESTS): Update.
Reported by Andy Wingo.
---
ChangeLog | 10
lib/Automake/VarDef.pm | 13 +-
tests/Makefile.am |1 +
tests/Makefile.in |1 +
tests/pluseq11.test| 54
5 files changed, 68 insertions(+), 11 deletions(-)
create mode 100755 tests/pluseq11.test
diff --git a/ChangeLog b/ChangeLog
index 6c17cd3..a928363 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2010-11-06 Stefano Lattarini stefano.lattar...@gmail.com
+
+ Fix a bug in variable concatanation with `+='.
+ * lib/Automake/VarDef.pm (append): Remove extra backslash-escaped
+ newlines from the end of the variable's content, before appending
+ to it.
+ * tests/pluseq11.test: New test, exposing the bug.
+ * tests/Makefile.am (TESTS): Update.
+ Reported by Andy Wingo.
+
2010-11-01 Ralf Wildenhues ralf.wildenh...@gmx.de
Fix and document rules to not touch the tree with `make -n'.
diff --git a/lib/Automake/VarDef.pm b/lib/Automake/VarDef.pm
index d7ba155..568c82a 100644
--- a/lib/Automake/VarDef.pm
+++ b/lib/Automake/VarDef.pm
@@ -185,17 +185,8 @@ sub append ($$$)
# Furthermore keeping `#' would not be portable if the variable is
# output on multiple lines.
$val =~ s/ ?#.*//;
-
- if (chomp $val)
-{
- # Insert a backslash before a trailing newline.
- $val .= \\\n;
-}
- elsif ($val)
-{
- # Insert a separator.
- $val .= ' ';
-}
+ # Insert a separator, if required.
+ $val .= ' ' if $val;
$self-{'value'} = $val . $value;
# Turn ASIS appended variables into PRETTY variables. This is to
# cope with `make' implementation that cannot read very long lines.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9c81564..da81c49 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -570,6 +570,7 @@ pluseq7.test \
pluseq8.test \
pluseq9.test \
pluseq10.test \
+pluseq11.test \
postproc.test \
ppf77.test \
pr2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index b568a09..eb461a9 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -837,6 +837,7 @@ pluseq7.test \
pluseq8.test \
pluseq9.test \
pluseq10.test \
+pluseq11.test \
postproc.test \
ppf77.test \
pr2.test \
diff --git a/tests/pluseq11.test b/tests/pluseq11.test
new file mode 100755
index 000..293270f
--- /dev/null
+++ b/tests/pluseq11.test
@@ -0,0 +1,54 @@
+#!/bin/sh
+# Copyright (C) 2010 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 bug in variable concatenation with `+=': an extra backslash
+# is erroneously retained in the final value.
+# See also sister test pluseq11b.test.
+
+. ./defs || Exit 1
+
+set -e
+
+cat configure.in 'END'
+AC_OUTPUT
+END
+
+cat Makefile.am 'END'
+## Use more line continuation to ensure we are robust and can (hopefully)
+## cope any number of them, and not just one
+FOO = \
+\
+\
+bar
+## Both this two variable additions are required to trigger the bug.
+FOO +=
+FOO += baz
+
+.PHONY: test
+test:
+ case '$(FOO)' in *\\*) exit 1;; *) exit 0;; esac
+END
+
+$ACLOCAL
+$AUTOMAKE
+
+grep '^ *FOO *=.*\\.' Makefile.in Exit 1
+
+$AUTOCONF
+./configure
+$MAKE test
+
+:
--
1.7.1
Re: More problems with `make -n' in automake-generated rules.
On Saturday 06 November 2010, Ralf Wildenhues wrote:
Hi Stefano,
* Stefano Lattarini wrote on Sat, Nov 06, 2010 at 05:52:57PM CET:
Hi Ralf, I've just spotted a bug in the patch ...
- $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n.
- \...@if test -f \$@; then :; else \\\n.
- \t rm -f \$(srcdir)/${derived}_vala.stamp; \\\n.
- \t \$(am__cd) \$(srcdir) \$(MAKE) \$(AM_MAKEFLAGS)
${derived}_vala.stamp; \\\n.
- \tfi\n;
+ $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n
+ . \...@if test -f \$@; then :; else rm -f
\$(srcdir)/${derived}_vala.stamp; \n
... here (missing fi). It causes a failure in `vala2.test'.
I'll install a fix later if you don't beat me (right now I'm doing other
testing, and prefer not to be sidetracked by this issue).
That's what I get for forgetting one testsuite addition. Thanks for
tracking this down, and please push the fix, ideally together with a
new test
Is a new test really needed? After all, I noticed the bug because it
broke the pre-existing `vala2.test'...
Regards,
Stefano
Re: [PATCH] New tests on obsoleted usages of automake/autoconf macros.
On Saturday 06 November 2010, Ralf Wildenhues wrote:
* Stefano Lattarini wrote on Wed, Nov 03, 2010 at 07:12:27PM CET:
Pinging this patch again, following this:
http://lists.gnu.org/archive/html/automake-patches/2010-11/msg3.html
I've also re-based the patch off of latest maint, extended some checks a
little bit, fixed a typo in comments, and fixed some very minor and
theoretic
portability problems (use of test -z).
The updated patch is attached.
The patch is ok except I don't think the PACKAGE_URL part will work with
Autoconf 2.62, it was added later only.
Yes, it works (tested), because we only check that it's empty if AC_INIT is
called with two arguments, and that was true also for older autoconf versions.
You can look at Automake's own
configure.ac for a workaround; the patch is OK with a fix to that end
that you deem suitable.
I don't think a fix is needed here.
When you post updated patches, then to avoid having to re-review the
whole patch again you could additionally post an incremental patch.
You can usually get one with something like
git diff h...@{1}
right after committing the increment, or some
git diff h...@{$a} h...@{$b}
for some suitable $a and $b. Thanks.
Will do (hopefully at least; just scold me again if I forgot!)
[CUT]
Regards,
Stefano
Re: More problems with `make -n' in automake-generated rules.
Hello Ralf.
On Saturday 06 November 2010, Stefano Lattarini wrote:
On Saturday 06 November 2010, Ralf Wildenhues wrote:
Hi Stefano,
* Stefano Lattarini wrote on Sat, Nov 06, 2010 at 05:52:57PM CET:
Hi Ralf, I've just spotted a bug in the patch ...
- $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n.
- \...@if test -f \$@; then :; else \\\n.
- \t rm -f \$(srcdir)/${derived}_vala.stamp; \\\n.
- \t \$(am__cd) \$(srcdir) \$(MAKE) \$(AM_MAKEFLAGS)
${derived}_vala.stamp; \\\n.
- \tfi\n;
+ $output_rules .= \$(srcdir)/$headerfile:
\$(srcdir)/${derived}_vala.stamp\n
+ . \...@if test -f \$@; then :; else rm -f
\$(srcdir)/${derived}_vala.stamp; \n
... here (missing fi). It causes a failure in `vala2.test'.
I'll install a fix later if you don't beat me (right now I'm doing other
testing, and prefer not to be sidetracked by this issue).
That's what I get for forgetting one testsuite addition. Thanks for
tracking this down, and please push the fix, ideally together with a
new test
Is a new test really needed? After all, I noticed the bug because it
broke the pre-existing `vala2.test'...
I've gone ahead and comitted the fix without adding a new testcase.
Please let me know if you still think a testcase addition would be
valuable, in which case I'll add it in a follow-up patch.
Regards,
Stefano
[FYI] merged maint into master
I merged maint into master, and pushed. The merged commits are: - Ralf Wildenhues (1): Fix and document rules to not touch the tree with `make -n'. - Stefano Lattarini (1): Fix bug in rules for creating vala vapi/header files. Regards, Stefano
[FYI] Patch pushed (was: Re: [PATCH] New tests on obsoleted usages of automake/autoconf macros.)
I just pushed the patch, by applying it to a temporary branch based off of maint (named backcompat-tests), and then merging that branch into master right away. Regards, Stefano
Re: tests updates
On Friday 05 November 2010, Stefano Lattarini wrote: On Friday 05 November 2010, Stefano Lattarini wrote: On Thursday 04 November 2010, Stefano Lattarini wrote: In the end, are you OK with having me to merge test-init to master right away, and do future testsuite work on master only? Yes. Good. I merged tests-init to master, fixed the resulting ChangeLog, and pushed. (oh, and the testsuite still passes for me, obviously). Oh, and I forgot to ask: it's ok now to remove the tests-init branch from the official automake repository? Sorry to contradict myself again, but I think that that branch might still be useful for some further work I've nearly completed, so I'd prefer not to remove it yet. Regards, Stefano
[FYI] master merged into tests-init
I've merged master into tests-init, and pushed. This has basically made the current tests-init branch equivalent to master (so that, in particular, important and useful changes such as the definition and use of variables `$PATH_SEPARATOR' and `$APIVERSION' have been imported into tests-init). This prepares the field for further work on the tests-init branch. Regards, Stefano
Re: Bison and automake together
Andrew W. Nosenko andrew.w.nose...@gmail.com writes: Therefore, there is no need neither AC_PROG_YACC nor ylwrap. Just need move them away, don't use them. Anyway, if byacc (for example) will be found and cough, it won't process GLR-targeted .y source anyway. Just threat bison like would be threated any another custom (unsupported especially by autoconf/automake) preprocessing tool. The same related to the classic Lex vs. Flex. I understand your opinion, unfortunately it then means that there's no support for Bison in C++ mode from the automake side which is unfortunate. :-/ -- PMatos
Re: Bison and automake together
pocma...@gmail.com (Paulo J. Matos) writes: I understand your opinion, unfortunately it then means that there's no support for Bison in C++ mode from the automake side which is unfortunate. :-/ I decided to forget automake support and instead go for my own: So, I removed AC_PROG_YACC from configure.ac and added: , | AC_PATH_PROG([BISON], [bison]) ` Then in Makefile.am: , | BISON_HEADER = scgparser.tab.hh | BISON_GENERATED = scgparser.tab.cc $(BISON_HEADER) | COMMON_OBJS = scgparser-driver.cc scgparser.yy scglexer.l \ | $(BISON_GENERATED) | | bin_PROGRAMS = scgc scgdoc scgmc | | scgc_SOURCES = scgc.cc $(COMMON_OBJS) | | $(BISON_GENERATED): scgparser.yy | $(BISON) --defines=$(BISON_HEADER) $ ` But then I get this annoying: , | Makefile.am: Yacc (C++) source seen but `YACC' is undefined | Makefile.am: The usual way to define `YACC' is to add `AC_PROG_YACC' | Makefile.am: to `configure.ac' and run `autoconf' again. | make: *** [Makefile.in] Error 1 ` Does someone confirm that there is a bug here? Bison in C++ mode with -y compatibility mode generates a file which includes y.tab.h. ylwrap _cannot_ rename the file or compilation will fail. If C++ is supported there's a problem with ylwrap. If it's not supported then Yacc shouldn't display the error and abort the build. Am I right? -- PMatos
Re: Bison and automake together
On Sat, Nov 06, 2010 at 04:37:26PM +, Paulo J. Matos wrote: Does someone confirm that there is a bug here? Bison in C++ mode with -y compatibility mode generates a file which includes y.tab.h. ylwrap _cannot_ rename the file or compilation will fail. If C++ is supported there's a problem with ylwrap. If it's not supported then Yacc shouldn't display the error and abort the build. In my opinion, ylwrap should gain knowledge about the used yacc implementation and pass a -o to it if the implementation supports it. That way, line numbers will be correct (aids debugging), header file inclusions will be correct (solves your problem) and it automatically allows for multiple yacc files to coexist in one project. -- Pippijn van Steenhoven signature.asc Description: Digital signature
Re: Bison and automake together
You can look at the problem in 2 ways, really you can say ylwrap is too simplistic and not great which is correct but it does work for standard-ish inputs like C parsers. But c++ and GLR doesnt work right but you could say they dont comform to the standard way ylwrap expects so it fails. I do want to start to look into this problem with ylwrap and automake support this week something i;ve been meaning to do for about a year or so but havent had the time untill about now. I have some old projects which had multiple lexer's and parsers and ylwrap could help more in this regard which it doesnt there are tricks you can search the mailing list for old posts about this though. --Phil On 6 November 2010 17:06, Pippijn van Steenhoven pip8...@gmail.com wrote: On Sat, Nov 06, 2010 at 04:37:26PM +, Paulo J. Matos wrote: Does someone confirm that there is a bug here? Bison in C++ mode with -y compatibility mode generates a file which includes y.tab.h. ylwrap _cannot_ rename the file or compilation will fail. If C++ is supported there's a problem with ylwrap. If it's not supported then Yacc shouldn't display the error and abort the build. In my opinion, ylwrap should gain knowledge about the used yacc implementation and pass a -o to it if the implementation supports it. That way, line numbers will be correct (aids debugging), header file inclusions will be correct (solves your problem) and it automatically allows for multiple yacc files to coexist in one project. -- Pippijn van Steenhoven -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEARECAAYFAkzVizIACgkQJc+zqGNdDgpmjgCgu153EM4ifKlkiVpb6EJdLJXx qigAn1HrC+EPynBJ96qty403O2aSudTO =r5cg -END PGP SIGNATURE-
