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

["Pali Rohár" <pali@kernel.org>]

On Wednesday 04 August 2021 18:15:55 Tom Rini wrote:
> 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.

For my tested boards it is also just one place. But for some other
boards with extended config options it may be more places (like emul_rtc
or work display, ...).

But if your testing discovered also only one place it could mean that it
can be common case for more people...