[U-Boot] [PATCH v2 01/19] Makefile: prepare for using Kbuild-style Makefile

[Simon Glass]

Hi Masahiro,

On Fri, Oct 4, 2013 at 3:59 AM, Masahiro Yamada
<yamada.m at jp.panasonic.com> wrote:
> Hello Simon
>
>
>
>> > +# Tentative step for Kbuild-style makefiles coexist with conventional
>> > U-Boot style makefiles
>> > +#  U-Boot conventional sub makefiles always include some other makefiles.
>> > +#  So, the build system searches a line beginning with "include" before
>> > entering into the sub makefile
>> > +#  in order to distinguish which style it is.
>> >
>>
>> Would looking for obj- be better or worse?
>
> At first I thought of this but I was kind of worried
> whether all makefiles should forcibly have obj-y or obj-.
>
> arch/arm/cpu/armv7/tegra114/
> arch/arm/cpu/armv7/tegra30/
> directories have no source files but only a Makefile.
>
> After all, I simply added
> obj- :=
> line in them, so I think your suggestion will work.
>
> If you prefer to search obj-, I will consider to change at version 3.

No, it doesn't matter, it sounds like what you are doing is just as
good, and it is temporary as you say. I wonder how temporary?

>
> In any case, this ugly grep switch is tentative.
> I want to refactor all makefiles and delete this switch
> as soon as possible.
>
>
>
>> an indent might help here, and below.
>
> I will fix at v3.
>
>
>
>> > +               fi
>> >
>> >  $(LIBS):       depend $(SUBDIR_TOOLS)
>> > -               $(MAKE) -C $(dir $(subst $(obj),,$@))
>> > +               if grep -q "^include" $(dir $(subst $(obj),,$@))Makefile;
>> > then \
>> >
>>
>> This check seems to appear a lot - could it become a $(call ...) perhaps?
>
>
> I did not care so much about this part becuase it is temporary.
> But re-writing it shortly with $(call ...) is not difficult.
> I will try at v3.
> Thanks for your advice.
>
>
>
>
>> Re testing, I used:
>>
>> $ ./tools/buildman/buildman -b try-kbuild -k
>>
>> and confirmed that the binaries do change, for example, with cm41xx:
>>
>> $ wc ...
>>    1348    5418 123176
>> try-kbuild/01_of_20_g0c5274e6_Prepare-v2013.04-rc4/cm41xx/u-boot.bin
>>    1348    5417  123188
>> try-kbuild/02_of_20_gd3068182_Makefile--prepare-fo/cm41xx/u-boot.bin
>>
>> The change may well be harmless though.
>
> It is probably because of include/generated/version_autogenerated.h
>
> The length of PLAIN_VERSION and U_BOOT_VERSION changes
> because the output `git describe` command is used for here.
>
> Actually I gave it a try.
>
>
> For master branch (Prepare v2013.04-rc4), I got:
>
> #define PLAIN_VERSION "2013.10-rc4"
> #define U_BOOT_VERSION "U-Boot 2013.10-rc4"
> #define CC_VERSION_STRING "arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3"
> #define LD_VERSION_STRING "GNU ld (GNU Binutils for Ubuntu) 2.23.2"
>
>
> For try-kbuild branch (Makefile: prepare for using Kbuild-style Makefile)
> I got:
>
> #define PLAIN_VERSION "2013.10-rc4-00001-gc31a399"
> #define U_BOOT_VERSION "U-Boot 2013.10-rc4-00001-gc31a399"
> #define CC_VERSION_STRING "arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3"
> #define LD_VERSION_STRING "GNU ld (GNU Binutils for Ubuntu) 2.23.2"
>
>
> For the list of things you should take into account to get identical
> output files, please refer the thread of version1 of this series:
> [U-Boot] [PATCH 00/19] First step towards Kbuild: Use Kbuild style makefiles
> Message-Id: <20130917093533.738A.AA925319 at jp.panasonic.com>
>
>
>> (2) Git commit hash
>>
>> Git commit hash is contained in include/generated/version_autogenerated.h.
>> So, I also modified include/version.h not to include it as follows:
>>
>>      #ifndef DO_DEPS_ONLY
>>     -#include "generated/version_autogenerated.h"
>>     +/* #include "generated/version_autogenerated.h" */
>>     +#define PLAIN_VERSION "__DUMMY__"
>>     +#define U_BOOT_VERSION "__DUMMY__"
>>     +#define CC_VERSION_STRING "__DUMMY__"
>>     +#define LD_VERSION_STRING "__DUMMY__"
>>      #endif
>
> This is what I did when I compared md5sum.
> After applying above as a prerequisite commit,
> I tried 'tools/buildman/buildman -b try-kbuild -k' again
> and I got the same size for cm41xx/u-boot.bin
>
>
> If you compare the size of u-boot.bin,
> please take care of the order of object files too.
>
> I also mentioned this in the thread of v1.
>
>
>
>
>
>> It would be nice to add a feature to buildman to compare binaries. Of
>> course we would need to add a Makefile option to disable the timestamp
>> embedding first, since all binaries are different because of that.
>
> Sounds a good idea.
> I will take a look when I have time.
>
>

Regards,
Simon