Re: [PATCH 11/11] Remove including timestamp.h in version.h

From: "Pali Rohár" <pali@kernel.org>
To: Sean Anderson <sean.anderson@seco.com>
Cc: "Simon Glass" <sjg@chromium.org>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"Alexander Graf" <agraf@csgraf.de>,
	"Huan Wang" <alison.wang@nxp.com>,
	"Angelo Dureghello" <angelo@sysam.it>,
	"Wolfgang Denk" <wd@denx.de>,
	"Priyanka Jain" <priyanka.jain@nxp.com>,
	"Christophe Leroy" <christophe.leroy@c-s.fr>,
	"Bin Meng" <bmeng.cn@gmail.com>,
	"Marek Behún" <marek.behun@nic.cz>,
	"Tom Rini" <trini@konsulko.com>,
	"U-Boot Mailing List" <u-boot@lists.denx.de>
Subject: Re: [PATCH 11/11] Remove including timestamp.h in version.h
Date: Wed, 4 Aug 2021 23:52:52 +0200	[thread overview]
Message-ID: <20210804215252.xycai6sna45opnzb@pali> (raw)
In-Reply-To: <815b40e2-aa57-abfb-901f-979507a9e3b7@seco.com>

On Wednesday 04 August 2021 17:43:41 Sean Anderson wrote:
> On 8/2/21 3:21 PM, Simon Glass wrote:
> > Hi Pali,
> > 
> > On Mon, 2 Aug 2021 at 07:20, Pali Rohár <pali@kernel.org> wrote:
> > > 
> > > Header file version.h does not use anything from timestamp.h. Including of
> > > timestamp.h has side effect which cause recompiling object file at every
> > > make run because timestamp.h changes at every run.
> > > 
> > > So remove timestamp.h from version.h and include timestamp.h in files
> > > which needs it.
> > > 
> > > This change reduce recompilation time of final U-Boot binary when U-Boot
> > > source files were not changed as less source files needs to be recompiled.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  arch/arm/mach-rockchip/tpl.c                         | 4 ++++
> > >  board/work-microwave/work_92105/work_92105_display.c | 1 +
> > >  cmd/version.c                                        | 1 +
> > >  common/spl/spl.c                                     | 4 ++++
> > >  drivers/rtc/emul_rtc.c                               | 2 +-
> > >  include/version.h                                    | 2 --
> > >  6 files changed, 11 insertions(+), 3 deletions(-)
> > 
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > I assume we do actually want to regenerate the timestamp when U-Boot
> > builds, even if nothing has changed. Is that right?
> 
> I know this is the current behavior, but it would be nice if this was
> not the case. If one is building U-Boot as part of a larger project, one
> might want to have a makefile rule like
> 
> 	u-boot/u-boot.bin:
> 		$(MAKE) -C u-boot $(@F)

You are missing 'FORCE' prerequisite for u-boot/u-boot.bin target (but I
understand what you mean; I'm using this stuff too).

> but u-boot/u-boot.bin will always be remade even if no changes have been
> done. This will cause make to remake all dependents of U-Boot as well
> (which can be rather time-consuming).
> 
> At the moment, I just use U-Boot as an order-only dependency and remake
> it manually. But I would love it if U-Boot was only remade if
> dependencies had actually changed, since this would make it easier to
> integrate it with the rest of my build system.

This behavior which you described is e.g. used by Linux kernel build
system (also based on Makefile(s) and Kbuild as U-Boot).

Maybe we can discuss if such change can or cannot be useful for U-Boot
too. But to not make another confusion, it is not part and it is not
related to this my patch series. So I would suggest to discuss about it
in separate / new thread (fell free to CC me).