[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o



On Tue, Sep 15, 2020 at 02:26:34PM +0200, Jan Beulich wrote:
> On 15.09.2020 13:56, Roger Pau Monné wrote:
> > On Mon, Sep 14, 2020 at 12:15:39PM +0200, Jan Beulich wrote:
> >> Switch to $(call if_changed,ld) where possible; presumably not doing so
> >> in e321576f4047 ("xen/build: start using if_changed") right away was an
> >> oversight, as it did for Arm in (just) one case. It failed to add
> >> prelink.o to $(targets), though, causing - judging from the observed
> >> behavior on x86 - undue rebuilds of the final binary (because of
> >> prelink.o getting rebuild for $(cmd_prelink.o) being empty, in turn
> >> because of .prelink.o.cmd not getting read) during "make install-xen".
> > 
> > I'm not sure I follow why prelink.o needs to be added to targets, does
> > this offer some kind of protection against rebuilds when doing make
> > install?
> 
> In a way, but (as I view it) not really. It is the use of ...
> 
> > The switch to if_changed LGTM.
> 
> ... if_changed which requires this. .*.cmd files will only be loaded
> for anything explicitly or implicitly listed as a target. While .o
> coming from $(obj-y) get added there automatically, prelink.o is not
> something that could be recognized as needing adding, hence the
> "manual" insertion.

This seems very prone to mistakes, as you have to remember that
whatever object that uses if_changed should also be part of the
targets set, or else it will get rebuild unconditionally.

I think adding some of the above reasoning to the commit message would
be helpful IMO.

> Without .prelink.o.cmd loaded, $(if_changed ) will always arrange
> for it to get re-built, because it then will consider the command
> used to build the file to have changed (as the stored one appears to
> be empty).

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.