--- Begin Message ---
|
Subject: |
Stripping of comments with the `+=' operator |
|
Date: |
Fri, 10 Dec 2010 19:06:43 +0100 |
|
User-agent: |
KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; ) |
Hello automakers.
The current automake implementation of variable-appending operator
`+=' tries to be smart w.r.t. comments in the values; for example:
$ cat configure.ac
AC_INIT(x,0)
AM_INIT_AUTOMAKE
AC_CONFIG_FILES(Makefile)
AC_OUTPUT
$ cat Makefile.am
foo1 = bar # comment 1
foo1 += baz # comment 2
foo2 = bar
foo2 += baz # comment 3
foo3 = bar baz # comment 4
results in:
$ aclocal && automake --foreign --add-missing
configure.ac:2: installing `./install-sh'
configure.ac:2: installing `./missing'
$ grep '^foo[0-9] *=' Makefile.in
foo1 = bar baz # comment 2
foo2 = bar baz # comment 3
foo3 = bar baz # comment 4
which is admittedly better than having something bogus like:
$ cat Makefile.am
foo = bar # comment
foo += baz
$ aclocal && automake ...
$ grep '^foo *=' Makefile.in
foo = bar # comment baz
However, the code in automake is apparently not smart enough to detect
that someone might be trying to put a *literal* `#' character in a
variable extended with `+=':
$ cat configure.ac
AC_INIT(x,0)
AM_INIT_AUTOMAKE
AC_CONFIG_FILES(Makefile)
AC_OUTPUT
$ cat Makefile.am
CDEFS = -D'FOO_STRING="\#\#foo\#\#"'
CDEFS += -DBAR
$ aclocal && automake --foreign --add-missing
configure.ac:2: installing `./install-sh'
configure.ac:2: installing `./missing'
$ grep '^CDEFS *=' Makefile.in
CDEFS = -D'FOO_STRING="\ -DBAR
Now, while the above use of -D'FOO_STRING="\#\#foo\#\#"' in a variable
definition is certainly bound to produce code unportable to some make
implementations, the right thing for automake to do is either to
assume the user knows what he's doing (maybe he's not interested into
portability to those make implementations), or to warn about the
unportable usage (if `-Wportability' is enabled, obviously).
Silently mangling the value of the resulting value of $(DEFS) is *not*
an acceptale behaviour IMHO, but unfortunately, that's exactly what
automake currently does (as seen above).
Tangentially, note that the definition:
CDEFS = -D'FOO_STRING="\#\#foo\#\#"'
actually works with at least GNU make, modern FreeBSD make,
and modern NetBSD make:
$ cat Makefile
CDEFS = -D'FOO_STRING="\#\#foo\#\#"'
all:; @echo $(CDEFS)
$ make # is either GNU, FreeBSD or NetBSD make
-DFOO_STRING="##foo##"
and "sorta" works with Solaris 10 make:
$ /usr/xpg4/bin/make
-DFOO_STRING="\#\#foo\#\#"
$ /usr/ccs/bin/make
-DFOO_STRING="\#\#foo\#\#"
Regards,
Stefano
--- End Message ---
--- Begin Message ---
|
Subject: |
[PATCH v2 committed] automake: support embedded \# in variable appends |
|
Date: |
Wed, 23 Feb 2022 23:04:27 -0500 |
Fixes automake bug https://bugs.gnu.org/7610.
Use of \# is not portable. POSIX does not provide any way of retaining
the # marker in variables. There is wide spread support for \# though
in GNU & BSD Make implementations.
Today, with plain variable assignments, Automake leaves the line alone:
foo = blah\#blah
This will leave it to the implementation to decide what to do. But if
you try to append to it, Automake follows POSIX and strips it:
foo = blah\#blah
foo += what
-> foo = blah\ what
Instead, let's issue a portability warning whenever \# is used, even if
it isn't being appended, and do not strip the \# when appending. Now:
foo = blah\#blah
foo += what
-> warning: escaping \# comment markers is not portable
-> foo = blah\#blah what
* NEWS: Mention change in \# handling.
* lib/Automake/VarDef.pm: Do not strip # if escaped.
* lib/Automake/Variable.pm: Warn if \# is used.
* t/comment12.sh: New test.
* t/comments-escaped-in-var.sh: New test.
* t/list-of-tests.mk: Add comment12.sh & comments-escaped-in-var.sh.
---
v2
- had to adjust the regex slightly to handle comments at the start of vars,
and add a new test to catch variations on it
NEWS | 3 +++
lib/Automake/VarDef.pm | 2 +-
lib/Automake/Variable.pm | 3 +++
t/comment12.sh | 44 +++++++++++++++++++++++++++++++++
t/comments-escaped-in-var.sh | 47 ++++++++++++++++++++++++++++++++++++
t/list-of-tests.mk | 2 ++
6 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 t/comment12.sh
create mode 100644 t/comments-escaped-in-var.sh
diff --git a/NEWS b/NEWS
index 512cdecd0cc5..665d8329d667 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,9 @@ New in 1.17:
drop support for them has been reversed. The ACCEPT_INFERIOR_RM_PROGRAM
setting no longer exists.
+ - Variables using escaped \# will trigger portability warnings, but be
+ retained when appended. GNU Make & BSD Makes are known to support it.
+
* Obsolescent features:
- py-compile no longer supports Python 0.x or 1.x versions. Python 2.0,
diff --git a/lib/Automake/VarDef.pm b/lib/Automake/VarDef.pm
index 59af4036b35f..00d694a6fbda 100644
--- a/lib/Automake/VarDef.pm
+++ b/lib/Automake/VarDef.pm
@@ -188,7 +188,7 @@ sub append ($$$)
# VAR = foo # com bar
# Furthermore keeping '#' would not be portable if the variable is
# output on multiple lines.
- $val =~ s/ ?#.*//;
+ $val =~ s/ ?(^|[^\\])#.*/$1/;
# Insert a separator, if required.
$val .= ' ' if $val;
$self->{'value'} = $val . $value;
diff --git a/lib/Automake/Variable.pm b/lib/Automake/Variable.pm
index 4f6a88d61c5c..1269d5d9f40e 100644
--- a/lib/Automake/Variable.pm
+++ b/lib/Automake/Variable.pm
@@ -857,6 +857,9 @@ sub define ($$$$$$$$)
msg ('portability', $where, "':='-style assignments are not portable")
if $type eq ':';
+ msg ('portability', $where, "escaping \\# comment markers is not portable")
+ if index ($value, '\#') != -1;
+
check_variable_expansions ($value, $where);
# If there's a comment, make sure it is \n-terminated.
diff --git a/t/comment12.sh b/t/comment12.sh
new file mode 100644
index 000000000000..e98159e40f3e
--- /dev/null
+++ b/t/comment12.sh
@@ -0,0 +1,44 @@
+#! /bin/sh
+# Copyright (C) 2022 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 <https://www.gnu.org/licenses/>.
+
+# Make sure that earlier comments are consumed when appending to it.
+
+. test-init.sh
+
+cat > Makefile.am << 'END'
+VAR1=# eat this comment
+VAR2 =# eat this comment
+VAR3 = # eat this comment
+VAR4 = # eat this comment
+VAR5 = val# eat this comment
+VAR6 = val # eat this comment
+
+VAR1 += val
+VAR2 += val
+VAR3 += val
+VAR4 += val
+VAR5 +=
+VAR6 +=
+END
+
+$ACLOCAL
+$AUTOMAKE
+
+# For debugging.
+grep '^VAR' Makefile.in
+
+count=`grep '^VAR. = val$' Makefile.in | wc -l`
+[ $count -eq 6 ]
diff --git a/t/comments-escaped-in-var.sh b/t/comments-escaped-in-var.sh
new file mode 100644
index 000000000000..e06c3f359b68
--- /dev/null
+++ b/t/comments-escaped-in-var.sh
@@ -0,0 +1,47 @@
+#! /bin/sh
+# Copyright (C) 2022 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 <https://www.gnu.org/licenses/>.
+
+# Make sure Automake preserves escaped comments in the output.
+
+. test-init.sh
+
+cat >> configure.ac <<'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+# This value should be preserved.
+var = -DVAL='"\#xxx\#"' # this should be stripped
+var += -DX=1 # this should be kept
+END
+
+$ACLOCAL
+
+# This should fail due to -Werror.
+$AUTOMAKE && exit 1
+
+# This should pass though.
+$AUTOMAKE -Wno-portability
+
+# For debugging.
+grep ^var Makefile.in
+
+# The full flag should be retained.
+grep '^var.*\\#xxx\\#.*DX=1' Makefile.in
+
+# Only the 2nd comment should be retained.
+grep '^var.*stripped' Makefile.in && exit 1 || :
+grep '^var.*should be kept' Makefile.in
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 1601154e031e..6bb6bef95dbf 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -265,7 +265,9 @@ t/comment8.sh \
t/comment9.sh \
t/commen10.sh \
t/commen11.sh \
+t/comment12.sh \
t/comment-block.sh \
+t/comments-escaped-in-var.sh \
t/comments-in-var-def.sh \
t/compile.sh \
t/compile2.sh \
--
2.34.1
--- End Message ---