[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of inclu
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of include search path |
Date: |
Tue, 24 Jan 2017 14:11:29 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 |
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 <address@hidden>
> ---
> 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) $(address@hidden) -c -o $@
> $<,"CC","$(TARGET_DIR)$@")
> + $(call quiet-command,$(CC) $(QEMU_LOCAL_INCLUDES) $(QEMU_INCLUDES)
> $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $(address@hidden) -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
- [Qemu-devel] [PATCH v3 0/8] Switch all subdirs over to modular trace.h file, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 4/8] trace: move hw/i386/xen events to correct subdir, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 3/8] trace: move hw/xen events to correct subdir, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of include search path, Daniel P. Berrange, 2017/01/24
- Re: [Qemu-devel] [PATCH v3 1/8] make: move top level dir to end of include search path,
Eric Blake <=
- [Qemu-devel] [PATCH v3 2/8] trace: move hw/block/dataplane events to correct subdir, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 8/8] trace: improve error reporting when parsing simpletrace header, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 7/8] trace: update docs to reflect new code generation approach, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 5/8] trace: move setting of group name into Makefiles, Daniel P. Berrange, 2017/01/24
- [Qemu-devel] [PATCH v3 6/8] trace: switch to modular code generation for sub-directories, Daniel P. Berrange, 2017/01/24