diff mbox

[RFC,v3,1/5] make.rule: fix $(obj) to a real relative path

Message ID 1378774978-22602-2-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng Sept. 10, 2013, 1:02 a.m. UTC 
Makefile.target includes rule.mak and unnested common-obj-y, then prefix
them with '../', this will ignore object specific QEMU_CFLAGS in subdir
Makefile.objs:

    $(obj)/curl.o: QEMU_CFLAGS += $(CURL_CFLAGS)

Because $(obj) here is './block', instead of '../block'. This doesn't
hurt compiling because we basically build all .o from top Makefile,
before entering Makefile.target, but it will affact arriving per-object
libs support.

The starting point of $(obj) is fixed in $(obj-base) before including
./Makefile.objs, to get consistency with nested Makefile rules in target
rule and variable definition.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 Makefile        | 18 +++++++++++++++++-
 Makefile.objs   | 14 --------------
 Makefile.target |  6 ++++--
 configure       |  1 +
 rules.mak       |  8 +++++---
 tests/Makefile  |  4 ++++
 6 files changed, 31 insertions(+), 20 deletions(-)

Comments

Paolo Bonzini Sept. 10, 2013, 6:34 a.m. UTC | #1 
Il 10/09/2013 03:02, Fam Zheng ha scritto:
> Makefile.target includes rule.mak and unnested common-obj-y, then prefix
> them with '../', this will ignore object specific QEMU_CFLAGS in subdir
> Makefile.objs:
> 
>     $(obj)/curl.o: QEMU_CFLAGS += $(CURL_CFLAGS)
> 
> Because $(obj) here is './block', instead of '../block'. This doesn't
> hurt compiling because we basically build all .o from top Makefile,
> before entering Makefile.target, but it will affact arriving per-object
> libs support.
> 
> The starting point of $(obj) is fixed in $(obj-base) before including
> ./Makefile.objs, to get consistency with nested Makefile rules in target
> rule and variable definition.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Glad this worked. )

> -dummy := $(call unnest-vars)
> diff --git a/Makefile.target b/Makefile.target
> index 9a49852..381022d 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -143,13 +143,15 @@ endif # CONFIG_SOFTMMU
>  # Workaround for http://gcc.gnu.org/PR55489, see configure.
>  %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
>  
> -nested-vars += obj-y
> +nested-vars += obj-y block-obj-y common-obj-y
>  
>  # This resolves all nested paths, so it must come last

This comment is now obsolete.

>  include $(SRC_PATH)/Makefile.objs
> +obj-base := ..

Strictly speaking obj-base=.. is incorrect for obj-y, isn't it?  Does it
work if you do

block-obj-y = ../
common-obj-y = ../

instead of including $(SRC_PATH)/Makefile.objs? Then obj-base can be
empty and is not needed.

But the patch looks good already, I'm asking mostly to get a better
understanding of the system.

Paolo

