Re: [XEN PATCH v2] build: Fix x86 out-of-tree build without EFI

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Aug 17, 2022 at 12:38:36PM +0200, Jan Beulich wrote:
> On 17.08.2022 11:15, Anthony PERARD wrote:
> > --- a/xen/common/efi/efi-common.mk
> > +++ b/xen/common/efi/efi-common.mk
> > @@ -9,9 +9,9 @@ CFLAGS-y += -iquote $(srcdir)
> >  # e.g.: It transforms "dir/foo/bar" into successively
> >  #       "dir foo bar", ".. .. ..", "../../.."
> >  $(obj)/%.c: $(srctree)/common/efi/%.c FORCE
> > -   $(Q)test -f $@ || \
> > -       ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, 
> > ,$(obj))))/source/common/efi/$(<F) $@
> > +   $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, 
> > ,$(obj))))/source/common/efi/$(<F) $@
> 
> I'm afraid the commit message hasn't made clear to me why this part of
> the change is (still) needed (or perhaps just wanted). The rest of this
> lgtm now, thanks.

There's an explanation in the commit message, quoted here:
> >  The issue is that in out-of-tree, make might find x86/efi/stub.c via
> >  VPATH, but as the target needs to be rebuilt due to FORCE, make
> >  actually avoid changing the source tree and rebuilt the target with
> >  VPATH ignored, so $@ lead to the build tree where "stub.c" dosen't
> >  exist yet so a link is made to "common/stub.c".

The problem with `test -f $@` it doesn't test what we think it does. It
doesn't test for the presence of a regular file in the source tree as
stated in the original tree. First, `test -f` happily follow symlinks.
Second, $@ is always going to point to the build dir, as GNU Make will
try to not make changes to the source tree, if I understand the logic
correctly.

Instead of `test -f`, we could probably remove the "FORCE" from the
prerequisite, but there's still going to be an issue if there's a file
with the same name in both common and per-arch directory, when the common
file is newer.

So `test -f` needs to go.

Cheers,

-- 
Anthony PERARD