[Xen-devel] [PATCH 4/4] DEPS handling: Remove absolute paths from references to cwd

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

In some directories we use gcc on source files elsewhere, to generate
a .o here in the current directory.  Eg in tools/libxl/,
   gcc -I -o build.o /path/to/libacpi/build.c
We pass -MMD and -MF options to generate a .d file right here.

In the general case this .c file might need to include things from the
directory here, eg libacpi/build.c eventually #includes various
*libxl*.h.  We pass gcc -I. for this, which means things from the cwd
where we invoked gcc, not the directory of the #including file.

When we do this, gcc's -MMD output mentions /path/to/libxl/*libxl*.h,
even though it could refer to simply *libxl*.h.  This is presumably
because gcc has noticed that `.' in this context must mean relative to
the invocation cwd, not relative to build.c, and gcc doesn't realise
that references in the .d file are also wrt the invocation cwd.

make distinguishes targets purely textually.  It will canonicalise a
target name by removing ./ before comparison (so _libxl_types.h and
./_libxl_types.h are considered the same target) but it won't examine
the filesystem.  So _libxl_types.h and
/path/to/tools/libxl/_libxl_types.h are different targets.

And, _libxl_types.h is generated from a pattern rule.  This pattern
rule is therefore instatiated twice, and the two instances may be run
concurrently - but use the same tempfiles and can therefore fail.

The thing that is wrong here is gcc's choice to output an absolute
path.

We could work around it by adding a rule to teach make about a
relationship between these `two different files'.  But this has to be
done for every autogenerated file and is therefore fragile (leaving a
race bug when we get it wrong).

Ideally we would fix the problem by fixing the .d file as it is
generated.  But the .d files are generated by many many rules
mentioning $(CC) and $(CFLAGS).  (We might in theory pass a bash
process substitution to -MF, but 1. that's not portable to people who
don't have bash and 2. it hangs, anyway.)

So instead we do this conversion at include time.  That is, we tell
make to include not the raw .d files, but the sedded ones.

The sedding removes occurrences of ` $PWD/'.  We use the shell
variable PWD because the make variable sometimes refers to the xen
toplevel.  If gcc's output format should change, then this sed rune
may not work any more, but that doesn't seem very likely.

The rune is only effective for dependencies on files which are exactly
in the current directory, or a subdirectory of it named simply by its
subdirectory name.  If there are autogenerated include files which
exist in a sibling (or worse, somewhere completely else), this
approach will not work, because we'd have to figure out what name this
Makefile usually uses to refer to them.  Hopefully such things don't
exist.

The indirect variables DEPS_RM and DEPS_INCLUDE are necessary to
preserve the assumptions made in the various Makefiles.  Specifically,
xen/ Makefiles assume that it is ok to say DEPS+=something (where
something is in a subdirectory); tools/ Makefiles all used to include
DEPS themselves (but now they include DEPS_INCLUDE); and many
Makefiles tended to explictly rm DEPS (but now rm DEPS_RM).

In the new scheme of things: DEPS is the files that come out of gcc
(or perhaps an assembler or something) and may be assigned to by
Makefiles.  DEPS_INCLUDE is the processed form.  And DEPS_RM is both
combined, so that they both get cleaned.

We need to explicitly use $(wildcard ) to do the wildcard expansion on
DEPS a bit earlier.  If we didn't, then DEPS_INCLUDE would contain
`.*.d2' which would not exist.

Evaluation order: DEPS_RM and DEPS_INCLUDE are recursively expanded
variables, so that although they are defined early (in Config.mk),
their actual values are computed at the time of use, using the value
of DEPS that is prevailing at that time.

Reported-by: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
 .gitignore | 2 ++
 Config.mk  | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 594ffd9..ecb198f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,9 +1,11 @@
 .hg
+.*.tmp
 *.orig
 *~
 *.swp
 *.o
 *.d
+*.d2
 *.opic
 *.a
 *.so
diff --git a/Config.mk b/Config.mk
index 5aca4bf..bba81be 100644
--- a/Config.mk
+++ b/Config.mk
@@ -56,8 +56,11 @@ HOSTCC ?= clang
 HOSTCXX ?= clang++
 endif
 
-DEPS_RM = $(DEPS)
-DEPS_INCLUDE = $(DEPS)
+DEPS_INCLUDE = $(addsuffix .d2, $(basename $(wildcard $(DEPS))))
+DEPS_RM = $(DEPS) $(DEPS_INCLUDE)
+
+%.d2: %.d
+       sed "s! $$PWD/! !" $^ >$@.tmp && mv -f $@.tmp $@
 
 include $(XEN_ROOT)/config/$(XEN_OS).mk
 include $(XEN_ROOT)/config/$(XEN_TARGET_ARCH).mk
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel