[PATCH] mk: don't escape '#' for `grep`

[PATCH] mk: don't escape '#' for `grep`

[Sergei Trofimovich]

Without the change build process complains

    $ LANG=C make -R help
    grep: warning: stray \ before #
    grep: warning: stray \ before #

Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
---
 share/mk/configure/build-depends/cpp/cpp.mk | 2 +-
 share/mk/src/sh.mk                          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/share/mk/configure/build-depends/cpp/cpp.mk b/share/mk/configure/build-depends/cpp/cpp.mk
index ef12a848a..8a4ccc442 100644
--- a/share/mk/configure/build-depends/cpp/cpp.mk
+++ b/share/mk/configure/build-depends/cpp/cpp.mk
@@ -21,7 +21,7 @@ endif
 CPP_HAS_ALREADY_D_FORTIFY_SOURCE := \
 	$(shell \
 		$(CPP) -dM - -Wno-error </dev/null \
-		| $(GREP) '\#define _FORTIFY_SOURCE ' >/dev/null \
+		| $(GREP) '#define _FORTIFY_SOURCE ' >/dev/null \
 		&& $(ECHO) yes \
 		|| $(ECHO) no; \
 	)
diff --git a/share/mk/src/sh.mk b/share/mk/src/sh.mk
index 05c9e0449..487eaf14d 100644
--- a/share/mk/src/sh.mk
+++ b/share/mk/src/sh.mk
@@ -14,7 +14,7 @@ include $(MAKEFILEDIR)/configure/directory_variables/src.mk
 
 
 BIN_sh := $(shell $(FIND) $(SRCBINDIR) -type f \
-		| $(XARGS) $(GREP) -l '^\#!/bin/\(sh\|bash\)\>' \
+		| $(XARGS) $(GREP) -l '^#!/bin/\(sh\|bash\)\>' \
 		| $(SORT))
 
 
-- 
2.48.1

Re: [PATCH] mk: don't escape '#' for `grep`

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 3976 bytes --]

Hi Sergei,

I'm CCing Boris, since he had the opposite problem, which triggered
adding the stray '\'.

On Thu, Feb 27, 2025 at 09:21:42AM +0000, Sergei Trofimovich wrote:
> Without the change build process complains
> 
>     $ LANG=C make -R help
>     grep: warning: stray \ before #
>     grep: warning: stray \ before #

I can't reproduce.  What's your version of grep(1) and your version of
make(1)?

	alx@debian:~/src/linux/man-pages/man-pages/contrib$ make -R help
	make: warning: undefined variable 'GNUMAKEFLAGS'
	To see a list of targets, run:
		$ make -R -p nothing \
		| grep '^\.PHONY:' \
		| tr ' ' '\n' \
		| grep -v '^\.PHONY:' \
		| sort;

	To see a list of variables, run:
		$ find GNUmakefile share/mk/configure -type f \
		| sort \
		| xargs grep '^[^[:space:]].*?=' \
		| sed 's/=.*/=/';

	To see a list of dependencies (package/program), run:
		$ find share/mk/configure/build-depends -type f \
		| sed 's,share/mk/configure/build-depends/,,' \
		| sed 's,\.mk,,' \
		| sort;

	alx@debian:~/src/linux/man-pages/man-pages/contrib$ LANG=C make -R help
	make: warning: undefined variable 'GNUMAKEFLAGS'
	To see a list of targets, run:
		$ make -R -p nothing \
		| grep '^\.PHONY:' \
		| tr ' ' '\n' \
		| grep -v '^\.PHONY:' \
		| sort;

	To see a list of variables, run:
		$ find GNUmakefile share/mk/configure -type f \
		| sort \
		| xargs grep '^[^[:space:]].*?=' \
		| sed 's/=.*/=/';

	To see a list of dependencies (package/program), run:
		$ find share/mk/configure/build-depends -type f \
		| sed 's,share/mk/configure/build-depends/,,' \
		| sed 's,\.mk,,' \
		| sort;

	alx@debian:~/src/linux/man-pages/man-pages/contrib$ make --version
	GNU Make 4.4.1
	Built for x86_64-pc-linux-gnu
	Copyright (C) 1988-2023 Free Software Foundation, Inc.
	License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
	This is free software: you are free to change and redistribute it.
	There is NO WARRANTY, to the extent permitted by law.
	alx@debian:~/src/linux/man-pages/man-pages/contrib$ grep --version
	grep (GNU grep) 3.11
	Copyright (C) 2023 Free Software Foundation, Inc.
	License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
	This is free software: you are free to change and redistribute it.
	There is NO WARRANTY, to the extent permitted by law.

	Written by Mike Haertel and others; see
	<https://git.savannah.gnu.org/cgit/grep.git/tree/AUTHORS>.

	grep -P uses PCRE2 10.45 2025-02-05

