On 01/24/2017 05:01 AM, Daniel P. Berrange wrote: > Currently the search path is > > 1. source dir corresponding to input file (implicit by compiler) > 2. top level build dir > 3. top level source dir > 4. top level source include/ dir > 5. source dir corresponding to input file > 6. build dir corresponding to output file > > This causes a semantic difference in behaviour for builds > where srcdir == builddir vs srcdir != builddir, because > item 5 moves from end to start, when srcdir == builddir.
Rather, item 5 is a no-op (because it duplicated 1), and item 6 moves from the end to the beginning when srcdir == builddir > > As a general rule we also want to move to have all shared > headers in the include/ dir, so move that ahead of the > top level dirs in the search order. Wait - are you proposing that you swap 4 to occur earlier than 2/3?... > > Thus we now have: > > 1. source dir corresponding to input file > 2. build dir corresponding to output file > 3. top level build dir > 4. top level source dir > 5. top level source include/ dir ...because this doesn't match that swap, and I don't see it in the patch body (but I may have missed it; I'm not as strong at reviewing make as I am at C) > > and items 1+2 and 4+5 collapse into a single dir when srcdir==builddir Isn't that items 3+4 (not 4+5) that collapse? > so overall order remains the same. > > Signed-off-by: Daniel P. Berrange <[email protected]> > --- > rules.mak | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/rules.mak b/rules.mak > index d5c516c..e09aabe 100644 > --- a/rules.mak > +++ b/rules.mak > @@ -26,8 +26,10 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out > -Wstrict-prototypes -Wmissing > # Flags for dependency generation > QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d > > -# Same as -I$(SRC_PATH) -I., but for the nested source/object directories > -QEMU_INCLUDES += -I$(<D) -I$(@D) In particular, this is the old code for 5 and 6, > +# Compiler searches the source file dir first, but in vpath builds > +# we need to make it search the build dir too, before any other > +# explicit search paths. > +QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) while this is the new code for 2, plus documentation that 1 is implicit. > > WL_U := -Wl,-u, > find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2))) > @@ -61,7 +63,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \ > $(filter-out %.o %.mo,$1)) > > %.o: %.c > - $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) > $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"CC","$(TARGET_DIR)$@") > + $(call quiet-command,$(CC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES) > $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ > $<,"CC","$(TARGET_DIR)$@") And the pre-pending of QEMU_LOCAL_INCLUDES is what changes the position of the local directory from last to first, thus delaying the top level dir to the end, but I don't see top/include/ moving. These are now some long lines; is it worth taking the time to add \ line splitting for legibility, either in this patch or as an add-on? > @@ -359,6 +361,7 @@ define unnest-vars > $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)), > $(error $o added in $v but $o-objs is not set))) > $(shell mkdir -p ./ $(sort $(dir $($v)))) > + $(shell cd $(BUILD_DIR) && mkdir -p ./ $(sort $(dir $($v)))) > # Include all the .d files Okay, this change makes sense (make sure all the build directories exist in time; no-op for in-tree build, but helpful for VPATH), but seems unrelated to the commit message. Rebase snafu? It looks like you're on the right track, but there's enough discrepancies between the commit message and actual change that I'd prefer a v4 before I grant R-b. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
