| Message ID | 1462434312-19422-1-git-send-email-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2016-05-05 at 16:45 +0900, Masahiro Yamada wrote: > > This commit fixes arg-check to compare two strings as a whole. > $(strip ...) is important because we want to ignore the difference > that comes from white-spaces. Do we? I can construct a hypothetical situation in which whitespace differs and we *do* want it to make a difference (for example I used to sign with a key called 'My Signing Key.pem' and now I've changed to use 'My Signing Key.pem'. (OK, it's a *stupid* example but still...) I couldn't come up with the converse — where whitespace does change for some reason, but we really don't want to rebuild. Should we err on the side of caution, and let whitespace changes trigger a rebuild?
2016-05-05 17:08 GMT+09:00 Woodhouse, David <david.woodhouse@intel.com>: > On Thu, 2016-05-05 at 16:45 +0900, Masahiro Yamada wrote: >> >> This commit fixes arg-check to compare two strings as a whole. >> $(strip ...) is important because we want to ignore the difference >> that comes from white-spaces. > > Do we? > > I can construct a hypothetical situation in which whitespace differs > and we *do* want it to make a difference (for example I used to sign > with a key called 'My Signing Key.pem' and now I've changed to use > 'My Signing Key.pem'. (OK, it's a *stupid* example but still...) > > I couldn't come up with the converse — where whitespace does change for > some reason, but we really don't want to rebuild. > > Should we err on the side of caution, and let whitespace changes > trigger a rebuild? > > -- > David Woodhouse Open Source Technology Centre > David.Woodhouse@intel.com Intel Corporation > Please hold on. I noticed some side effect on this patch. I need to test it more carefully.
2016-05-05 23:49 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: > 2016-05-05 17:08 GMT+09:00 Woodhouse, David <david.woodhouse@intel.com>: >> On Thu, 2016-05-05 at 16:45 +0900, Masahiro Yamada wrote: >>> >>> This commit fixes arg-check to compare two strings as a whole. >>> $(strip ...) is important because we want to ignore the difference >>> that comes from white-spaces. >> >> Do we? >> >> I can construct a hypothetical situation in which whitespace differs >> and we *do* want it to make a difference (for example I used to sign >> with a key called 'My Signing Key.pem' and now I've changed to use >> 'My Signing Key.pem'. (OK, it's a *stupid* example but still...) >> >> I couldn't come up with the converse — where whitespace does change for >> some reason, but we really don't want to rebuild. >> >> Should we err on the side of caution, and let whitespace changes >> trigger a rebuild? >> >> -- >> David Woodhouse Open Source Technology Centre >> David.Woodhouse@intel.com Intel Corporation >> > > > > Please hold on. > > I noticed some side effect on this patch. > > I need to test it more carefully. This patch is not working at all. Please disregard it. Probably, I will send v2 in a few days.
Hi David, 2016-05-05 17:08 GMT+09:00 Woodhouse, David <david.woodhouse@intel.com>: > On Thu, 2016-05-05 at 16:45 +0900, Masahiro Yamada wrote: >> >> This commit fixes arg-check to compare two strings as a whole. >> $(strip ...) is important because we want to ignore the difference >> that comes from white-spaces. > > Do we? > > I can construct a hypothetical situation in which whitespace differs > and we *do* want it to make a difference (for example I used to sign > with a key called 'My Signing Key.pem' and now I've changed to use > 'My Signing Key.pem'. (OK, it's a *stupid* example but still...) > Have you ever succeeded in passing such a string in Kbuild in the first place? For example, I added the following line into init/Makefile CFLAGS_main.o += -DHELLO_WORLD='"hello world!"' and printk("%s\n", HELLO_WORLD); to start_kernel(). But, I got the console log [ 0.001639] hello world! The root cause of this problem is the following line _c_flags = $(filter-out $(CFLAGS_REMOVE_$(basetarget).o), $(orig_c_flags)) $(filter-out ) strips extra spaces, so Kbuild can not keep white-spaces as they are. Maybe, we can fix this problem. (This is another problem, though) Thank you for spotting this.
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index b2ab2a9..2d03480 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -228,8 +228,8 @@ objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o))) ifneq ($(KBUILD_NOCMDDEP),1) # Check if both arguments has same arguments. Result is empty string if equal. # User may override this check using make KBUILD_NOCMDDEP=1 -arg-check = $(strip $(filter-out $(cmd_$(1)), $(cmd_$@)) \ - $(filter-out $(cmd_$@), $(cmd_$(1))) ) +arg-check = $(filter-out $(quote)$(strip $(cmd_$1))$(quote), \ + $(quote)$(strip $(cmd_$@))$(quote)) else arg-check = $(if $(strip $(cmd_$@)),,1) endif
Currently, arg-check is implemented as follows: arg-check = $(strip $(filter-out $(cmd_$(1)), $(cmd_$@)) \ $(filter-out $(cmd_$@), $(cmd_$(1))) ) This does not care about the order of arguments that appear in $(cmd_$(1)) and $(cmd_$@). So, if_changed and friends never rebuild the target if only the argument order is changed. This is a problem when the link order is changed. Apparently, obj-y += foo.o obj-y += bar.o and obj-y += bar.o obj-y += foo.o should be distinguished because the link order determines the probe order of drivers. So, built-in.o should be rebuilt if the order of objects is changed. This commit fixes arg-check to compare two strings as a whole. $(strip ...) is important because we want to ignore the difference that comes from white-spaces. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- scripts/Kbuild.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)