> +dummy := $(call unnest-vars)
>  
>  all-obj-y = $(obj-y)
> -all-obj-y += $(addprefix ../, $(common-obj-y))
> +all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))
>  
>  ifndef CONFIG_HAIKU
>  LIBS+=-lm
> diff --git a/configure b/configure
> index e989609..cc3cd4d 100755
> --- a/configure
> +++ b/configure
> @@ -2251,6 +2251,7 @@ fi
>  if $pkg_config --atleast-version=$glib_req_ver gthread-2.0; then
>      glib_cflags=`$pkg_config --cflags gthread-2.0`
>      glib_libs=`$pkg_config --libs gthread-2.0`
> +    CFLAGS="$glib_cflags $CFLAGS"
>      LIBS="$glib_libs $LIBS"
>      libs_qga="$glib_libs $libs_qga"
>  else
> diff --git a/rules.mak b/rules.mak
> index 4499745..3ff7d7a 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -103,7 +103,7 @@ clean: clean-timestamp
>  
>  # magic to descend into other directories
>  
> -obj := .
> +obj = $(obj-base)
>  old-nested-dirs :=
>  
>  define push-var
> @@ -119,9 +119,11 @@ endef
>  
>  define unnest-dir
>  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
> -$(eval obj := $(obj)/$1)
> +$(eval obj-parent-$1 := $(obj))
> +$(eval obj := $(if $(obj),$(obj)/$1,$1))
>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
> -$(eval obj := $(patsubst %/$1,%,$(obj)))
> +$(eval obj := $(obj-parent-$1))
> +$(eval obj-parent-$1 := )
>  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
>  endef
>  
> diff --git a/tests/Makefile b/tests/Makefile
> index baba9e9..15ef039 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -110,6 +110,10 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>  $(test-obj-y): QEMU_INCLUDES += -Itests
>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>  
> +nested-vars := block-obj-y
> +obj-base := ..
> +dummy := $(call unnest-vars)
> +
>  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>  
>  tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
>
Fam Zheng Sept. 10, 2013, 7:16 a.m. UTC | #2 
On Tue, 09/10 08:34, Paolo Bonzini wrote:
> Il 10/09/2013 03:02, Fam Zheng ha scritto:
> > Makefile.target includes rule.mak and unnested common-obj-y, then prefix
> > them with '../', this will ignore object specific QEMU_CFLAGS in subdir
> > Makefile.objs:
> > 
> >     $(obj)/curl.o: QEMU_CFLAGS += $(CURL_CFLAGS)
> > 
> > Because $(obj) here is './block', instead of '../block'. This doesn't
> > hurt compiling because we basically build all .o from top Makefile,
> > before entering Makefile.target, but it will affact arriving per-object
> > libs support.
> > 
> > The starting point of $(obj) is fixed in $(obj-base) before including
> > ./Makefile.objs, to get consistency with nested Makefile rules in target
> > rule and variable definition.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> Glad this worked. )
> 
> > -dummy := $(call unnest-vars)
> > diff --git a/Makefile.target b/Makefile.target
> > index 9a49852..381022d 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -143,13 +143,15 @@ endif # CONFIG_SOFTMMU
> >  # Workaround for http://gcc.gnu.org/PR55489, see configure.
> >  %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
> >  
> > -nested-vars += obj-y
> > +nested-vars += obj-y block-obj-y common-obj-y
> >  
> >  # This resolves all nested paths, so it must come last
> 
> This comment is now obsolete.
> 
> >  include $(SRC_PATH)/Makefile.objs
> > +obj-base := ..
> 
> Strictly speaking obj-base=.. is incorrect for obj-y, isn't it?  Does it

You are right, I'd do it respectively, since...

> work if you do
> 
> block-obj-y = ../
> common-obj-y = ../
> 
> instead of including $(SRC_PATH)/Makefile.objs? Then obj-base can be
> empty and is not needed.
> 
> But the patch looks good already, I'm asking mostly to get a better
> understanding of the system.
> 
it's not working.

The base point of unnesting is fixed to SRC_PATH:

    define unnest-dir
    ...
    $(eval include $(SRC_PATH)/$1/Makefile.objs)
    ...
    endef

So it can't unnest  "../". block-obj-y = ./ makes a little more sense, but
$(obj) can be messed, again.

Do you think two calls of unnest-vars OK?

    nested-vars = obj-y
    dummy := $(call unnest-vars)
    include $(SRC_PATH)/Makefile.objs
    obj-base := ..
    nested-vars = block-obj-y common-obj-y
    dummy := $(call unnest-vars)

