* [PATCH] kbuild: fix if_change and friends to consider argument order
@ 2016-05-05 7:45 Masahiro Yamada
2016-05-05 8:08 ` Woodhouse, David
0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2016-05-05 7:45 UTC (permalink / raw)
To: linux-kbuild
Cc: Masahiro Yamada, Michal Marek, David Howells, David Woodhouse,
linux-kernel
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(-)
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
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: fix if_change and friends to consider argument order
2016-05-05 7:45 [PATCH] kbuild: fix if_change and friends to consider argument order Masahiro Yamada
@ 2016-05-05 8:08 ` Woodhouse, David
2016-05-05 14:49 ` Masahiro Yamada
2016-05-06 2:12 ` Masahiro Yamada
0 siblings, 2 replies; 5+ messages in thread
From: Woodhouse, David @ 2016-05-05 8:08 UTC (permalink / raw)
To: linux-kbuild, yamada.masahiro; +Cc: mmarek, linux-kernel, dhowells
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
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
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3437 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: fix if_change and friends to consider argument order
2016-05-05 8:08 ` Woodhouse, David
@ 2016-05-05 14:49 ` Masahiro Yamada
2016-05-05 18:00 ` Masahiro Yamada
2016-05-06 2:12 ` Masahiro Yamada
1 sibling, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2016-05-05 14:49 UTC (permalink / raw)
To: Woodhouse, David; +Cc: linux-kbuild, mmarek, linux-kernel, dhowells
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.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: fix if_change and friends to consider argument order
2016-05-05 14:49 ` Masahiro Yamada
@ 2016-05-05 18:00 ` Masahiro Yamada
0 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2016-05-05 18:00 UTC (permalink / raw)
To: Woodhouse, David; +Cc: linux-kbuild, mmarek, linux-kernel, dhowells
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.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kbuild: fix if_change and friends to consider argument order
2016-05-05 8:08 ` Woodhouse, David
2016-05-05 14:49 ` Masahiro Yamada
@ 2016-05-06 2:12 ` Masahiro Yamada
1 sibling, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2016-05-06 2:12 UTC (permalink / raw)
To: Woodhouse, David; +Cc: linux-kbuild, mmarek, linux-kernel, dhowells
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.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-06 2:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-05 7:45 [PATCH] kbuild: fix if_change and friends to consider argument order Masahiro Yamada
2016-05-05 8:08 ` Woodhouse, David
2016-05-05 14:49 ` Masahiro Yamada
2016-05-05 18:00 ` Masahiro Yamada
2016-05-06 2:12 ` Masahiro Yamada
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).