> 
> Signed-off-by: Sergei Trofimovich <slyich@gmail.com>

Please add a tag mentioning that it reverts a certain commit:

	Fixes: 76f12e3fd3ea (2025-02-10; "share/mk/: Escape '#' in regexes")


Have a lovely day!
Alex

> ---
>  share/mk/configure/build-depends/cpp/cpp.mk | 2 +-
>  share/mk/src/sh.mk                          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/share/mk/configure/build-depends/cpp/cpp.mk b/share/mk/configure/build-depends/cpp/cpp.mk
> index ef12a848a..8a4ccc442 100644
> --- a/share/mk/configure/build-depends/cpp/cpp.mk
> +++ b/share/mk/configure/build-depends/cpp/cpp.mk
> @@ -21,7 +21,7 @@ endif
>  CPP_HAS_ALREADY_D_FORTIFY_SOURCE := \
>  	$(shell \
>  		$(CPP) -dM - -Wno-error </dev/null \
> -		| $(GREP) '\#define _FORTIFY_SOURCE ' >/dev/null \
> +		| $(GREP) '#define _FORTIFY_SOURCE ' >/dev/null \
>  		&& $(ECHO) yes \
>  		|| $(ECHO) no; \
>  	)
> diff --git a/share/mk/src/sh.mk b/share/mk/src/sh.mk
> index 05c9e0449..487eaf14d 100644
> --- a/share/mk/src/sh.mk
> +++ b/share/mk/src/sh.mk
> @@ -14,7 +14,7 @@ include $(MAKEFILEDIR)/configure/directory_variables/src.mk
>  
>  
>  BIN_sh := $(shell $(FIND) $(SRCBINDIR) -type f \
> -		| $(XARGS) $(GREP) -l '^\#!/bin/\(sh\|bash\)\>' \
> +		| $(XARGS) $(GREP) -l '^#!/bin/\(sh\|bash\)\>' \
>  		| $(SORT))
>  
>  
> -- 
> 2.48.1
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mk: don't escape '#' for `grep`

[Sergei Trofimovich]

On Thu, Feb 27, 2025 at 11:23:46AM +0100, Alejandro Colomar wrote:
> Hi Sergei,
> 
> I'm CCing Boris, since he had the opposite problem, which triggered
> adding the stray '\'.
> 
> On Thu, Feb 27, 2025 at 09:21:42AM +0000, Sergei Trofimovich wrote:
> > Without the change build process complains
> > 
> >     $ LANG=C make -R help
> >     grep: warning: stray \ before #
> >     grep: warning: stray \ before #
> 
> I can't reproduce.  What's your version of grep(1) and your version of
> make(1)?
> 
> 	alx@debian:~/src/linux/man-pages/man-pages/contrib$ make -R help
> 	make: warning: undefined variable 'GNUMAKEFLAGS'
> 	To see a list of targets, run:
> 		$ make -R -p nothing \
> 		| grep '^\.PHONY:' \
> 		| tr ' ' '\n' \
> 		| grep -v '^\.PHONY:' \
> 		| sort;
> 
> 	To see a list of variables, run:
> 		$ find GNUmakefile share/mk/configure -type f \
> 		| sort \
> 		| xargs grep '^[^[:space:]].*?=' \
> 		| sed 's/=.*/=/';
> 
> 	To see a list of dependencies (package/program), run:
> 		$ find share/mk/configure/build-depends -type f \
> 		| sed 's,share/mk/configure/build-depends/,,' \
> 		| sed 's,\.mk,,' \
> 		| sort;
> 
> 	alx@debian:~/src/linux/man-pages/man-pages/contrib$ LANG=C make -R help
> 	make: warning: undefined variable 'GNUMAKEFLAGS'
> 	To see a list of targets, run:
> 		$ make -R -p nothing \
> 		| grep '^\.PHONY:' \
> 		| tr ' ' '\n' \
> 		| grep -v '^\.PHONY:' \
> 		| sort;
> 
> 	To see a list of variables, run:
> 		$ find GNUmakefile share/mk/configure -type f \
> 		| sort \
> 		| xargs grep '^[^[:space:]].*?=' \
> 		| sed 's/=.*/=/';
> 
> 	To see a list of dependencies (package/program), run:
> 		$ find share/mk/configure/build-depends -type f \
> 		| sed 's,share/mk/configure/build-depends/,,' \
> 		| sed 's,\.mk,,' \
> 		| sort;
> 
> 	alx@debian:~/src/linux/man-pages/man-pages/contrib$ make --version
> 	GNU Make 4.4.1
> 	Built for x86_64-pc-linux-gnu
> 	Copyright (C) 1988-2023 Free Software Foundation, Inc.
> 	License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
> 	This is free software: you are free to change and redistribute it.
> 	There is NO WARRANTY, to the extent permitted by law.
> 	alx@debian:~/src/linux/man-pages/man-pages/contrib$ grep --version
> 	grep (GNU grep) 3.11
> 	Copyright (C) 2023 Free Software Foundation, Inc.
> 	License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
> 	This is free software: you are free to change and redistribute it.
> 	There is NO WARRANTY, to the extent permitted by law.
> 
> 	Written by Mike Haertel and others; see
> 	<https://git.savannah.gnu.org/cgit/grep.git/tree/AUTHORS>.
> 
> 	grep -P uses PCRE2 10.45 2025-02-05
> 