> > +dummy := $(call unnest-vars)
> >  
> >  all-obj-y = $(obj-y)
> > -all-obj-y += $(addprefix ../, $(common-obj-y))
> > +all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))
> >  
> >  ifndef CONFIG_HAIKU
> >  LIBS+=-lm
> > diff --git a/configure b/configure
> > index e989609..cc3cd4d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2251,6 +2251,7 @@ fi
> >  if $pkg_config --atleast-version=$glib_req_ver gthread-2.0; then
> >      glib_cflags=`$pkg_config --cflags gthread-2.0`
> >      glib_libs=`$pkg_config --libs gthread-2.0`
> > +    CFLAGS="$glib_cflags $CFLAGS"
> >      LIBS="$glib_libs $LIBS"
> >      libs_qga="$glib_libs $libs_qga"
> >  else
> > diff --git a/rules.mak b/rules.mak
> > index 4499745..3ff7d7a 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -103,7 +103,7 @@ clean: clean-timestamp
> >  
> >  # magic to descend into other directories
> >  
> > -obj := .
> > +obj = $(obj-base)
> >  old-nested-dirs :=
> >  
> >  define push-var
> > @@ -119,9 +119,11 @@ endef
> >  
> >  define unnest-dir
> >  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
> > -$(eval obj := $(obj)/$1)
> > +$(eval obj-parent-$1 := $(obj))
> > +$(eval obj := $(if $(obj),$(obj)/$1,$1))
> >  $(eval include $(SRC_PATH)/$1/Makefile.objs)
> > -$(eval obj := $(patsubst %/$1,%,$(obj)))
> > +$(eval obj := $(obj-parent-$1))
> > +$(eval obj-parent-$1 := )
> >  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
> >  endef
> >  
> > diff --git a/tests/Makefile b/tests/Makefile
> > index baba9e9..15ef039 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -110,6 +110,10 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
> >  $(test-obj-y): QEMU_INCLUDES += -Itests
> >  QEMU_CFLAGS += -I$(SRC_PATH)/tests
> >  
> > +nested-vars := block-obj-y
> > +obj-base := ..
> > +dummy := $(call unnest-vars)
> > +
> >  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
> >  
> >  tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
> > 
>
Paolo Bonzini Sept. 10, 2013, 7:58 a.m. UTC | #3 
Il 10/09/2013 09:16, Fam Zheng ha scritto:
> The base point of unnesting is fixed to SRC_PATH:
> 
>     define unnest-dir
>     ...
>     $(eval include $(SRC_PATH)/$1/Makefile.objs)
>     ...
>     endef
> 
> So it can't unnest  "../". block-obj-y = ./ makes a little more sense, but
> $(obj) can be messed, again.

Right.  I was confusing the object directory with the source directory.

> Do you think two calls of unnest-vars OK?
> 
>     nested-vars = obj-y
>     dummy := $(call unnest-vars)
>     include $(SRC_PATH)/Makefile.objs
>     obj-base := ..
>     nested-vars = block-obj-y common-obj-y
>     dummy := $(call unnest-vars)

Yeah, that's fine.

Perhaps nested-vars and obj-base could become arguments to unnest-vars, too.

Paolo

>>> +dummy := $(call unnest-vars)
>>>  
>>>  all-obj-y = $(obj-y)
>>> -all-obj-y += $(addprefix ../, $(common-obj-y))
>>> +all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))
>>>  
>>>  ifndef CONFIG_HAIKU
>>>  LIBS+=-lm
>>> diff --git a/configure b/configure
>>> index e989609..cc3cd4d 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2251,6 +2251,7 @@ fi
>>>  if $pkg_config --atleast-version=$glib_req_ver gthread-2.0; then
>>>      glib_cflags=`$pkg_config --cflags gthread-2.0`
>>>      glib_libs=`$pkg_config --libs gthread-2.0`
>>> +    CFLAGS="$glib_cflags $CFLAGS"
>>>      LIBS="$glib_libs $LIBS"
>>>      libs_qga="$glib_libs $libs_qga"
>>>  else
>>> diff --git a/rules.mak b/rules.mak
>>> index 4499745..3ff7d7a 100644
>>> --- a/rules.mak
>>> +++ b/rules.mak
>>> @@ -103,7 +103,7 @@ clean: clean-timestamp
>>>  
>>>  # magic to descend into other directories
>>>  
>>> -obj := .
>>> +obj = $(obj-base)
>>>  old-nested-dirs :=
>>>  
>>>  define push-var
>>> @@ -119,9 +119,11 @@ endef
>>>  
>>>  define unnest-dir
>>>  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
>>> -$(eval obj := $(obj)/$1)
>>> +$(eval obj-parent-$1 := $(obj))
>>> +$(eval obj := $(if $(obj),$(obj)/$1,$1))
>>>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
>>> -$(eval obj := $(patsubst %/$1,%,$(obj)))
>>> +$(eval obj := $(obj-parent-$1))
>>> +$(eval obj-parent-$1 := )
>>>  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
>>>  endef
>>>  
>>> diff --git a/tests/Makefile b/tests/Makefile
>>> index baba9e9..15ef039 100644
>>> --- a/tests/Makefile
>>> +++ b/tests/Makefile
>>> @@ -110,6 +110,10 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>>>  $(test-obj-y): QEMU_INCLUDES += -Itests
>>>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>>>  
>>> +nested-vars := block-obj-y
>>> +obj-base := ..
>>> +dummy := $(call unnest-vars)
>>> +
>>>  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>>>  
>>>  tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
>>>
>>
> 
>
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 806946e..ef80f0d 100644
--- a/Makefile
+++ b/Makefile
@@ -115,14 +115,30 @@  defconfig:
 
 ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/Makefile.objs
