[U-Boot] [PATCH] tools: Makefile: improve cross_tools target usability

[Stefan Müller-Klieser]

On 05.11.2016 18:41, Masahiro Yamada wrote:
> Hi.
Hi!

> 
> 2016-10-31 22:15 GMT+09:00 Stefan Müller-Klieser <s.mueller-klieser at phytec.de>:
>> When building the cross_tools target, HOSTCFLAGS and HOSTLDFLAGS will
>> propagate to the target build. This should not happen and is easy to
>> prevent.
>>
>> Signed-off-by: Stefan Müller-Klieser <s.mueller-klieser at phytec.de>
>> ---
>>  tools/Makefile | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 400588c..305336c 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -263,6 +263,8 @@ subdir- += env
>>
>>  ifneq ($(CROSS_BUILD_TOOLS),)
>>  HOSTCC = $(CC)
>> +HOSTCFLAGS = $(CFLAGS)
>> +HOSTLDFLAGS = $(LDFLAGS)
> 
> 
> In the current U-Boot build system (= Kbuild),
> CFLAGS is never set, never referenced.
> 
> For the target build, KBUILD_CFLAGS is used instead.

Well, having the cross_tools target, Kbuild actually creates binaries
for two different runtime environments. First we have the u-boot, which
is a bare metal app controlled by e.g. the KBUILD_CFLAGS, and then
we have the cross tools which are linux userspace binaries. The compilation
of the userspace binaries can be very different and kbuild cannot know about
it, e.g. it does not make much sense to set compiler defaults, in my opinion,

> 
> 
> So, $(CFLAGS) is always empty unless your environment
> explicitly sets it.
> Likewise for LDFLAGS.

As intended.

> 
> 
> Did you mean
> 
> HOSTCFLAGS =
> HOSTLDFLAGS =

That would certainly fix the current bug. Reading your comments make me
think that I should have created two different patches, and I even have
one more queued.

> 
> or
> 
> HOSTCFLAGS = $(KBUILD_CFLAGS)
> HOSTLDFLAGS =

That being said, no I think that would create even more confusion.


> 
> ?

Thanks for the review. Looking at the kbuild documentation, the case for CFLAGS
and LDFLAGS is actually different, as LDFLAGS are well defined and CFLAGS are not.
The case for cross tools is not specified, to my understanding, please correct me.
So another proposal would be:

HOSTCFLAGS = $(TARGETCFLAGS)
HOSTLDFLAGS = $(TARGETLDFLAGS)

?

Regards, Stefan

> 
>