Aha, I wonder if it's also related to the escaping behaviour of `sh` and `bash.
Here are my tool versions:

    $ make --version
        GNU Make 4.4.1
        Built for x86_64-pc-linux-gnu
        Copyright (C) 1988-2023 Free Software Foundation, Inc.
        License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
        This is free software: you are free to change and redistribute it.
        There is NO WARRANTY, to the extent permitted by law.


    $ grep --version
        grep (GNU grep) 3.11
        Copyright (C) 2023 Free Software Foundation, Inc.
        License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
        This is free software: you are free to change and redistribute it.
        There is NO WARRANTY, to the extent permitted by law.
        
        Written by Mike Haertel and others; see
        <https://git.savannah.gnu.org/cgit/grep.git/tree/AUTHORS>.
        
        grep -P uses PCRE2 10.44 2024-06-07

    $ sh --version
        GNU bash, version 5.2.37(1)-release (x86_64-pc-linux-gnu)
        Copyright (C) 2022 Free Software Foundation, Inc.
        License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
        
        This is free software; you are free to change and redistribute it.
        There is NO WARRANTY, to the extent permitted by law.

    $ bash --version
        GNU bash, version 5.2.37(1)-release (x86_64-pc-linux-gnu)
        Copyright (C) 2022 Free Software Foundation, Inc.
        License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
        
        This is free software; you are free to change and redistribute it.
        There is NO WARRANTY, to the extent permitted by law.

> > Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
> 
> Please add a tag mentioning that it reverts a certain commit:
> 
> 	Fixes: 76f12e3fd3ea (2025-02-10; "share/mk/: Escape '#' in regexes")
> 
> 
> Have a lovely day!
> Alex
> 
> > ---
> >  share/mk/configure/build-depends/cpp/cpp.mk | 2 +-
> >  share/mk/src/sh.mk                          | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/share/mk/configure/build-depends/cpp/cpp.mk b/share/mk/configure/build-depends/cpp/cpp.mk
> > index ef12a848a..8a4ccc442 100644
> > --- a/share/mk/configure/build-depends/cpp/cpp.mk
> > +++ b/share/mk/configure/build-depends/cpp/cpp.mk
> > @@ -21,7 +21,7 @@ endif
> >  CPP_HAS_ALREADY_D_FORTIFY_SOURCE := \
> >  	$(shell \
> >  		$(CPP) -dM - -Wno-error </dev/null \
> > -		| $(GREP) '\#define _FORTIFY_SOURCE ' >/dev/null \
> > +		| $(GREP) '#define _FORTIFY_SOURCE ' >/dev/null \
> >  		&& $(ECHO) yes \
> >  		|| $(ECHO) no; \
> >  	)
> > diff --git a/share/mk/src/sh.mk b/share/mk/src/sh.mk
> > index 05c9e0449..487eaf14d 100644
> > --- a/share/mk/src/sh.mk
> > +++ b/share/mk/src/sh.mk
> > @@ -14,7 +14,7 @@ include $(MAKEFILEDIR)/configure/directory_variables/src.mk
> >  
> >  
> >  BIN_sh := $(shell $(FIND) $(SRCBINDIR) -type f \
> > -		| $(XARGS) $(GREP) -l '^\#!/bin/\(sh\|bash\)\>' \
> > +		| $(XARGS) $(GREP) -l '^#!/bin/\(sh\|bash\)\>' \
> >  		| $(SORT))
> >  
> >  
> > -- 
> > 2.48.1
> > 
> 
> -- 
> <https://www.alejandro-colomar.es/>



