diff mbox

Kbuild: Avoid DTB rebuilds if source files are untouched

Message ID 5168822C.6040303@wwwdotorg.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Warren April 12, 2013, 9:52 p.m. UTC
On 04/12/2013 01:40 AM, Vineet Gupta wrote:
> On 04/09/2013 07:40 PM, Vineet Gupta wrote:
>> On 04/04/2013 11:06 PM, Stephen Warren wrote:
>>
>>> +.SECONDARY: $(obj)/%.dtb.S
>>> +
>>>  $(obj)/%.dtb.S: $(obj)/%.dtb
>>>         $(call cmd,dt_S_dtb)
>>>
>>> and no longer see make rm'ing the .dtb.S file. So, the .SECONDARY is
>>> behaving as expected, and should fix your problem.
>> Interestingly, if I make the file name explicit, .SECONDARY works for me too.
>>
>> .SECONDARY: $(obj)/angel4.dtb.S
>>
>> http://stackoverflow.com/questions/5426934/why-this-makefile-removes-my-goal
>>
>> Is this a make bug or is it related to when the rule is parsed by make !
>>
>> -Vineet
> 
> Ping ? Anyone know how to resolve this

After installing the ARC toolchain (which was a bit painful to track
down and install...) I reproduced your exact problem. I believe the
patch below fixes it:


--
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

Comments

Vineet Gupta April 15, 2013, 1:59 p.m. UTC | #1
On 04/13/2013 03:22 AM, Stephen Warren wrote:
>
> After installing the ARC toolchain (which was a bit painful to track
> down and install...) 

Sorry about that - we have a buildroot based distro about to be pushed upstream -
that would make it easier.

> I reproduced your exact problem. I believe the
> patch below fixes it:
>
> diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
> index 5776835..2f2cf23 100644
> --- a/arch/arc/boot/dts/Makefile
> +++ b/arch/arc/boot/dts/Makefile
> @@ -8,6 +8,8 @@ endif
>  obj-y   += $(builtindtb-y).dtb.o
>  targets += $(builtindtb-y).dtb
>
> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> +
>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>
>  clean-files := *.dtb

Indeed it does - I fell stupid why this didn't occur to me. But given that you
have dealt with the dtb Makefile stuff alot more than I have :-) do you know why
it was not working when put in Makefile.lib because I feel a few other arches also
suffer from the same issue and would need similar fixes.

Anyhow, for the patch, I can manually add --author="you" but it'll still lack your
SOB - you OK with that or do you want to send a formal patch.

Thx,
-Vineet


--
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
Stephen Warren April 15, 2013, 5:12 p.m. UTC | #2
On 04/15/2013 07:59 AM, Vineet Gupta wrote:
> On 04/13/2013 03:22 AM, Stephen Warren wrote:
>>
>> After installing the ARC toolchain (which was a bit painful to track
>> down and install...) 
> 
> Sorry about that - we have a buildroot based distro about to be pushed upstream -
> that would make it easier.
> 
>> I reproduced your exact problem. I believe the
>> patch below fixes it:
>>
>> diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
>> index 5776835..2f2cf23 100644
>> --- a/arch/arc/boot/dts/Makefile
>> +++ b/arch/arc/boot/dts/Makefile
>> @@ -8,6 +8,8 @@ endif
>>  obj-y   += $(builtindtb-y).dtb.o
>>  targets += $(builtindtb-y).dtb
>>
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>>
>>  clean-files := *.dtb
> 
> Indeed it does - I fell stupid why this didn't occur to me. But given that you
> have dealt with the dtb Makefile stuff alot more than I have :-) do you know why
> it was not working when put in Makefile.lib because I feel a few other arches also
> suffer from the same issue and would need similar fixes.

Searching in Google implies that .SECONDARY doesn't work with wildcards
(%.dtb.S for example), whereas .PRECIOUS does. Seems like perhaps a bug
in make to me, but who knows. Perhaps that's why?

