All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	George Dunlap <George.Dunlap@eu.citrix.com>
Subject: Re: [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o
Date: Tue, 15 Sep 2020 15:46:30 +0200	[thread overview]
Message-ID: <20200915134630.GM753@Air-de-Roger> (raw)
In-Reply-To: <ae76b7d7-ea16-f668-3408-13c01a660e06@suse.com>

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@citrix.com>

Thanks, Roger.


  reply	other threads:[~2020-09-15 13:47 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 10:12 [PATCH 0/9] xen: beginnings of moving library-like code into an archive Jan Beulich
2020-09-14 10:15 ` [PATCH 1/9] build: use if_changed more consistently (and correctly) for prelink*.o Jan Beulich
2020-09-15 11:56   ` Roger Pau Monné
2020-09-15 12:26     ` Jan Beulich
2020-09-15 13:46       ` Roger Pau Monné [this message]
2020-09-15 15:14         ` Jan Beulich
2020-09-21 10:17   ` Ping: " Jan Beulich
2020-09-21 11:39     ` Julien Grall
2020-09-22  8:28       ` Jan Beulich
2020-09-22  9:24         ` Julien Grall
2020-09-22 10:55           ` Jan Beulich
2020-09-22 11:47             ` Julien Grall
2020-09-22 17:40               ` Stefano Stabellini
2020-09-14 10:16 ` [PATCH 2/9] lib: split _ctype[] into its own object, under lib/ Jan Beulich
2020-09-14 10:16 ` [PATCH 3/9] lib: collect library files in an archive Jan Beulich
2020-09-14 10:16 ` [PATCH 4/9] lib: move list sorting code Jan Beulich
2020-09-14 10:17 ` [PATCH 5/9] lib: move parse_size_and_unit() Jan Beulich
2020-09-22 19:41   ` Andrew Cooper
2020-09-24  7:04     ` Jan Beulich
2020-09-14 10:17 ` [PATCH 6/9] lib: move init_constructors() Jan Beulich
2020-09-14 10:18 ` [PATCH 7/9] lib: move rbtree code Jan Beulich
2020-09-14 10:18 ` [PATCH 8/9] lib: move bsearch code Jan Beulich
2020-09-22 19:34   ` Andrew Cooper
2020-09-24  7:09     ` Jan Beulich
2020-09-14 10:18 ` [PATCH 9/9] lib: move sort code Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200915134630.GM753@Air-de-Roger \
    --to=roger.pau@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.