| Message ID | 20180325230918.23161-1-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2018-03-26 8:09 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > The latest official Make release is 4.2.1 from mid-2016, but the current > git release has this relevant note in the NEWS file: > > * 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: > C := \# > foo := $(shell echo '$C') > 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. > > Prepare for whatever future Make release contains that change by fixing > up the .cmd file escaping - without this, make always thinks the command > string has changed and hence rebuilds everything. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > So the previous patch made everything build, but building again > revealed that this central place very much also needed fixing. > > scripts/Kbuild.include | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include > index 065324a8046f..7a926e4688f4 100644 > --- a/scripts/Kbuild.include > +++ b/scripts/Kbuild.include > @@ -10,6 +10,7 @@ space := $(empty) $(empty) > space_escape := _-_SPACE_-_ > right_paren := ) > left_paren := ( > +pound := \# > > ### > # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o > @@ -328,7 +329,7 @@ endif > # (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))))) > +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1))))) > Thanks for the patch, but this changes the behavior. With '#' replaced with $(pound), '\\' does not escape anything, so it is treated as '\\'. The following keeps the current behavior: make-cmd = $(call escsq,$(subst $(pound),\$(pound),$(subst $$,$$$$,$(cmd_$(1))))) But, I think the following is an even better fix: make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1))))) The following 4 test cases produce the same foo.txt, but insane .cmd file is produced for some case. Adjusting the number of back-slashes is not a perfect solution. Replacing # with $(pound) works for all the cases. [test code 1] extra-y := foo.txt quiet_cmd_foo = DEFINE $@ cmd_foo = echo '\#define FOO' >$@ $(obj)/foo.txt: FORCE $(call if_changed,foo) [test code 2] extra-y := foo.txt quiet_cmd_foo = DEFINE $@ cmd_foo = echo \\\#define FOO >$@ $(obj)/foo.txt: FORCE $(call if_changed,foo) [test code3] extra-y := foo.txt quiet_cmd_foo = DEFINE $@ cmd_foo = echo '$(pound)define FOO' >$@ $(obj)/foo.txt: FORCE $(call if_changed,foo) [test code4] extra-y := foo.txt quiet_cmd_foo = DEFINE $@ cmd_foo = echo \$(pound)define FOO >$@ $(obj)/foo.txt: FORCE $(call if_changed,foo) | current | \$(pound) | \\$(pound) | $$(pound) ---------------------------------------------------------- test code1 | OK | OK | NG | OK test code2 | NG | NG | OK | OK test code3 | OK | OK | NG | OK test code4 | NG | NG | OK | OK
On 2018-03-26 13:48, Masahiro Yamada wrote: > 2018-03-26 8:09 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: >> The latest official Make release is 4.2.1 from mid-2016, but the current >> git release has this relevant note in the NEWS file: >> >> * 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: >> C := \# >> foo := $(shell echo '$C') >> 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. >> >> Prepare for whatever future Make release contains that change by fixing >> up the .cmd file escaping - without this, make always thinks the command >> string has changed and hence rebuilds everything. >> >> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> >> --- >> So the previous patch made everything build, but building again >> revealed that this central place very much also needed fixing. >> >> scripts/Kbuild.include | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include >> index 065324a8046f..7a926e4688f4 100644 >> --- a/scripts/Kbuild.include >> +++ b/scripts/Kbuild.include >> @@ -10,6 +10,7 @@ space := $(empty) $(empty) >> space_escape := _-_SPACE_-_ >> right_paren := ) >> left_paren := ( >> +pound := \# >> >> ### >> # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o >> @@ -328,7 +329,7 @@ endif >> # (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))))) >> +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1))))) >> > > Thanks for the patch, but this changes the behavior. > With '#' replaced with $(pound), '\\' does not escape anything, > so it is treated as '\\'. > > > The following keeps the current behavior: > > make-cmd = $(call escsq,$(subst $(pound),\$(pound),$(subst > $$,$$$$,$(cmd_$(1))))) > > > > But, I think the following is an even better fix: > > make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst > $$,$$$$,$(cmd_$(1))))) Makefile quoting has never been my strong suit. I actually thought I tested my change by looking at some .o.cmd files containing \# before and after, but I was fooled by tools/ containing and using their own copy of make-cmd - so tools/build/Build.include will also need fixing. Yes, writing $(pound) to the .cmd file seems like the safest choice. Do you want me to respin or can/will you do the patch(es) yourself? Rasmus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-04-06 6:43 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > On 2018-03-26 13:48, Masahiro Yamada wrote: >> 2018-03-26 8:09 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: >>> The latest official Make release is 4.2.1 from mid-2016, but the current >>> git release has this relevant note in the NEWS file: >>> >>> * 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: >>> C := \# >>> foo := $(shell echo '$C') >>> 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. >>> >>> Prepare for whatever future Make release contains that change by fixing >>> up the .cmd file escaping - without this, make always thinks the command >>> string has changed and hence rebuilds everything. >>> >>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> >>> --- >>> So the previous patch made everything build, but building again >>> revealed that this central place very much also needed fixing. >>> >>> scripts/Kbuild.include | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include >>> index 065324a8046f..7a926e4688f4 100644 >>> --- a/scripts/Kbuild.include >>> +++ b/scripts/Kbuild.include >>> @@ -10,6 +10,7 @@ space := $(empty) $(empty) >>> space_escape := _-_SPACE_-_ >>> right_paren := ) >>> left_paren := ( >>> +pound := \# >>> >>> ### >>> # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o >>> @@ -328,7 +329,7 @@ endif >>> # (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))))) >>> +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1))))) >>> >> >> Thanks for the patch, but this changes the behavior. >> With '#' replaced with $(pound), '\\' does not escape anything, >> so it is treated as '\\'. >> >> >> The following keeps the current behavior: >> >> make-cmd = $(call escsq,$(subst $(pound),\$(pound),$(subst >> $$,$$$$,$(cmd_$(1))))) >> >> >> >> But, I think the following is an even better fix: >> >> make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst >> $$,$$$$,$(cmd_$(1))))) > > Makefile quoting has never been my strong suit. I actually thought I > tested my change by looking at some .o.cmd files containing \# before > and after, but I was fooled by tools/ containing and using their own > copy of make-cmd - so tools/build/Build.include will also need fixing. > > Yes, writing $(pound) to the .cmd file seems like the safest choice. Do > you want me to respin or can/will you do the patch(es) yourself? > Can you send v2 please? I will pick it up shortly.
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 065324a8046f..7a926e4688f4 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -10,6 +10,7 @@ space := $(empty) $(empty) space_escape := _-_SPACE_-_ right_paren := ) left_paren := ( +pound := \# ### # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o @@ -328,7 +329,7 @@ endif # (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))))) +make-cmd = $(call escsq,$(subst $(pound),\\$(pound),$(subst $$,$$$$,$(cmd_$(1))))) # Find any prerequisites that is newer than target or that does not exist. # PHONY targets skipped in both cases.
The latest official Make release is 4.2.1 from mid-2016, but the current git release has this relevant note in the NEWS file: * 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: C := \# foo := $(shell echo '$C') 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. Prepare for whatever future Make release contains that change by fixing up the .cmd file escaping - without this, make always thinks the command string has changed and hence rebuilds everything. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- So the previous patch made everything build, but building again revealed that this central place very much also needed fixing. scripts/Kbuild.include | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)