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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

On Thu, Aug 05, 2021 at 12:14:29AM +0200, Pali Rohár wrote:
> On Wednesday 04 August 2021 18:09:14 Tom Rini wrote:
> > On Wed, Aug 04, 2021 at 05:43:41PM -0400, 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)
> > > 
> > > 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.
> > 
> > Note that with this series applied, if you made use of
> > SOURCE_DATE_EPOCH, nothing would be rebuilt.
> 
> IIRC with SOURCE_DATE_EPOCH nothing would be rebuilt also without
> applying this my patch series.

Correct, it does not.

> > That may or may not make
> > sense however, in your case.  This series does get us closer to being
> > able to do what the linux kernel does as there's now just one place
> > rather than a bunch of places that are rebuilt.
> 
> Just small correction. Even with this patch series there would not be
> just one place. There are still more places but this patch series reduce
> number of these places.

Ah, I only saw one place in my limited testing this morning.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]