> Anyhow, for the patch, I can manually add --author="you" but it'll still lack your
> SOB - you OK with that or do you want to send a formal patch.

Sure, you can either just say Suggested-by: for me, or apply my s-o-b below:

Signed-off-by: Stephen Warren <swarren@nvidia.com>
--
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
James Hogan April 16, 2013, 3:53 p.m. UTC | #3
On 12/04/13 22:52, Stephen Warren wrote:
> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S

Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
since it'll have quotes around it, so you may instead need:
.SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S

(at least that's what's required for the metag equivalent)

> +
>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)

You might find the same thing here too.

Cheers
James

--
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
James Hogan April 16, 2013, 4:02 p.m. UTC | #4
On 16/04/13 16:53, James Hogan wrote:
> On 12/04/13 22:52, Stephen Warren wrote:
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> 
> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
> since it'll have quotes around it, so you may instead need:
> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
> 
> (at least that's what's required for the metag equivalent)
> 
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
> 
> You might find the same thing here too.

Also, I think you probably now want *.dtb.S added to clean-files,
otherwise they won't get removed by make clean.

Cheers
James

--
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
Vineet Gupta April 17, 2013, 4:13 a.m. UTC | #5
Hi James,

On 04/16/2013 09:23 PM, James Hogan wrote:
> On 12/04/13 22:52, Stephen Warren wrote:
>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
> since it'll have quotes around it, so you may instead need:
> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
>
> (at least that's what's required for the metag equivalent)
>
>> +
>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
> You might find the same thing here too.

Actually in my Makefile, the quotes are stripped off in the very beginning to
avoid duplicating it in every place.

ifneq ($(CONFIG_ARC_BUILTIN_DTB_NAME),"")
    builtindtb-y    := $(patsubst "%",%,$(CONFIG_ARC_BUILTIN_DTB_NAME))
endif
....

Thus both the above are not required - redundant if at all.

-Vineet
--
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
Vineet Gupta April 17, 2013, 4:15 a.m. UTC | #6
On 04/16/2013 09:32 PM, James Hogan wrote:
>
> Also, I think you probably now want *.dtb.S added to clean-files,
> otherwise they won't get removed by make clean.

Good catch !

Thx,
-Vineet
--
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
James Hogan April 17, 2013, 9:13 a.m. UTC | #7
On 17/04/13 05:13, Vineet Gupta wrote:
> Hi James,
> 
> On 04/16/2013 09:23 PM, James Hogan wrote:
>> On 12/04/13 22:52, Stephen Warren wrote:
>>> +.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
>> Note, this may not work if you're using CONFIG_ARC_BUILTIN_DTB_NAME,
>> since it'll have quotes around it, so you may instead need:
>> .SECONDARY: $(obj)/$(patsubst "%",%,$(builtindtb-y)).dtb.S
>>
>> (at least that's what's required for the metag equivalent)
>>
>>> +
>>>  dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)
>> You might find the same thing here too.
> 
> Actually in my Makefile, the quotes are stripped off in the very beginning to
> avoid duplicating it in every place.
> 
> ifneq ($(CONFIG_ARC_BUILTIN_DTB_NAME),"")
>     builtindtb-y    := $(patsubst "%",%,$(CONFIG_ARC_BUILTIN_DTB_NAME))
> endif

Ah yes, I didn't notice that difference. I'll do the same thing for
metag (it only had 1 reference to it before).

Thanks
James

--
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
diff mbox

Patch

diff --git a/arch/arc/boot/dts/Makefile b/arch/arc/boot/dts/Makefile
index 5776835..2f2cf23 100644
--- a/arch/arc/boot/dts/Makefile
+++ b/arch/arc/boot/dts/Makefile
@@ -8,6 +8,8 @@  endif
 obj-y   += $(builtindtb-y).dtb.o
 targets += $(builtindtb-y).dtb

+.SECONDARY: $(obj)/$(builtindtb-y).dtb.S
+
 dtbs:  $(addprefix  $(obj)/, $(builtindtb-y).dtb)

 clean-files := *.dtb