* [PATCH] kbuild: escape single backslashes in macro make-cmd @ 2014-07-26 16:35 Konstantin Khlebnikov 2014-08-06 11:45 ` Michal Marek 0 siblings, 1 reply; 8+ messages in thread From: Konstantin Khlebnikov @ 2014-07-26 16:35 UTC (permalink / raw) To: Michal Marek, linux-kernel, linux-kbuild; +Cc: Sascha Hauer, x86, Andrew Morton 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. ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kbuild: escape single backslashes in macro make-cmd 2014-07-26 16:35 [PATCH] kbuild: escape single backslashes in macro make-cmd Konstantin Khlebnikov @ 2014-08-06 11:45 ` Michal Marek 2014-08-06 12:19 ` Konstantin Khlebnikov 0 siblings, 1 reply; 8+ messages in thread From: Michal Marek @ 2014-08-06 11:45 UTC (permalink / raw) To: Konstantin Khlebnikov Cc: linux-kernel, linux-kbuild, Sascha Hauer, x86, Andrew Morton 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kbuild: escape single backslashes in macro make-cmd 2014-08-06 11:45 ` Michal Marek @ 2014-08-06 12:19 ` Konstantin Khlebnikov 2014-08-06 15:01 ` Michal Marek 0 siblings, 1 reply; 8+ messages in thread From: Konstantin Khlebnikov @ 2014-08-06 12:19 UTC (permalink / raw) To: Michal Marek Cc: Linux Kernel Mailing List, linux-kbuild, Sascha Hauer, x86, Andrew Morton 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kbuild: escape single backslashes in macro make-cmd 2014-08-06 12:19 ` Konstantin Khlebnikov @ 2014-08-06 15:01 ` Michal Marek 2014-08-07 16:07 ` Michal Marek 0 siblings, 1 reply; 8+ messages in thread From: Michal Marek @ 2014-08-06 15:01 UTC (permalink / raw) To: Konstantin Khlebnikov Cc: Linux Kernel Mailing List, linux-kbuild, Sascha Hauer, x86, Andrew Morton 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kbuild: escape single backslashes in macro make-cmd 2014-08-06 15:01 ` Michal Marek @ 2014-08-07 16:07 ` Michal Marek 2014-08-07 16:54 ` Konstantin Khlebnikov 2014-08-07 17:08 ` Sam Ravnborg 0 siblings, 2 replies; 8+ messages in thread From: Michal Marek @ 2014-08-07 16:07 UTC (permalink / raw) To: Konstantin Khlebnikov Cc: Linux Kernel Mailing List, linux-kbuild, Sascha Hauer, x86, Andrew Morton 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) ), \ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kbuild: escape single backslashes in macro make-cmd 2014-08-07 16:07 ` Michal Marek @ 2014-08-07 16:54 ` Konstantin Khlebnikov 2014-08-07 17:08 ` Sam Ravnborg 1 sibling, 0 replies; 8+ messages in thread From: Konstantin Khlebnikov @ 2014-08-07 16:54 UTC (permalink / raw) To: Michal Marek Cc: Linux Kernel Mailing List, linux-kbuild, Sascha Hauer, x86, Andrew Morton 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) ), \ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kbuild: escape single backslashes in macro make-cmd 2014-08-07 16:07 ` Michal Marek 2014-08-07 16:54 ` Konstantin Khlebnikov @ 2014-08-07 17:08 ` Sam Ravnborg 2014-08-07 19:56 ` [PATCH] kbuild: Fix handling of backslashes in *.cmd files Michal Marek 1 sibling, 1 reply; 8+ messages in thread From: Sam Ravnborg @ 2014-08-07 17:08 UTC (permalink / raw) To: Michal Marek Cc: Konstantin Khlebnikov, Linux Kernel Mailing List, linux-kbuild, Sascha Hauer, x86, Andrew Morton > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] kbuild: Fix handling of backslashes in *.cmd files 2014-08-07 17:08 ` Sam Ravnborg @ 2014-08-07 19:56 ` Michal Marek 0 siblings, 0 replies; 8+ messages in thread From: Michal Marek @ 2014-08-07 19:56 UTC (permalink / raw) To: linux-kbuild Cc: Sam Ravnborg, Konstantin Khlebnikov, Sascha Hauer, x86, Andrew Morton, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-08-07 19:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-26 16:35 [PATCH] kbuild: escape single backslashes in macro make-cmd Konstantin Khlebnikov 2014-08-06 11:45 ` Michal Marek 2014-08-06 12:19 ` Konstantin Khlebnikov 2014-08-06 15:01 ` Michal Marek 2014-08-07 16:07 ` Michal Marek 2014-08-07 16:54 ` Konstantin Khlebnikov 2014-08-07 17:08 ` Sam Ravnborg 2014-08-07 19:56 ` [PATCH] kbuild: Fix handling of backslashes in *.cmd files Michal Marek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).