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

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



On 30.01.2020 19:10, Anthony PERARD wrote:
> 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@xxxxxxxxx>
>>>
>>> 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.

But with Config.mk still getting included from elsewhere underneath
xen/, this is going to be confusing. Changing the Kbuild.include
instances should really affect the entire xen/ tree then, at which
point the Config.mk instances could be declared "for everything
else" (and eventually be moved into the subtrees). I agree it's
not very helpful that Config.mk contains not only common
definitions, but also ones actually controlling how certain parts
of the build process work (like the two DEPS_*; going through the
file I wonder whether these two are actually the only outliers).

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

Right, an incremental switch of the build machinery is going to
run into oddities. Calling them out explicitly (including what
the plan is to resolve them by the end of the transformation
process) would be helpful, though.

Jan

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

 


Rackspace

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