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
