Re: [Xen-devel] [XEN PATCH v2 04/12] xen/build: extract clean target from Rules.mk

From: Anthony PERARD <anthony.perard@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Anthony PERARD <anthony.perard@gmail.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [XEN PATCH v2 04/12] xen/build: extract clean target from Rules.mk
Date: Thu, 30 Jan 2020 18:10:38 +0000	[thread overview]
Message-ID: <20200130181038.GQ1288@perard.uk.xensource.com> (raw)
In-Reply-To: <db579f9f-d816-47f9-e75c-4b20d60bda3b@suse.com>

On Wed, Jan 29, 2020 at 03:30:19PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > From: Anthony PERARD <anthony.perard@gmail.com>
> > 
> > Most of the code executed by Rules.mk isn't necessary for the clean
> > target, especially not the CFLAGS. This make running make clean much
> > faster.
> > 
> > This extract the code into a different Makefile. It doesn't want to
> > include Config.mk either so variables DEPS_RM and DEPS_INCLUDE are
> > extracted from Config.mk as well. DEPS_INCLUDE is put into
> > Kbuild.include so it could be use by other Makefiles.
> 
> "extracted" makes it sound as if the intention was to move things,
> yet ...
> 
> > ---
> >  xen/Rules.mk               | 13 -------------
> >  xen/scripts/Kbuild.include |  7 ++++++-
> >  xen/scripts/Makefile.clean | 33 +++++++++++++++++++++++++++++++++
> >  3 files changed, 39 insertions(+), 14 deletions(-)
> 
> ... ./Config.mk doesn't get touched at all. I guess there are reasons
> for this, but I consider it dangerous to leave independent definitions
> of the same variables in disconnected places. What if one side gets
> updated without noticing the other?

I guess the word "extracted" is the wrong one. I'll need to rewrite the
patch commentary.

As for why Config.mk isn't change, it's because it is used by both the
hypervisor makefiles and the tools makefiles. I would like for recursive
makefiles to not include Config.mk anymore, so having only xen/Makefile
doing that include. (I would like to go further and not used Config.mk
anymore, but that might not be necessary.)

As for the last point, the variables DEPS_RM and DEPS_INCLUDE are copied
because Makefile.clean doesn't have them and at some point Rules.mk (no
patch yet) isn't going to have them either, so there will be a single
location which is Kbuild.include. Currently with this patch, both
variables from Kbuild.include are the one used by Rules.mk, so it
doesn't matter if Config.mk is modified.

Things doesn't look great yet, but it doesn't feel like there are better
way to refactor the build system.

> > --- /dev/null
> > +++ b/xen/scripts/Makefile.clean
> > +# Figure out what we need to build from the various variables
> 
> s/build/clean/ ?

Yep, I just copy the typo from Linux. But I can fix it in our repo.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel