[Xen-changelog] [xen master] DEPS handling: Remove absolute paths from references to cwd

[patchbot@xxxxxxx]

commit d6b12add90da95e657ba398475285ae8de3ad0ad
Author:     Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
AuthorDate: Mon Sep 4 17:46:16 2017 +0100
Commit:     Wei Liu <wei.liu2@xxxxxxxxxx>
CommitDate: Thu Sep 7 16:22:05 2017 +0100

    DEPS handling: Remove absolute paths from references to cwd
    
    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>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 .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
--
generated by git-patchbot for /home/xen/git/xen.git#master

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