2017-10-10 7:03 GMT+09:00 Doug Anderson <dianders@xxxxxxxxxxxx>:
> Hi,
>
> On Tue, Oct 3, 2017 at 8:56 PM, Masahiro Yamada
> <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>> The top Makefile is divided into some sections such as mixed targets,
>> config targets, build targets, etc.
>>
>> When we build mixed targets, Kbuild just invokes submake to process
>> them one by one. In this case, compiler-related variables like CC,
>> KBUILD_CFLAGS, etc. are unneeded.
>>
>> Check what kind of targets we are building first, then parse necessary
>> variables for building them.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
>> ---
>>
>> Makefile | 233 ++++++++++++++++++++++++++++++++-------------------------------
>> 1 file changed, 118 insertions(+), 115 deletions(-)
>
> I'm even further outside my comfort zone with this big of a change,
> but I will say that as far as I can tell this seems like a good
> change. If it were me I'd have probably broken it up into several
> tinier changes that were each massively easy to check/verify, but
> presumably for those who know the Makefile better then rolling them
> together is fine. ;)
>
> I're spent some time reviewing this (not tons--maybe an hour or so),
> but IMHO I don't know this well enough to add a Reviewed-by tag.
>
>
>> diff --git a/Makefile b/Makefile
>> index 39a7c03..a4fd682 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -186,15 +186,6 @@ ifeq ("$(origin M)", "command line")
>> KBUILD_EXTMOD := $(M)
>> endif
>>
>> -# If building an external module we do not care about the all: rule
>> -# but instead _all depend on modules
>> -PHONY += all
>> -ifeq ($(KBUILD_EXTMOD),)
>> -_all: all
>> -else
>> -_all: modules
>> -endif
>> -
>> ifeq ($(KBUILD_SRC),)
>> # building in the source tree
>> srctree := .
>> @@ -206,6 +197,9 @@ else
>> srctree := $(KBUILD_SRC)
>> endif
>> endif
>> +
>> +export KBUILD_CHECKSRC KBUILD_EXTMOD KBUILD_SRC
>
> The old Makefile also used to export KBUILD_SRC, but I'm not sure why.
> Shouldn't this be implicit because KBUILD_SRC always comes from a
> command line parameter or environment variable?
As the comment line around line 109 says,
"KBUILD_SRC is not intended to be used by the regular user (for now)"
It is set by "sub-make" around line 143:
sub-make:
$(Q)$(MAKE) -C $(KBUILD_OUTPUT) KBUILD_SRC=$(CURDIR) \
-f $(CURDIR)/Makefile $(filter-out _all sub-make,$(MAKECMDGOALS))
I'd like to improve it in the future,
but currently KBUILD_SRC works like that.
But, you are right.
I also thought "export KBUILD_SRC" is redundant.
KBUILD_SRC=$(CURDIR) implies exporting it.
I see some more redundant exporting
for variables we know they only come from command line or environment.
I think cleaning-up is OK, but should be
separated to another patch just in case.
(my commit claims I am just changing the code order...)
--
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
