qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Fam Zheng <famz@redhat.com>
Cc: mjt@tls.msk.ru, qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [Qemu-devel] [RFC PATCH 1/6] make.rule: fix $(obj) to a real relative path
Date: Fri, 06 Sep 2013 11:41:28 +0200	[thread overview]
Message-ID: <5229A348.3080204@redhat.com> (raw)
In-Reply-To: <1378376448-29036-2-git-send-email-famz@redhat.com>

Il 05/09/2013 12:20, 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 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.target | 3 ++-
>  rules.mak       | 6 +++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 9a49852..b35e7c1 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -144,12 +144,13 @@ endif # CONFIG_SOFTMMU
>  %/translate.o: QEMU_CFLAGS += $(TRANSLATE_OPT_CFLAGS)
>  
>  nested-vars += obj-y
> +obj := ..
>  
>  # This resolves all nested paths, so it must come last
>  include $(SRC_PATH)/Makefile.objs
>  
>  all-obj-y = $(obj-y)
> -all-obj-y += $(addprefix ../, $(common-obj-y))
> +all-obj-y += $(addprefix $(obj)/, $(common-obj-y))

The bug is clearly there, but I'm not sure this is correct.

"obj" is the path to the object file.  obj-y files are built in the
target directory, thus the extra ".." should apply only to common-obj-y;
$(obj-y) should not be prefixed.  So perhaps you need to:

1) move the nested-vars and unnest-vars call from Makefile.objs to
Makefile.  also move this line:

   QEMU_CFLAGS+=$(GLIB_CFLAGS)

which has no business in Makefile.objs.  With this change, Makefile.objs
is just a list, like other */Makefile.objs files.

2) remove the additional complication where common-obj-y is including
block-obj-y, just add it to all-obj-y manually.

3) in Makefile.target, do this:

block-obj-y = ../
common-obj-y = ../
nested-vars = obj-y block-obj-y common-obj-y
dummy := $(call unnest-vars)
all-obj-y = $(obj-y) $(common-obj-y) $(block-obj-y)

(Can all be done in a single patch).

?

>  
>  ifndef CONFIG_HAIKU
>  LIBS+=-lm
> diff --git a/rules.mak b/rules.mak
> index 4499745..5758137 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -103,7 +103,6 @@ clean: clean-timestamp
>  
>  # magic to descend into other directories
>  
> -obj := .
>  old-nested-dirs :=
>  
>  define push-var
> @@ -119,9 +118,10 @@ endef
>  
>  define unnest-dir
>  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
> -$(eval obj := $(obj)/$1)
> +$(eval old-obj := $(obj))
> +$(eval obj := $(if $(obj),$(obj)/$1,$1))
>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
> -$(eval obj := $(patsubst %/$1,%,$(obj)))
> +$(eval obj := $(old-obj))
>  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))

old-obj doesn't work if you have multiple levels of nesting.  But you
can call the variable something like obj-parent-$1 instead to solve this
problem.

However, please clean it up afterwards with "$(eval obj-parent-$1 := )".

Paolo

>  endef
>  
> 

  reply	other threads:[~2013-09-06  9:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-05 10:20 [Qemu-devel] [RFC PATCH 0/6] Shared Library Module Support Fam Zheng
2013-09-05 10:20 ` [Qemu-devel] [RFC PATCH 1/6] make.rule: fix $(obj) to a real relative path Fam Zheng
2013-09-06  9:41   ` Paolo Bonzini [this message]
2013-09-05 10:20 ` [Qemu-devel] [RFC PATCH 2/6] rule.mak: allow per object cflags and libs Fam Zheng
2013-09-05 10:20 ` [Qemu-devel] [RFC PATCH 3/6] Makefile: define curl cflags and libs with object Fam Zheng
2013-09-05 10:20 ` [Qemu-devel] [RFC PATCH 4/6] Makefile: introduce common-obj-m and block-obj-m for DSO Fam Zheng
2013-09-05 19:24   ` Richard Henderson
2013-09-05 19:41     ` Paolo Bonzini
2013-09-05 21:45       ` Peter Maydell
2013-09-06  8:26         ` Paolo Bonzini
2013-09-06  5:53     ` Fam Zheng
2013-09-06  7:20       ` Richard Henderson
2013-09-06  7:27         ` Fam Zheng
2013-09-05 10:20 ` [Qemu-devel] [RFC PATCH 5/6] module: load modules at start Fam Zheng
2013-09-05 11:43   ` Lluís Vilanova
2013-09-06  6:33     ` Fam Zheng
2013-09-05 11:49   ` Michael Tokarev
2013-09-05 10:20 ` [Qemu-devel] [RFC PATCH 6/6] curl: build as shared library Fam Zheng
2013-09-05 10:25 ` [Qemu-devel] [RFC PATCH 0/6] Shared Library Module Support Fam Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5229A348.3080204@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=famz@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).