GNU bug report logs - #7610
Stripping of comments with the `+=' operator

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 7610 in the body.
You can then email your comments to 7610 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: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Subject: Stripping of comments with the `+=' operator
Date: Fri, 10 Dec 2010 19:06:43 +0100

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

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

From: Mike Frysinger <vapier <at> gentoo.org>
To: 7610 <at> debbugs.gnu.org
Subject: [PATCH] automake: support embedded \# in variable appends
Date: Sun, 20 Feb 2022 13:35:11 -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                         |  3 +++
 lib/Automake/VarDef.pm       |  2 +-
 lib/Automake/Variable.pm     |  3 +++
 t/comments-escaped-in-var.sh | 47 ++++++++++++++++++++++++++++++++++++
 t/list-of-tests.mk           |  1 +
 5 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 t/comments-escaped-in-var.sh

diff --git a/NEWS b/NEWS
index fbb3f758ce52..6e04ee123ea0 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..599b57fe85d6 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/ ?[^\\]#.*//;
   # 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/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..d8e50b080166 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -266,6 +266,7 @@ t/comment9.sh \
 t/commen10.sh \
 t/commen11.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

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

From: Karl Berry <karl <at> freefriends.org>
To: vapier <at> gentoo.org
Cc: 7610 <at> debbugs.gnu.org
Subject: Re: bug#7610: [PATCH] automake: support embedded \# in variable
 appends
Date: Sun, 20 Feb 2022 15:25:38 -0700

    Use of \# is not portable.

Is there any known make implementation which does not preserve the #?

      foo = blah\#blah
      foo += what
      -> warning: escaping \# comment markers is not portable
      -> foo = blah\#blah what

Assuming there is in fact variability in behavior in practice, sounds
fine. --thanks, karl.

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

From: Mike Frysinger <vapier <at> gentoo.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: 7610 <at> debbugs.gnu.org
Subject: Re: bug#7610: [PATCH] automake: support embedded \# in variable
 appends
Date: Sun, 20 Feb 2022 19:20:41 -0500

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

On 20 Feb 2022 15:25, Karl Berry wrote:
>     Use of \# is not portable.
> 
> Is there any known make implementation which does not preserve the #?
> 
>       foo = blah\#blah
>       foo += what
>       -> warning: escaping \# comment markers is not portable
>       -> foo = blah\#blah what
> 
> Assuming there is in fact variability in behavior in practice, sounds
> fine. --thanks, karl.

at least POSIX has this to say:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/make.html#tag_20_76_16
> Because of widespread historical practice, interpreting a <number-sign> ( '#' )
> inside a variable as the start of a comment has the unfortunate side-effect of
> making it impossible to place a <number-sign> in a variable, thus forbidding
> something like:
> 
> CFLAGS = "-D COMMENT_CHAR='#'"

from this specific bug report, Stefano mentioned Solaris 10 make preserves both
the \ and the #.
-mike

[signature.asc (application/pgp-signature, inline)]

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

From: Karl Berry <karl <at> freefriends.org>
To: vapier <at> gentoo.org
Cc: 7610 <at> debbugs.gnu.org
Subject: Re: bug#7610: [PATCH] automake: support embedded \# in variable
 appends
Date: Wed, 23 Feb 2022 19:17:43 -0700

Agreed that it is better to preserve the input text.

Agreed that a warning is indicated, but as Stefano suggested, I think
the warning should only be given if -Wportability is enabled (which it
is by default, I gather), not unconditionally. From our manual:

       `portability'  
          Portability issues (e.g., use of `make' features that are
          known to be not portable).

which, so far as I can see, is what is being dealt with here. --thanks, karl.

Message #26 received at 7610-done <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: 7610-done <at> debbugs.gnu.org
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

Previous Next

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