-include $(SRC_PATH)/tests/Makefile
 endif
 ifeq ($(CONFIG_SMARTCARD_NSS),y)
 include $(SRC_PATH)/libcacard/Makefile
 endif
 
+nested-vars += \
+	stub-obj-y \
+	util-obj-y \
+	qga-obj-y \
+	block-obj-y \
+	common-obj-y
+
+dummy := $(call unnest-vars)
+
+ifneq ($(wildcard config-host.mak),)
+include $(SRC_PATH)/tests/Makefile
+endif
+
 all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
 
+vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
+
+vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
+
 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx
diff --git a/Makefile.objs b/Makefile.objs
index f46a4cd..efd5b0f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -109,17 +109,3 @@  version-lobj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.lo
 # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
 # by libqemuutil.a.  These should be moved to a separate .json schema.
 qga-obj-y = qga/ qapi-types.o qapi-visit.o
-
-vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
-
-vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
-
-QEMU_CFLAGS+=$(GLIB_CFLAGS)
-
-nested-vars += \
-	stub-obj-y \
-	util-obj-y \
-	qga-obj-y \
-	block-obj-y \
-	common-obj-y
-dummy := $(call unnest-vars)
diff --git a/Makefile.target b/Makefile.target
index 9a49852..381022d 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -143,13 +143,15 @@  endif # CONFIG_SOFTMMU
 # Workaround for http://gcc.gnu.org/PR55489, see configure.
 %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
 
-nested-vars += obj-y
+nested-vars += obj-y block-obj-y common-obj-y
 
 # This resolves all nested paths, so it must come last
 include $(SRC_PATH)/Makefile.objs
+obj-base := ..
+dummy := $(call unnest-vars)
 
 all-obj-y = $(obj-y)
-all-obj-y += $(addprefix ../, $(common-obj-y))
+all-obj-y += $(addprefix ../, $(common-obj-y) $(block-obj-y))
 
 ifndef CONFIG_HAIKU
 LIBS+=-lm
diff --git a/configure b/configure
index e989609..cc3cd4d 100755
--- a/configure
+++ b/configure
@@ -2251,6 +2251,7 @@  fi
 if $pkg_config --atleast-version=$glib_req_ver gthread-2.0; then
     glib_cflags=`$pkg_config --cflags gthread-2.0`
     glib_libs=`$pkg_config --libs gthread-2.0`
+    CFLAGS="$glib_cflags $CFLAGS"
     LIBS="$glib_libs $LIBS"
     libs_qga="$glib_libs $libs_qga"
 else
diff --git a/rules.mak b/rules.mak
index 4499745..3ff7d7a 100644
--- a/rules.mak
+++ b/rules.mak
@@ -103,7 +103,7 @@  clean: clean-timestamp
 
 # magic to descend into other directories
 
-obj := .
+obj = $(obj-base)
 old-nested-dirs :=
 
 define push-var
@@ -119,9 +119,11 @@  endef
 
 define unnest-dir
 $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
-$(eval obj := $(obj)/$1)
+$(eval obj-parent-$1 := $(obj))
+$(eval obj := $(if $(obj),$(obj)/$1,$1))
 $(eval include $(SRC_PATH)/$1/Makefile.objs)
-$(eval obj := $(patsubst %/$1,%,$(obj)))
+$(eval obj := $(obj-parent-$1))
+$(eval obj-parent-$1 := )
 $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
 endef
 
diff --git a/tests/Makefile b/tests/Makefile
index baba9e9..15ef039 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -110,6 +110,10 @@  test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
+nested-vars := block-obj-y
+obj-base := ..
+dummy := $(call unnest-vars)
+
 tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
 
 tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a