Re: [PATCH] build: omit "source" symlink when building hypervisor in-tree

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.03.2023 16:20, Anthony PERARD wrote:
> On Wed, Mar 15, 2023 at 03:56:21PM +0100, Jan Beulich wrote:
>> This symlink is getting in the way of using e.g. "find" on the xen/
>> subtree, and it isn't really needed when not building out-of-tree:
>> the one use that there was can easily be avoided.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/common/efi/efi-common.mk
>> +++ b/xen/common/efi/efi-common.mk
>> @@ -5,11 +5,16 @@ CFLAGS-y += -fshort-wchar
>>  CFLAGS-y += -iquote $(srctree)/common/efi
>>  CFLAGS-y += -iquote $(srcdir)
>>  
>> +source :=
>> +ifneq ($(abs_objtree),$(abs_srctree))
> 
> Could you use "ifdef building_out_of_srctree" instead, or at least use
> the variable $(building_out_of_srctree)? At least that mean there's a
> single way in the tree to differentiate between both kind of build.

I should have added a remark, I realize. I am actually aware of that
variable and also the fact that it is getting exported, but I was
seriously wondering why we do that: It's redundant information, and imo
a variable of this name shouldn't really be exported.

Furthermore I consider the conditional I'm presently using (matching
the one controlling the definition of building_out_of_srctree) more
descriptive: I had to go and convince myself that the variable really
is set based on comparing the paths; I had suspected it might be some
other conditional, not the least because of me not expecting that we'd
carry (and even export) redundant information.

So yes, if you insist I will switch. My preferred route would be to
ditch building_out_of_srctree, though.

>> +source := source/
>> +endif
>> +
>>  # Part of the command line transforms $(obj)
>>  # e.g.: It transforms "dir/foo/bar" into successively
>>  #       "dir foo bar", ".. .. ..", "../../.."
>>  $(obj)/%.c: $(srctree)/common/efi/%.c FORCE
>> -    $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, 
>> ,$(obj))))/source/common/efi/$(<F) $@
>> +    $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, 
>> ,$(obj))))/$(source)common/efi/$(<F) $@
> 
> Instead of $(source), I did proposed initially
> "$(if $(building_out_of_srctree),source/)" for here, or it that making
> the command line too long?
> https://lore.kernel.org/xen-devel/YebpHJk1JIArcdvW@perard/t/#u

Oh, I'm sorry for driving you into adding that symlink, which is now
getting in the way. But yes, putting it inline would imo make an already
too long line yet worse. Otoh I could of course wrap the line, albeit
some care may then be needed to not introduce whitespace in the wrong
place.

> Having "source := $(if $(building_out_of_srctree),source/)" might be an
> ok alternative in place of the use if "ifneq/endif" which take 4 lines?

As per above, if you're determined that building_out_of_srctree should
stay around, then I could do so. Alternatively how about

source := $(if $(patsubst $(abs_objtree),,$(abs_srctree)),,source/)

or the same with $(subst ...) or yet shorter

source := $(if $(abs_srctree:$(abs_objtree)=),,source/)

if you're after cutting the number of lines?

Jan