GNU bug report logs - #7333
bug concatenating CLEANFILES in automake 1.11

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 7333 in the body.
You can then email your comments to 7333 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> oblong.com>
To: bug-automake <at> gnu.org
Cc: Ben Denckla <bdenckla <at> oblong.com>
Subject: bug concatenating CLEANFILES in automake 1.11
Date: Fri, 05 Nov 2010 11:36:24 +0100

Greets,

We have an automake file that, after includes are processed out, looks
something like this:

# -*- makefile-gmake -*-

CLEANFILES =
BUILT_SOURCES =

CLEANFILES += \
  $(addprefix log/, $(addsuffix .log, $(notdir $(TESTS) _timing))) \
  $(wildcard scratch/*) \
  #
CLEANFILES += \
  $(BUILT_SOURCES) \
  $(OBLONG_pcifiles) \
  #
CLEANFILES += \
  $(man_MANS) \
  $(htmlman_DATA) \
  pod2htmi.tmp \
  pod2htmd.tmp
CLEANFILES +=
if HAVE_FOO
GOBJECTS = $(SOURCES:%.scm=%.go)
CLEANFILES += $(GOBJECTS)
endif

SOURCES =
SOURCES += plasma.scm

BUILT_SOURCES += env
CLEANFILES += env


In the generated Makefile, there is an extraneous backslash:

CLEANFILES = $(addprefix log/, $(addsuffix .log, $(notdir $(TESTS) \
	_timing))) $(wildcard scratch/*) $(BUILT_SOURCES) \
	$(OBLONG_pcifiles) $(man_MANS) $(htmlman_DATA) pod2htmi.tmp \
	pod2htmd.tmp\ $(am__append_3) env
                    ^ it's here

Regardless of the taste of that snippet above ;-), that snippet should
be sufficient to reproduce this error, with --foreign and the following
configure.ac:


AC_INIT(foo, 0.1)
AM_INIT_AUTOMAKE

AC_CONFIG_FILES([Makefile])
AM_CONDITIONAL(HAVE_FOO, 1)


AC_OUTPUT


Thanks,

Andy

Message #8 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Cc: Andy Wingo <wingo <at> oblong.com>, Ben Denckla <bdenckla <at> oblong.com>,
	7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: bug concatenating CLEANFILES in automake 1.11
Date: Fri, 5 Nov 2010 12:14:36 +0100

[Message part 1 (text/plain, inline)]

Hello Andy, and thanks for the report.

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.

Regards,
   Stefano

[foo.test (application/x-shellscript, inline)]

Message #14 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: automake-patches <at> gnu.org
Cc: Andy Wingo <wingo <at> oblong.com>, Ben Denckla <bdenckla <at> oblong.com>,
	bug-automake <at> gnu.org, 7333 <at> debbugs.gnu.org
Subject: [PATCH] {maint} Fix a bug in variable concatanation with `+='. (was:
	Re: bug#7333: bug concatenating CLEANFILES in automake 1.11)
Date: Sat, 6 Nov 2010 18:10:44 +0100

[Message part 1 (text/plain, inline)]

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

[0001-Fix-a-bug-in-variable-concatanation-with.patch (text/x-patch, inline)]

From b8de299295e081909c6d0a8a1cef957b337e3732 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <stefano.lattarini <at> 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.lattarini <at> 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.Wildenhues <at> 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 0000000..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

Message #17 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: Andy Wingo <wingo <at> oblong.com>, Ben Denckla <bdenckla <at> oblong.com>,
	7333 <at> debbugs.gnu.org, automake-patches <at> gnu.org
Subject: Re: [PATCH] {maint} Fix a bug in variable concatanation with `+='.
Date: Sun, 7 Nov 2010 14:26:09 +0100

Hello Stefano, Andy,

* Stefano Lattarini wrote on Sat, Nov 06, 2010 at 06:10:44PM CET:
> 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?

OK if it passes the whole testsuite.

Thanks to both of you, for the report and the fast fix!

Cheers,
Ralf

> From b8de299295e081909c6d0a8a1cef957b337e3732 Mon Sep 17 00:00:00 2001
> From: Stefano Lattarini <stefano.lattarini <at> 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.

Message #20 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: automake-patches <at> gnu.org,
 7333 <at> debbugs.gnu.org
Cc: Andy Wingo <wingo <at> oblong.com>, Ben Denckla <bdenckla <at> oblong.com>
Subject: Re: [PATCH] {maint} Fix a bug in variable concatanation with `+='.
Date: Sun, 7 Nov 2010 15:26:52 +0100

On Sunday 07 November 2010, Ralf Wildenhues wrote:
> Hello Stefano, Andy,
> 
> * Stefano Lattarini wrote on Sat, Nov 06, 2010 at 06:10:44PM CET:
> > 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?
> 
> OK if it passes the whole testsuite.
It does for me.  BTW, I've also squashed a minor typofix in the new
test's comments, and a better ChangeLog entry:

   Fix a bug in variable concatanation with `+='.
   * lib/Automake/VarDef.pm (append): Since the content of the
   "appended-to" variable is going to be unconditionally normalized
   later, simply separate the appended value with a single whitespace
   character, instead of trying to be uselesssly smarter by using
   escaped newlines.  This fixes a bug in which extra backslashes
   where erroneously inserted in the variable's final value.
   * tests/pluseq11.test: New test, exposing the bug.
   * tests/Makefile.am (TESTS): Update.
   Reported by Andy Wingo.

Now, should I also try to close the bug #7333 on debbugs?
The instruction at <http://debbugs.gnu.org/Developer.html> seems
quite clear about how to do so, so I think I can manage to get it
right.

BTW, Ouch!  I see that my previous reply presenting the patch
has erroneously opended a new, spurious bug report (#7345) in
the tracker!  Ralf, could you please you close that report as
invalid?

Regards,
   Stefano

Message #23 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: Andy Wingo <wingo <at> oblong.com>, Ben Denckla <bdenckla <at> oblong.com>,
	7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: [PATCH] {maint} Fix a bug in variable concatanation
	with `+='.
Date: Mon, 8 Nov 2010 22:46:09 +0100

merge 7333 7345
tags 7333 + patch
close 7333 v1.11-222-g7a020d6
thanks

* Stefano Lattarini wrote on Sun, Nov 07, 2010 at 03:26:52PM CET:
> Now, should I also try to close the bug #7333 on debbugs?
> The instruction at <http://debbugs.gnu.org/Developer.html> seems
> quite clear about how to do so, so I think I can manage to get it
> right.
> 
> BTW, Ouch!  I see that my previous reply presenting the patch
> has erroneously opended a new, spurious bug report (#7345) in
> the tracker!  Ralf, could you please you close that report as
> invalid?

Not sure how that happened, probably the Subject: change.
The commands above (Bcc:ed to control at debbugs) should merge and
close both bugs.

Cheers,
Ralf

Message #30 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
Cc: Stefano Lattarini <stefano.lattarini <at> gmail.com>, 7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: [PATCH] {maint} Fix a bug in variable concatanation
	with `+='.
Date: Tue, 9 Nov 2010 02:34:23 -0500

Ralf Wildenhues wrote (on Mon, 8 Nov 2010 at 22:46 +0100):

> > BTW, Ouch!  I see that my previous reply presenting the patch
> > has erroneously opended a new, spurious bug report (#7345) in
> > the tracker!  Ralf, could you please you close that report as
> > invalid?
> 
> Not sure how that happened, probably the Subject: change.

It was a combination of changing the subject plus cc'ing bug-automake.
bug-automake is basically an alias for submit <at> debbugs now.
Anything sent there that is not "obviously" replying to an existing
report creates a new bug.

There is no need to cc bug-automake on replies to reports; ###@debbugs
can be thought of as an alias for that. The only time to use the
bug-automake address now is in new bug reports. Sending to both just
creates duplicates.

> The commands above (Bcc:ed to control at debbugs) should merge and
> close both bugs.

The (first) close didn't work, I think it was because your version
number started with a `v', not a digit. The second close worked.

Message #33 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Stefano Lattarini <stefano.lattarini <at> gmail.com>, 7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: [PATCH] {maint} Fix a bug in variable concatanation
	with `+='.
Date: Wed, 10 Nov 2010 19:33:07 +0100

* Glenn Morris wrote on Tue, Nov 09, 2010 at 08:34:23AM CET:
> Ralf Wildenhues wrote (on Mon, 8 Nov 2010 at 22:46 +0100):
> > The commands above (Bcc:ed to control at debbugs) should merge and
> > close both bugs.
> 
> The (first) close didn't work, I think it was because your version
> number started with a `v', not a digit. The second close worked.

Can that be fixed in debbugs?  I would love to have both 'git describe'
output as well as commit SHA1 strings be acceptable as version strings.
git describe output starts with v but then contrains fairly much
arbitrary characters as put in the last signed git tag.  SHA1's are
easier, and if the other is a problem then those would still be better
than no version information, I guess.

Thanks,
Ralf

Message #36 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
Cc: Stefano Lattarini <stefano.lattarini <at> gmail.com>, 7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: [PATCH] {maint} Fix a bug in variable concatanation
	with `+='.
Date: Wed, 10 Nov 2010 15:40:38 -0500

Ralf Wildenhues wrote:

> Can that be fixed in debbugs?  I would love to have both 'git describe'
> output as well as commit SHA1 strings be acceptable as version strings.
> git describe output starts with v but then contrains fairly much
> arbitrary characters as put in the last signed git tag.  SHA1's are
> easier, and if the other is a problem then those would still be better
> than no version information, I guess.

Sure. Here is the current regexp:

m/^close\s+\#?(-?\d+)(?:\s+(\d.*))?$/i

$1 = bug number
$2 = version number

What would you like it to be?

Message #39 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>,
	Stefano Lattarini <stefano.lattarini <at> gmail.com>, 7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: [PATCH] {maint} Fix a bug in variable concatanation
	with `+='.
Date: Wed, 10 Nov 2010 15:46:01 -0500

Actually, that was probably too glib a response. The version number
information is probably used in other places, and needs to be sortable
so that the fixed/found commands can work. So I don't think arbitrary
version strings can work. You could use the date of a commit perhaps.

Message #42 received at 7333 <at> debbugs.gnu.org (full text, mbox):

From: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Stefano Lattarini <stefano.lattarini <at> gmail.com>, 7333 <at> debbugs.gnu.org
Subject: Re: bug#7333: [PATCH] {maint} Fix a bug in variable concatanation
	with `+='.
Date: Wed, 10 Nov 2010 22:31:47 +0100

* Glenn Morris wrote on Wed, Nov 10, 2010 at 09:46:01PM CET:
> 
> Actually, that was probably too glib a response. The version number
> information is probably used in other places, and needs to be sortable
> so that the fixed/found commands can work. So I don't think arbitrary
> version strings can work. You could use the date of a commit perhaps.

If that's the case, let's I'd say just live with it.  Anyway better to
send mail to NNN-done, where one can include arbitrary detail.

Thanks for checking,
Ralf

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.