Re: [PATCH v2 1/2] build: don't export building_out_of_srctree

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Mar 29, 2023 at 12:22:16PM +0200, Jan Beulich wrote:
> I don't view a variable of this name as suitable for exporting, the more

We could rename it.

> that it carries entirely redundant information. The reasons for its

The patch replace building_out_of_srctree with abs_objtree and
abs_srctree which also carries redundant informations. abs_objtree can
probably be replaced by $(CURDIR), abs_srctree can be
recalculated from $(srctree).

> introduction in Linux commit 051f278e9d81 ("kbuild: replace
> KBUILD_SRCTREE with boolean building_out_of_srctree") also don't apply
> to us. Ditch exporting of the variable, replacing uses suitably.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

This patch feels like obfuscation of the intended test. Instead of
reading a test for "out_of_tree", we now have to guess why the two paths
are been compared.

Anyway, there isn't that many use outside of the main Makefile, so I
guess the patch is kind of ok:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>


> For further reasons (besides the similar redundancy aspect) exporting
> VPATH looks also suspicious: Its name being all uppercase makes it a
> "non application private" variable, i.e. it or its (pre-existing) value
> may have a purpose/use elsewhere. And exporting it looks to be easily

This sounds like you don't know what VPATH is for, but I'm pretty sure
you do. If there's an pre-existing value, we just ignore it. If VPATH is
used by a program that our Makefile used and that program is intended to
be used by build system then that a bug in that program for not knowing
about makes' VPATH. So I don't think we need to worried about it been
exported.

> avoidable: Instead of setting it in xen/Makefile, it looks like it could
> be set in xen/scripts/Kbuild.include. Thoughts?

I'd rather not make that change unless there's a real issue with
exporting VPATH. We are more likely to introduce a bug than to avoid
one.

Thanks,

-- 
Anthony PERARD