-- 

  Sergei

Re: [PATCH] mk: don't escape '#' for `grep`

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2076 bytes --]

Hi Sergei,

On Thu, Feb 27, 2025 at 11:23:54AM +0000, Sergei Trofimovich wrote:
> Aha, I wonder if it's also related to the escaping behaviour of `sh` and `bash.

sh(1) shouldn't matter, because the I explicitly request bash(1) in the
makefiles.

	$ grep -rn SHELL GNUmakefile 
	5:SHELL       := bash
	6:.SHELLFLAGS := -Eeuo pipefail -c

> Here are my tool versions:
> 
>     $ sh --version
>         GNU bash, version 5.2.37(1)-release (x86_64-pc-linux-gnu)
>         Copyright (C) 2022 Free Software Foundation, Inc.
>         License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
>         
>         This is free software; you are free to change and redistribute it.
>         There is NO WARRANTY, to the extent permitted by law.

	alx@debian:~$ which sh
	/usr/bin/sh
	alx@debian:~$ which sh | xargs dpkg -S
	dash: /usr/bin/sh
	alx@debian:~$ which sh | xargs dpkg -S | cut -f1 -d: | xargs dpkg -l | tail -n1
	ii  dash           0.5.12-12    amd64        POSIX-compliant shell

(But as said, this shouldn't matter.)

> 
>     $ bash --version
>         GNU bash, version 5.2.37(1)-release (x86_64-pc-linux-gnu)
>         Copyright (C) 2022 Free Software Foundation, Inc.
>         License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
>         
>         This is free software; you are free to change and redistribute it.
>         There is NO WARRANTY, to the extent permitted by law.

	$ bash --version
	GNU bash, version 5.2.37(1)-release (x86_64-pc-linux-gnu)
	Copyright (C) 2022 Free Software Foundation, Inc.
	License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

	This is free software; you are free to change and redistribute it.
	There is NO WARRANTY, to the extent permitted by law.

Could you please try to find out the cause of the actual problem?  Maybe
there's a fix that doesn't involve reverting that patch.  Or maybe
there's a bug in some tool, and we can report it.


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mk: don't escape '#' for `grep`

[Florian Weimer]

* Alejandro Colomar:

> Could you please try to find out the cause of the actual problem?  Maybe
> there's a fix that doesn't involve reverting that patch.  Or maybe
> there's a bug in some tool, and we can report it.

NEWS in GNU make explains this:

* WARNING: Backward-incompatibility!
  Number signs (#) appearing inside a macro reference or function invocation
  no longer introduce comments and should not be escaped with backslashes:
  thus a call such as:
    foo := $(shell echo '#')
  is legal.  Previously the number sign needed to be escaped, for example:
    foo := $(shell echo '\#')
  Now this latter will resolve to "\#".  If you want to write makefiles
  portable to both versions, assign the number sign to a variable:
    H := \#
    foo := $(shell echo '$H')
  This was claimed to be fixed in 3.81, but wasn't, for some reason.
  To detect this change search for 'nocomment' in the .FEATURES variable.

Applying the $H suggestion seems harmless enough, although this make
change seems to be fairly problematic in general.

Re: [PATCH] mk: don't escape '#' for `grep`

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

Hi Florian,

On Thu, Feb 27, 2025 at 03:43:29PM +0100, Florian Weimer wrote:
> * Alejandro Colomar:
> 
> > Could you please try to find out the cause of the actual problem?  Maybe
> > there's a fix that doesn't involve reverting that patch.  Or maybe
> > there's a bug in some tool, and we can report it.
> 
> NEWS in GNU make explains this:
> 
> * WARNING: Backward-incompatibility!
>   Number signs (#) appearing inside a macro reference or function invocation
>   no longer introduce comments and should not be escaped with backslashes:
>   thus a call such as:
>     foo := $(shell echo '#')
>   is legal.  Previously the number sign needed to be escaped, for example:
>     foo := $(shell echo '\#')
>   Now this latter will resolve to "\#".  If you want to write makefiles
>   portable to both versions, assign the number sign to a variable:
>     H := \#
>     foo := $(shell echo '$H')
>   This was claimed to be fixed in 3.81, but wasn't, for some reason.
>   To detect this change search for 'nocomment' in the .FEATURES variable.

Thanks for pasting the relevant NEWS entry.

> 
> Applying the $H suggestion seems harmless enough, although this make
> change seems to be fairly problematic in general.

However, I learned about that change last month (after Boris's report),
and I think it's not our problem here.  The thing is, on my system,
grep(1) is just fine with that escaped hash:

	$ echo '#foo' | grep '\#foo'
	#foo

While Sergei seems to be getting a warning from grep(1).  So, something
in our systems differs, even if our make(1) version is the same.

Our bash(1) version is also the same (sh(1) is not, but we're not using
sh(1), AFAIK).  So something other than make(1) seems to be treating the
\# differently.


Have a lovely day!
Alex


-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mk: don't escape '#' for `grep`

[Jakub Wilk]

* Alejandro Colomar <alx@kernel.org>, 2025-02-27 16:20:
>>   Now this latter will resolve to "\#".  If you want to write makefiles
>>   portable to both versions, assign the number sign to a variable:
>>     H := \#
>>     foo := $(shell echo '$H')

Alternatively, you could take advantage of the fact that in some shell 
contexts # and \# mean the same thing:

   foo := $(shell echo ''\#)

>The thing is, on my system, grep(1) is just fine with that escaped 
>hash:
>
>	$ echo '#foo' | grep '\#foo'
>	#foo

The warning comes from GNU grep (>= 3.8): "Regular expressions with 
stray backslashes now cause warnings, as their unspecified behavior can 
lead to unexpected results."

You're probably not seeing the warnings because Debian patched them out:
https://bugs.debian.org/1019724

-- 
Jakub Wilk

Re: [PATCH] mk: don't escape '#' for `grep`

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

Hi Jakub,

On Thu, Feb 27, 2025 at 05:28:00PM +0100, Jakub Wilk wrote:
> * Alejandro Colomar <alx@kernel.org>, 2025-02-27 16:20:
> > >   Now this latter will resolve to "\#".  If you want to write makefiles
> > >   portable to both versions, assign the number sign to a variable:
> > >     H := \#
> > >     foo := $(shell echo '$H')
> 
> Alternatively, you could take advantage of the fact that in some shell
> contexts # and \# mean the same thing:
> 
>   foo := $(shell echo ''\#)

Hmmm, sounds reasonable, since a # not preceeded by a space is not a
comment.

	alx@debian:~$ echo ''\#
	#
	alx@debian:~$ echo ''#
	#

I'll take this workaround.  I'll put this on the commit:

	Reported-by: Sergei Trofimovich <slyich@gmail.com>
	Cc: Florian Weimer <fw@deneb.enyo.de>
	Cc: Boris Pigin <boris.pigin@gmail.com>
	Suggested-by: Jakub Wilk <jwilk@jwilk.net>

> 
> > The thing is, on my system, grep(1) is just fine with that escaped hash:
> > 
> > 	$ echo '#foo' | grep '\#foo'
> > 	#foo
> 
> The warning comes from GNU grep (>= 3.8): "Regular expressions with stray
> backslashes now cause warnings, as their unspecified behavior can lead to
> unexpected results."
> 
> You're probably not seeing the warnings because Debian patched them out:
> https://bugs.debian.org/1019724

Ahhh, that makes sense.  Thanks!


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mk: don't escape '#' for `grep`

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

Hi all,

On Thu, Feb 27, 2025 at 08:00:20PM +0100, Alejandro Colomar wrote:
> > Alternatively, you could take advantage of the fact that in some shell
> > contexts # and \# mean the same thing:
> > 
> >   foo := $(shell echo ''\#)
> 
> Hmmm, sounds reasonable, since a # not preceeded by a space is not a
> comment.
> 
> 	alx@debian:~$ echo ''\#
> 	#
> 	alx@debian:~$ echo ''#
> 	#
> 
> I'll take this workaround.  I'll put this on the commit:
> 
> 	Reported-by: Sergei Trofimovich <slyich@gmail.com>
> 	Cc: Florian Weimer <fw@deneb.enyo.de>
> 	Cc: Boris Pigin <boris.pigin@gmail.com>
> 	Suggested-by: Jakub Wilk <jwilk@jwilk.net>

I've applied the suggestion by Jakub, which you can find here:
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=46b7bcaae3cbd18b3022caa3ddd52e7be66bb21a>

Tomorrow I'll push that to the master branch, if there are no concerns.

Since this probably affects packagers, I'll issue a new release in a few
days.


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]