[PATCH] kbuild: escape single backslashes in macro make-cmd

[PATCH] kbuild: escape single backslashes in macro make-cmd

[Konstantin Khlebnikov]

This already has been fixed in commit c353acba28fb3fa1fd05fd
("kbuild: make: fix if_changed when command contains backslashes")
but escaping still isn't perfect and triggers false-positive rebuilds.

For x86 problem happens every time, because rules in arch/x86/realmode/rm/
and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
Macro if_changed detects command change and rebuilds target again and again.

Backslash escaping conflicts with other passes because it's used for escaping
other symbols. To avoid that current macro handles only double backslashes.
Obviously this doesn't work for \1 like above.

This patch reorders passes. It doubles all backslashes before escaping # and '

Visible effect in rebuilding x86/defconfig without changes, before patch:

blind@zurg:~/src/linux$ make V=2
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh - due to target missing
  CHK     include/generated/compile.h
  PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change
  LDS     arch/x86/realmode/rm/realmode.lds - due to: arch/x86/realmode/rm/pasyms.h
  LD      arch/x86/realmode/rm/realmode.elf - due to: arch/x86/realmode/rm/realmode.lds
  RELOCS  arch/x86/realmode/rm/realmode.relocs - due to: arch/x86/realmode/rm/realmode.elf
  OBJCOPY arch/x86/realmode/rm/realmode.bin - due to: arch/x86/realmode/rm/realmode.elf arch/x86/realmode/rm/realmode.relocs
  AS      arch/x86/realmode/rmpiggy.o - due to: arch/x86/realmode/rm/realmode.bin
  LD      arch/x86/realmode/built-in.o - due to: arch/x86/realmode/rmpiggy.o
  LD      arch/x86/built-in.o - due to: arch/x86/realmode/built-in.o
  LINK    vmlinux - due to: arch/x86/built-in.o
  LD      vmlinux.o
  MODPOST vmlinux.o - due to vmlinux.o not in $(targets)
  GEN     .version
  CHK     include/generated/compile.h
  UPD     include/generated/compile.h
  CC      init/version.o - due to: include/generated/compile.h
  LD      init/built-in.o - due to: init/version.o
  KSYM    .tmp_kallsyms1.o
  KSYM    .tmp_kallsyms2.o
  LD      vmlinux
  SORTEX  vmlinux
  SYSMAP  System.map
  VOFFSET arch/x86/boot/voffset.h - due to: vmlinux
  OBJCOPY arch/x86/boot/compressed/vmlinux.bin - due to: vmlinux
  GZIP    arch/x86/boot/compressed/vmlinux.bin.gz - due to: arch/x86/boot/compressed/vmlinux.bin
  MKPIGGY arch/x86/boot/compressed/piggy.S - due to: arch/x86/boot/compressed/vmlinux.bin.gz
  AS      arch/x86/boot/compressed/piggy.o - due to: arch/x86/boot/compressed/piggy.S
  LD      arch/x86/boot/compressed/vmlinux - due to: arch/x86/boot/compressed/piggy.o
  ZOFFSET arch/x86/boot/zoffset.h - due to: arch/x86/boot/compressed/vmlinux
  AS      arch/x86/boot/header.o - due to: arch/x86/boot/voffset.h arch/x86/boot/zoffset.h
  CC      arch/x86/boot/version.o - due to: include/generated/compile.h
  LD      arch/x86/boot/setup.elf - due to: arch/x86/boot/header.o arch/x86/boot/version.o
  OBJCOPY arch/x86/boot/setup.bin - due to: arch/x86/boot/setup.elf
  OBJCOPY arch/x86/boot/vmlinux.bin - due to: arch/x86/boot/compressed/vmlinux
  BUILD   arch/x86/boot/bzImage - due to: arch/x86/boot/setup.bin arch/x86/boot/vmlinux.bin
Setup is 15676 bytes (padded to 15872 bytes).
System is 5569 kB
CRC c06bbc0c
Kernel: arch/x86/boot/bzImage is ready  (#4)
  Building modules, stage 2.
  MODPOST 12 modules - due to target is PHONY

After:

blind@zurg:~/src/linux$ make V=2
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh - due to target missing
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#5)
  Building modules, stage 2.
  MODPOST 12 modules - due to target is PHONY

Signed-off-by: Konstantin Khlebnikov <koct9i@gmail.com>
---
 scripts/Kbuild.include |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 122f95c..edd0b0d 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -219,7 +219,7 @@ endif
 # >$< substitution to preserve $ when reloading .cmd file
 # note: when using inline perl scripts [perl -e '...$$t=1;...']
 # in $(cmd_xxx) double $$ your perl vars
-make-cmd = $(subst \\,\\\\,$(subst \#,\\\#,$(subst $$,$$$$,$(call escsq,$(cmd_$(1))))))
+make-cmd = $(call escsq,$(subst \#,\\\#,$(subst \,\\,$(subst $$,$$$$,$(cmd_$(1))))))
 
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.

Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Michal Marek]

On 2014-07-26 18:35, Konstantin Khlebnikov wrote:
> This already has been fixed in commit c353acba28fb3fa1fd05fd
> ("kbuild: make: fix if_changed when command contains backslashes")
> but escaping still isn't perfect and triggers false-positive rebuilds.
> 
> For x86 problem happens every time, because rules in arch/x86/realmode/rm/
> and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
> Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
> Macro if_changed detects command change and rebuilds target again and again.
> 
> Backslash escaping conflicts with other passes because it's used for escaping
> other symbols. To avoid that current macro handles only double backslashes.
> Obviously this doesn't work for \1 like above.
> 
> This patch reorders passes. It doubles all backslashes before escaping # and '
> 
> Visible effect in rebuilding x86/defconfig without changes, before patch:
> 
> blind@zurg:~/src/linux$ make V=2
>   CHK     include/config/kernel.release
>   CHK     include/generated/uapi/linux/version.h
>   CHK     include/generated/utsrelease.h
>   CALL    scripts/checksyscalls.sh - due to target missing
>   CHK     include/generated/compile.h
>   PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change

With which make and shell version are you seeing this? While the patch
looks correct, I can't reproduce the error here:

$ make V=2
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh - due to target missing
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 12 modules - due to target is PHONY
$ cat arch/x86/realmode/rm/.pasyms.h.cmd
cmd_arch/x86/realmode/rm/pasyms.h := nm arch/x86/realmode/rm/header.o
arch/x86/realmode/rm/trampoline_64.o arch/x86/realmode/rm/stack.o
arch/x86/realmode/rm/reboot.o arch/x86/realmode/rm/wakeup_asm.o
arch/x86/realmode/rm/wakemain.o arch/x86/realmode/rm/video-mode.o
arch/x86/realmode/rm/copy.o arch/x86/realmode/rm/bioscall.o
arch/x86/realmode/rm/regs.o arch/x86/realmode/rm/video-vga.o
arch/x86/realmode/rm/video-vesa.o arch/x86/realmode/rm/video-bios.o |
sed -n -r -e 's/^([0-9a-fA-F]+) [ABCDGRSTVW] (.+)$$/pa_\2 = \2;/p' |
sort | uniq > arch/x86/realmode/rm/pasyms.h

This is with both 3.16 and my kbuild branch, which is based on 3.16-rc1.
Also, in the description, you say that backslash escaping conflicts with
escaping # and ', but this has nothing to do with the \2 backreference
in the sed command.

Michal

Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Konstantin Khlebnikov]

On Wed, Aug 6, 2014 at 3:45 PM, Michal Marek <mmarek@suse.cz> wrote:
> On 2014-07-26 18:35, Konstantin Khlebnikov wrote:
>> This already has been fixed in commit c353acba28fb3fa1fd05fd
>> ("kbuild: make: fix if_changed when command contains backslashes")
>> but escaping still isn't perfect and triggers false-positive rebuilds.
>>
>> For x86 problem happens every time, because rules in arch/x86/realmode/rm/
>> and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
>> Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
>> Macro if_changed detects command change and rebuilds target again and again.
>>
>> Backslash escaping conflicts with other passes because it's used for escaping
>> other symbols. To avoid that current macro handles only double backslashes.
>> Obviously this doesn't work for \1 like above.
>>
>> This patch reorders passes. It doubles all backslashes before escaping # and '
>>
>> Visible effect in rebuilding x86/defconfig without changes, before patch:
>>
>> blind@zurg:~/src/linux$ make V=2
>>   CHK     include/config/kernel.release
>>   CHK     include/generated/uapi/linux/version.h
>>   CHK     include/generated/utsrelease.h
>>   CALL    scripts/checksyscalls.sh - due to target missing
>>   CHK     include/generated/compile.h
>>   PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change
>
> With which make and shell version are you seeing this? While the patch
> looks correct, I can't reproduce the error here:

/bin/sh points to dash (debian default setup).

I cannot reproduce this using bash. That explains why this bug is still here.

>
> $ make V=2
>   CHK     include/config/kernel.release
>   CHK     include/generated/uapi/linux/version.h
>   CHK     include/generated/utsrelease.h
>   CALL    scripts/checksyscalls.sh - due to target missing
>   CHK     include/generated/compile.h
> Kernel: arch/x86/boot/bzImage is ready  (#1)
>   Building modules, stage 2.
>   MODPOST 12 modules - due to target is PHONY
> $ cat arch/x86/realmode/rm/.pasyms.h.cmd
> cmd_arch/x86/realmode/rm/pasyms.h := nm arch/x86/realmode/rm/header.o
> arch/x86/realmode/rm/trampoline_64.o arch/x86/realmode/rm/stack.o
> arch/x86/realmode/rm/reboot.o arch/x86/realmode/rm/wakeup_asm.o
> arch/x86/realmode/rm/wakemain.o arch/x86/realmode/rm/video-mode.o
> arch/x86/realmode/rm/copy.o arch/x86/realmode/rm/bioscall.o
> arch/x86/realmode/rm/regs.o arch/x86/realmode/rm/video-vga.o
> arch/x86/realmode/rm/video-vesa.o arch/x86/realmode/rm/video-bios.o |
> sed -n -r -e 's/^([0-9a-fA-F]+) [ABCDGRSTVW] (.+)$$/pa_\2 = \2;/p' |
> sort | uniq > arch/x86/realmode/rm/pasyms.h
>
> This is with both 3.16 and my kbuild branch, which is based on 3.16-rc1.
> Also, in the description, you say that backslash escaping conflicts with
> escaping # and ', but this has nothing to do with the \2 backreference
> in the sed command.

I'm not quite understand what the difference between escaping standalone
backslash ( ' \ ' -> \' \\ \' ) that backreference (  ' \2 ' -> \' \\2 \' )

We have to double backslashes, but this must be done before escaping # and ',
otherwise doubling undoes their escaping.

>
> Michal

Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Michal Marek]

On 2014-08-06 14:19, Konstantin Khlebnikov wrote:
> On Wed, Aug 6, 2014 at 3:45 PM, Michal Marek <mmarek@suse.cz> wrote:
>> On 2014-07-26 18:35, Konstantin Khlebnikov wrote:
>>> This already has been fixed in commit c353acba28fb3fa1fd05fd
>>> ("kbuild: make: fix if_changed when command contains backslashes")
>>> but escaping still isn't perfect and triggers false-positive rebuilds.
>>>
>>> For x86 problem happens every time, because rules in arch/x86/realmode/rm/
>>> and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
>>> Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
>>> Macro if_changed detects command change and rebuilds target again and again.
>>>
>>> Backslash escaping conflicts with other passes because it's used for escaping
>>> other symbols. To avoid that current macro handles only double backslashes.
>>> Obviously this doesn't work for \1 like above.
>>>
>>> This patch reorders passes. It doubles all backslashes before escaping # and '
>>>
>>> Visible effect in rebuilding x86/defconfig without changes, before patch:
>>>
>>> blind@zurg:~/src/linux$ make V=2
>>>   CHK     include/config/kernel.release
>>>   CHK     include/generated/uapi/linux/version.h
>>>   CHK     include/generated/utsrelease.h
>>>   CALL    scripts/checksyscalls.sh - due to target missing
>>>   CHK     include/generated/compile.h
>>>   PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change
>>
>> With which make and shell version are you seeing this? While the patch
>> looks correct, I can't reproduce the error here:
> 
> /bin/sh points to dash (debian default setup).
> 
> I cannot reproduce this using bash. That explains why this bug is still here.

So the difference between the shells is that their 'echo' builtin treats
\<number> differently:
$ ./dash -c "echo '\2'"
\x02\x02
$ ./dash -c "echo '\2'" | xxd
0000000: 020a                                     ..
$ /bin/bash -c "echo '\2'"
\2

For some reason we fail to escape the \ and dash treats \2 as \02. POSIX
says that this is implementation-defined, so none of the shells is wrong.

I need to look into this a bit further. I'll most likely end up applying
your patch, but I'd like to understand the fix in detail first.

Thanks,
Michal

Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Michal Marek]

On Wed, Aug 06, 2014 at 05:01:57PM +0200, Michal Marek wrote:
> On 2014-08-06 14:19, Konstantin Khlebnikov wrote:
> > On Wed, Aug 6, 2014 at 3:45 PM, Michal Marek <mmarek@suse.cz> wrote:
> >> On 2014-07-26 18:35, Konstantin Khlebnikov wrote:
> >>> This already has been fixed in commit c353acba28fb3fa1fd05fd
> >>> ("kbuild: make: fix if_changed when command contains backslashes")
> >>> but escaping still isn't perfect and triggers false-positive rebuilds.
> >>>
> >>> For x86 problem happens every time, because rules in arch/x86/realmode/rm/
> >>> and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
> >>> Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
> >>> Macro if_changed detects command change and rebuilds target again and again.
> >>>
> >>> Backslash escaping conflicts with other passes because it's used for escaping
> >>> other symbols. To avoid that current macro handles only double backslashes.
> >>> Obviously this doesn't work for \1 like above.
> >>>
> >>> This patch reorders passes. It doubles all backslashes before escaping # and '
> >>>
> >>> Visible effect in rebuilding x86/defconfig without changes, before patch:
> >>>
> >>> blind@zurg:~/src/linux$ make V=2
> >>>   CHK     include/config/kernel.release
> >>>   CHK     include/generated/uapi/linux/version.h
> >>>   CHK     include/generated/utsrelease.h
> >>>   CALL    scripts/checksyscalls.sh - due to target missing
> >>>   CHK     include/generated/compile.h
> >>>   PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change
> >>
> >> With which make and shell version are you seeing this? While the patch
> >> looks correct, I can't reproduce the error here:
> > 
> > /bin/sh points to dash (debian default setup).
> > 
> > I cannot reproduce this using bash. That explains why this bug is still here.
> 
> So the difference between the shells is that their 'echo' builtin treats
> \<number> differently:
> $ ./dash -c "echo '\2'"
> \x02\x02
> $ ./dash -c "echo '\2'" | xxd
> 0000000: 020a                                     ..
> $ /bin/bash -c "echo '\2'"
> \2

The problem is that the bash echo does not interpred _any_ \-sequences
without -e. So the nicely escaped \\ stays a \\ and we get spurious
rebuilds with bash instead. It previously worked with bash, because the
backslash escaping was completely broken. You not only moved it up in
the chain, but also fixed it by removing one superflous level of
escaping.

I'm now testing this, i.e. dropping the backslash escaping completely
and doing a printf '%s\n' instead. Can you try if it works for you?


diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 122f95c..8a9a4e1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -215,11 +215,13 @@ else
 arg-check = $(if $(strip $(cmd_$@)),,1)
 endif
 
-# >'< substitution is for echo to work,
-# >$< substitution to preserve $ when reloading .cmd file
-# note: when using inline perl scripts [perl -e '...$$t=1;...']
-# in $(cmd_xxx) double $$ your perl vars
-make-cmd = $(subst \\,\\\\,$(subst \#,\\\#,$(subst $$,$$$$,$(call escsq,$(cmd_$(1))))))
+# Replace >$< with >$$< to preserve $ when reloading the .cmd file
+# (needed for make)
+# Replace >#< with >\#< to avoid starting a comment in the .cmd file
+# (needed for make)
+# Replace >'< with >'\''< to be able to enclose the whole string in '...'
+# (needed for the shell)
+make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
 
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.
@@ -230,7 +232,7 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
 if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
 	@set -e;                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
-	echo 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
+	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
 
 # Execute the command and also postprocess generated .d dependencies file.
 if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \

Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Konstantin Khlebnikov]

On Thu, Aug 7, 2014 at 8:07 PM, Michal Marek <mmarek@suse.cz> wrote:
> On Wed, Aug 06, 2014 at 05:01:57PM +0200, Michal Marek wrote:
>> On 2014-08-06 14:19, Konstantin Khlebnikov wrote:
>> > On Wed, Aug 6, 2014 at 3:45 PM, Michal Marek <mmarek@suse.cz> wrote:
>> >> On 2014-07-26 18:35, Konstantin Khlebnikov wrote:
>> >>> This already has been fixed in commit c353acba28fb3fa1fd05fd
>> >>> ("kbuild: make: fix if_changed when command contains backslashes")
>> >>> but escaping still isn't perfect and triggers false-positive rebuilds.
>> >>>
>> >>> For x86 problem happens every time, because rules in arch/x86/realmode/rm/
>> >>> and arch/x86/boot/ contains commands like sed -n -e 's/foo\(.*\)/\1/p'.
>> >>> Backslash in \1 isn't escaped and turns into ascii symbol with code 1.
>> >>> Macro if_changed detects command change and rebuilds target again and again.
>> >>>
>> >>> Backslash escaping conflicts with other passes because it's used for escaping
>> >>> other symbols. To avoid that current macro handles only double backslashes.
>> >>> Obviously this doesn't work for \1 like above.
>> >>>
>> >>> This patch reorders passes. It doubles all backslashes before escaping # and '
>> >>>
>> >>> Visible effect in rebuilding x86/defconfig without changes, before patch:
>> >>>
>> >>> blind@zurg:~/src/linux$ make V=2
>> >>>   CHK     include/config/kernel.release
>> >>>   CHK     include/generated/uapi/linux/version.h
>> >>>   CHK     include/generated/utsrelease.h
>> >>>   CALL    scripts/checksyscalls.sh - due to target missing
>> >>>   CHK     include/generated/compile.h
>> >>>   PASYMS  arch/x86/realmode/rm/pasyms.h - due to command line change
>> >>
>> >> With which make and shell version are you seeing this? While the patch
>> >> looks correct, I can't reproduce the error here:
>> >
>> > /bin/sh points to dash (debian default setup).
>> >
>> > I cannot reproduce this using bash. That explains why this bug is still here.
>>
>> So the difference between the shells is that their 'echo' builtin treats
>> \<number> differently:
>> $ ./dash -c "echo '\2'"
>>
>> $ ./dash -c "echo '\2'" | xxd
>> 0000000: 020a                                     ..
>> $ /bin/bash -c "echo '\2'"
>> \2
>
> The problem is that the bash echo does not interpred _any_ \-sequences
> without -e. So the nicely escaped \\ stays a \\ and we get spurious
> rebuilds with bash instead. It previously worked with bash, because the
> backslash escaping was completely broken. You not only moved it up in
> the chain, but also fixed it by removing one superflous level of
> escaping.
>
> I'm now testing this, i.e. dropping the backslash escaping completely
> and doing a printf '%s\n' instead. Can you try if it works for you?

Yes, it works. I completely forgot about printf, it's much more
reliable than echo.

So, you can add my Acked-by/Tested-by

>
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 122f95c..8a9a4e1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -215,11 +215,13 @@ else
>  arg-check = $(if $(strip $(cmd_$@)),,1)
>  endif
>
> -# >'< substitution is for echo to work,
> -# >$< substitution to preserve $ when reloading .cmd file
> -# note: when using inline perl scripts [perl -e '...$$t=1;...']
> -# in $(cmd_xxx) double $$ your perl vars
> -make-cmd = $(subst \\,\\\\,$(subst \#,\\\#,$(subst $$,$$$$,$(call escsq,$(cmd_$(1))))))
> +# Replace >$< with >$$< to preserve $ when reloading the .cmd file
> +# (needed for make)
> +# Replace >#< with >\#< to avoid starting a comment in the .cmd file
> +# (needed for make)
> +# Replace >'< with >'\''< to be able to enclose the whole string in '...'
> +# (needed for the shell)
> +make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
>
>  # Find any prerequisites that is newer than target or that does not exist.
>  # PHONY targets skipped in both cases.
> @@ -230,7 +232,7 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
>  if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
>         @set -e;                                                             \
>         $(echo-cmd) $(cmd_$(1));                                             \
> -       echo 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
> +       printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
>
>  # Execute the command and also postprocess generated .d dependencies file.
>  if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \

Re: [PATCH] kbuild: escape single backslashes in macro make-cmd

[Sam Ravnborg]

> 
> The problem is that the bash echo does not interpred _any_ \-sequences
> without -e. So the nicely escaped \\ stays a \\ and we get spurious
> rebuilds with bash instead. It previously worked with bash, because the
> backslash escaping was completely broken. You not only moved it up in
> the chain, but also fixed it by removing one superflous level of
> escaping.
> 
> I'm now testing this, i.e. dropping the backslash escaping completely
> and doing a printf '%s\n' instead. Can you try if it works for you?
> 
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 122f95c..8a9a4e1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -215,11 +215,13 @@ else
>  arg-check = $(if $(strip $(cmd_$@)),,1)
>  endif
>  
> -# >'< substitution is for echo to work,
> -# >$< substitution to preserve $ when reloading .cmd file
> -# note: when using inline perl scripts [perl -e '...$$t=1;...']
> -# in $(cmd_xxx) double $$ your perl vars
> -make-cmd = $(subst \\,\\\\,$(subst \#,\\\#,$(subst $$,$$$$,$(call escsq,$(cmd_$(1))))))
> +# Replace >$< with >$$< to preserve $ when reloading the .cmd file
> +# (needed for make)
> +# Replace >#< with >\#< to avoid starting a comment in the .cmd file
> +# (needed for make)
> +# Replace >'< with >'\''< to be able to enclose the whole string in '...'
> +# (needed for the shell)
> +make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
This is simpler and remotely readble - nice!
As a bonus you added more descriptive comments

Looks good.
You may add my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

[PATCH] kbuild: Fix handling of backslashes in *.cmd files

[Michal Marek]

Commit c353acba ("kbuild: make: fix if_changed when command contains
backslashes") attempted to handle backslashes in *.cmd files, but it
only handled double backslashes for some reason. Changing make-cmd to also
handle single backslashes fixes rebuilds with dash, but it breaks bash
again. The reason is that the two shells disagree about the
interpretation of backslash sequences in the echo builtin. The way out
of this is to print the command with printf '%s\n'. While at it,
document what the individual parts of make-cmd do and why.

Reported-and-tested-by: Konstantin Khlebnikov <koct9i@gmail.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 scripts/Kbuild.include | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 122f95c..8a9a4e1 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -215,11 +215,13 @@ else
 arg-check = $(if $(strip $(cmd_$@)),,1)
 endif
 
-# >'< substitution is for echo to work,
-# >$< substitution to preserve $ when reloading .cmd file
-# note: when using inline perl scripts [perl -e '...$$t=1;...']
-# in $(cmd_xxx) double $$ your perl vars
-make-cmd = $(subst \\,\\\\,$(subst \#,\\\#,$(subst $$,$$$$,$(call escsq,$(cmd_$(1))))))
+# Replace >$< with >$$< to preserve $ when reloading the .cmd file
+# (needed for make)
+# Replace >#< with >\#< to avoid starting a comment in the .cmd file
+# (needed for make)
+# Replace >'< with >'\''< to be able to enclose the whole string in '...'
+# (needed for the shell)
+make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
 
 # Find any prerequisites that is newer than target or that does not exist.
 # PHONY targets skipped in both cases.
@@ -230,7 +232,7 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
 if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
 	@set -e;                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
-	echo 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
+	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd)
 
 # Execute the command and also postprocess generated .d dependencies file.
 if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
-- 
1.8.4.5