[Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

The following working patchset demonstrates a one step to plugins system:
it moves various dependent libraries and stuff out from libs_softmmu or
libs_tools to object-specific variables.  When that object is linked
into final executable, corresponding libs are expanded and appended to
the linking command line.

I took block/curl.o as an example (in the last patch).

This is a working example which can be used as is as a starting point
to convert other similar cases.

There are a few questions still.

I'm not sure whenever this $(obj)foo.o syntax (instead of $(obj)/foo.o)
is okay, using the slash is more natural, but when you realize that
objects can be stored in current dir it may be okay.  However, it is
easy to make mistakes here in new code -- probably trivially catchable.

The foo.cflags isn't really necessary, but when you specify one
thing one way (target-specific variable), and another thing completely
different way, resulting code does not look nice.  In particular, the
two curl definitions in block/Makefile.objs look somewhat funky if
curl.cflags isn't used.

It is quite a bit ugly to strip out ../ in the link line, but it is
also ugly to add that ../ in the first place.  Maybe I should add a
comment in Makefile.target where it adds ../ to also refer to
rules.mak, and back, for clarity.

In configure, we may define $(obj)curl.libs directly, but for that
to work we should know where exactly the curl.o is located.

At the same time, we may just as well use basenames (without paths)
to define per-object variables -- this way we wont need to strip
./ from $(obj) or any other black magic, but we should ensure that
all basenames are unique, or else bad things may happen.

When you build a library from an object, its libs aren't propagated.
It is fixable, by creating library.libs variables and expanding them
at executable link time, but since not all objects from the lib may
be necessary, this isn't nice.

Generally, is is expected that obj.libs variables will be used only
for common optional objects like "plugins".

Michael Tokarev (4):
  build-sys: strip leading ./ from $(obj)
  build-sys: allow object-specific libraries to be used to link executables
  build-sys: allow per-object foo.cflags variables
  build-sys: move -lcurl out of libs and specify it for curl.o

 audio/Makefile.objs    |    4 ++--
 backends/Makefile.objs |    2 +-
 block/Makefile.objs    |    4 ++--
 configure              |    3 +--
 hw/audio/Makefile.objs |    2 +-
 rules.mak              |   16 ++++++++++------
 trace/Makefile.objs    |   26 +++++++++++++-------------
 ui/Makefile.objs       |    6 +++---
 8 files changed, 33 insertions(+), 30 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [RFC PATCH 1/4] build-sys: strip leading ./ from $(obj)

[Michael Tokarev]

This change modifies the way how subdir objects are handled.
Previously, $(obj) make variable were starting with `.' (current
directory), and were appended `/subdir' when recursing into subdirs,
so that actual objects in top level dir were named ./obj and in
subdirs - ./subdir/obj.  Remove the `./' suffix and make it plain,
so that file names in variables will match those in objects/flags.

This allows to have, for example, per-object link flags
(to be added in subsequent patches).

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 audio/Makefile.objs    |    4 ++--
 backends/Makefile.objs |    2 +-
 block/Makefile.objs    |    2 +-
 hw/audio/Makefile.objs |    2 +-
 rules.mak              |    6 +++---
 trace/Makefile.objs    |   26 +++++++++++++-------------
 ui/Makefile.objs       |    6 +++---
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/audio/Makefile.objs b/audio/Makefile.objs
index d71a877..6ce7409 100644
--- a/audio/Makefile.objs
+++ b/audio/Makefile.objs
@@ -13,5 +13,5 @@ common-obj-$(CONFIG_AUDIO_PT_INT) += audio_pt_int.o
 common-obj-$(CONFIG_AUDIO_WIN_INT) += audio_win_int.o
 common-obj-y += wavcapture.o
 
-$(obj)/audio.o $(obj)/fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS)
-$(obj)/sdlaudio.o: QEMU_CFLAGS += $(SDL_CFLAGS)
+$(obj)audio.o $(obj)fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS)
+$(obj)sdlaudio.o: QEMU_CFLAGS += $(SDL_CFLAGS)
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 42557d5..5a172d9 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -3,6 +3,6 @@ common-obj-$(CONFIG_POSIX) += rng-random.o
 
 common-obj-y += msmouse.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
-$(obj)/baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
+$(obj)baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
 
 common-obj-$(CONFIG_TPM) += tpm.o
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 2981654..7b7189c 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -22,4 +22,4 @@ common-obj-y += stream.o
 common-obj-y += commit.o
 common-obj-y += mirror.o
 
-$(obj)/curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
+$(obj)curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
diff --git a/hw/audio/Makefile.objs b/hw/audio/Makefile.objs
index 7ce85a2..8dd9827 100644
--- a/hw/audio/Makefile.objs
+++ b/hw/audio/Makefile.objs
@@ -15,4 +15,4 @@ common-obj-$(CONFIG_CS4231) += cs4231.o
 common-obj-$(CONFIG_MARVELL_88W8618) += marvell_88w8618.o
 common-obj-$(CONFIG_MILKYMIST) += milkymist-ac97.o
 
-$(obj)/adlib.o $(obj)/fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0
+$(obj)adlib.o $(obj)fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0
diff --git a/rules.mak b/rules.mak
index 4499745..a6bb4f1 100644
--- a/rules.mak
+++ b/rules.mak
@@ -103,7 +103,7 @@ clean: clean-timestamp
 
 # magic to descend into other directories
 
-obj := .
+obj :=
 old-nested-dirs :=
 
 define push-var
@@ -119,9 +119,9 @@ endef
 
 define unnest-dir
 $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
-$(eval obj := $(obj)/$1)
+$(eval obj := $(obj)$1/)
 $(eval include $(SRC_PATH)/$1/Makefile.objs)
-$(eval obj := $(patsubst %/$1,%,$(obj)))
+$(eval obj := $(patsubst %$1/,%,$(obj)))
 $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
 endef
 
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 3b88e49..5588506 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -3,16 +3,16 @@
 ######################################################################
 # Auto-generated event descriptions
 
-$(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
-$(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
+$(obj)generated-events.h: $(obj)generated-events.h-timestamp
+$(obj)generated-events.h-timestamp: $(SRC_PATH)/trace-events
 	$(call quiet-command,$(TRACETOOL) \
 		--format=events-h \
 		--backend=events \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
-$(obj)/generated-events.c: $(obj)/generated-events.c-timestamp $(BUILD_DIR)/config-host.mak
-$(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events
+$(obj)generated-events.c: $(obj)generated-events.c-timestamp $(BUILD_DIR)/config-host.mak
+$(obj)generated-events.c-timestamp: $(SRC_PATH)/trace-events
 	$(call quiet-command,$(TRACETOOL) \
 		--format=events-c \
 		--backend=events \
@@ -25,9 +25,9 @@ util-obj-y += generated-events.o
 ######################################################################
 # Auto-generated tracing routines
 
-$(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
+$(obj)generated-tracers.h: $(obj)generated-tracers.h-timestamp
 	@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=h \
 		--backend=$(TRACE_BACKEND) \
@@ -37,15 +37,15 @@ $(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 # Auto-generated tracing routines (non-DTrace)
 
 ifneq ($(TRACE_BACKEND),dtrace)
-$(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
+$(obj)generated-tracers.c: $(obj)generated-tracers.c-timestamp
 	@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=c \
 		--backend=$(TRACE_BACKEND) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
-$(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.h
+$(obj)generated-tracers.o: $(obj)generated-tracers.c $(obj)generated-tracers.h
 endif
 
 
@@ -56,18 +56,18 @@ endif
 # but that gets picked up by QEMU's Makefile as an external dependency
 # rule file. So we use '.dtrace' instead
 ifeq ($(TRACE_BACKEND),dtrace)
-$(obj)/generated-tracers.dtrace: $(obj)/generated-tracers.dtrace-timestamp
-$(obj)/generated-tracers.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+$(obj)generated-tracers.dtrace: $(obj)generated-tracers.dtrace-timestamp
+$(obj)generated-tracers.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=d \
 		--backend=$(TRACE_BACKEND) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
-$(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers.dtrace
+$(obj)generated-tracers-dtrace.h: $(obj)generated-tracers.dtrace
 	$(call quiet-command,dtrace -o $@ -h -s $<, "  GEN   $@")
 
-$(obj)/generated-tracers.o: $(obj)/generated-tracers.dtrace
+$(obj)generated-tracers.o: $(obj)generated-tracers.dtrace
 endif
 
 ######################################################################
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 6ddc0de..7265c42 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -15,8 +15,8 @@ common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
 
-$(obj)/sdl.o $(obj)/sdl_zoom.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
+$(obj)sdl.o $(obj)sdl_zoom.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
 
-$(obj)/cocoa.o: $(SRC_PATH)/$(obj)/cocoa.m
+$(obj)cocoa.o: $(SRC_PATH)/$(obj)cocoa.m
 
-$(obj)/gtk.o: QEMU_CFLAGS += $(GTK_CFLAGS) $(VTE_CFLAGS)
+$(obj)gtk.o: QEMU_CFLAGS += $(GTK_CFLAGS) $(VTE_CFLAGS)
-- 
1.7.10.4

[Qemu-devel] [RFC PATCH 2/4] build-sys: allow object-specific libraries to be used to link executables

[Michael Tokarev]

When linking final executables consiting of various object files foo.o,
one may define foo.libs variable to hold libraries needed to be linked
with foo.o.

This way, we may specify dependencies inside the makefile fragments
where corresponding objects are defined.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 rules.mak |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/rules.mak b/rules.mak
index a6bb4f1..69764b9 100644
--- a/rules.mak
+++ b/rules.mak
@@ -22,10 +22,14 @@ QEMU_INCLUDES += -I$(<D) -I$(@D)
 %.o: %.rc
 	$(call quiet-command,$(WINDRES) -I. -o $@ $<,"  RC    $(TARGET_DIR)$@")
 
+# check all .o files and extract any object-specific .libs
+# we remove leading ../ in case the build is done in a subdir.
+extract-libs = $(strip $(foreach o, $(patsubst ../%, %, $(filter %.o, $1)), $($(o:%.o=%.libs))))
+
 ifeq ($(LIBTOOL),)
 LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
        $(sort $(filter %.o, $1)) $(filter-out %.o, $1) $(version-obj-y) \
-       $(LIBS),"  LINK  $(TARGET_DIR)$@")
+       $(LIBS) $(call extract-libs, $1),"  LINK  $(TARGET_DIR)$@")
 else
 LIBTOOL += $(if $(V),,--quiet)
 %.lo: %.c
@@ -41,7 +45,7 @@ LINK = $(call quiet-command,\
        $(sort $(filter %.o, $1)) $(filter-out %.o, $1) \
        $(if $(filter %.lo %.la,$^),$(version-lobj-y),$(version-obj-y)) \
        $(if $(filter %.lo %.la,$^),$(LIBTOOLFLAGS)) \
-       $(LIBS),$(if $(filter %.lo %.la,$^),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
+       $(LIBS) $(call extract-libs, $1),$(if $(filter %.lo %.la,$^),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
 endif
 
 %.asm: %.S
-- 
1.7.10.4

[Qemu-devel] [RFC PATCH 3/4] build-sys: allow per-object foo.cflags variables

[Michael Tokarev]

In addition to allowing specifying foo.libs for additional
libraries, recognize also foo.cflags variable when compiling
foo.c into foo.o.

This is more for completness/symmetry with foo.libs.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 rules.mak |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rules.mak b/rules.mak
index 69764b9..6087c68 100644
--- a/rules.mak
+++ b/rules.mak
@@ -18,7 +18,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
 QEMU_INCLUDES += -I$(<D) -I$(@D)
 
 %.o: %.c
-	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
+	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($*.cflags) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
 %.o: %.rc
 	$(call quiet-command,$(WINDRES) -I. -o $@ $<,"  RC    $(TARGET_DIR)$@")
 
-- 
1.7.10.4

[Qemu-devel] [RFC PATCH 4/4] build-sys: move -lcurl out of libs and specify it for curl.o

[Michael Tokarev]

We only need to link -lcurl into executables which actually
use/link curl.o, so make -lcurl object-specific library,
using curl.libs variable.

While at it, also use new curl.cflags variable to specify
curl-specific cflags, for symmetry with .libs, and move
whole declaration into the if..endif block where curl.o
is defined.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 block/Makefile.objs |    4 ++--
 configure           |    3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 7b7189c..9dc54c9 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -13,6 +13,8 @@ ifeq ($(CONFIG_POSIX),y)
 block-obj-y += nbd.o sheepdog.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(CONFIG_CURL) += curl.o
+$(obj)curl.cflags = $(CURL_CFLAGS)
+$(obj)curl.libs = $(CURL_LIBS)
 block-obj-$(CONFIG_RBD) += rbd.o
 block-obj-$(CONFIG_GLUSTERFS) += gluster.o
 block-obj-$(CONFIG_LIBSSH2) += ssh.o
@@ -21,5 +23,3 @@ endif
 common-obj-y += stream.o
 common-obj-y += commit.o
 common-obj-y += mirror.o
-
-$(obj)curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
diff --git a/configure b/configure
index ad32f87..fe6d8b1 100755
--- a/configure
+++ b/configure
@@ -2176,8 +2176,6 @@ EOF
   curl_libs=`$curlconfig --libs 2>/dev/null`
   if compile_prog "$curl_cflags" "$curl_libs" ; then
     curl=yes
-    libs_tools="$curl_libs $libs_tools"
-    libs_softmmu="$curl_libs $libs_softmmu"
   else
     if test "$curl" = "yes" ; then
       feature_not_found "curl"
@@ -3784,6 +3782,7 @@ fi
 if test "$curl" = "yes" ; then
   echo "CONFIG_CURL=y" >> $config_host_mak
   echo "CURL_CFLAGS=$curl_cflags" >> $config_host_mak
+  echo "CURL_LIBS=$curl_libs" >> $config_host_mak
 fi
 if test "$brlapi" = "yes" ; then
   echo "CONFIG_BRLAPI=y" >> $config_host_mak
-- 
1.7.10.4

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

18.06.2013 21:34, Michael Tokarev wrote:
> The following working patchset demonstrates a one step to plugins system:
> it moves various dependent libraries and stuff out from libs_softmmu or
> libs_tools to object-specific variables.  When that object is linked
> into final executable, corresponding libs are expanded and appended to
> the linking command line.

I also made a few more changes, to recognize "complex modules" which
consists of more than one .o file, and also added a few other minor
changes.

It is all available at

  http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/mjt-buildsys

> I took block/curl.o as an example (in the last patch).

It now also converts bluez.

Also, it uses qed as a "complex module" example.

> This is a working example which can be used as is as a starting point
> to convert other similar cases.
> 
> There are a few questions still.
> 
> I'm not sure whenever this $(obj)foo.o syntax (instead of $(obj)/foo.o)
> is okay, using the slash is more natural, but when you realize that
> objects can be stored in current dir it may be okay.  However, it is
> easy to make mistakes here in new code -- probably trivially catchable.
> 
> The foo.cflags isn't really necessary, but when you specify one
> thing one way (target-specific variable), and another thing completely
> different way, resulting code does not look nice.  In particular, the
> two curl definitions in block/Makefile.objs look somewhat funky if
> curl.cflags isn't used.
> 
> It is quite a bit ugly to strip out ../ in the link line, but it is
> also ugly to add that ../ in the first place.  Maybe I should add a
> comment in Makefile.target where it adds ../ to also refer to
> rules.mak, and back, for clarity.

It is not done anymore, but LINK command now accepts extra argument.
Both ways aren't nide.

> In configure, we may define $(obj)curl.libs directly, but for that
> to work we should know where exactly the curl.o is located.
> 
> At the same time, we may just as well use basenames (without paths)
> to define per-object variables -- this way we wont need to strip
> ./ from $(obj) or any other black magic, but we should ensure that
> all basenames are unique, or else bad things may happen.
> 
> When you build a library from an object, its libs aren't propagated.
> It is fixable, by creating library.libs variables and expanding them
> at executable link time, but since not all objects from the lib may
> be necessary, this isn't nice.
> 
> Generally, is is expected that obj.libs variables will be used only
> for common optional objects like "plugins".

The next step is to introduce common-obj-m and block-obj-m variables
to build the plugins.  This is almost trivial now, provided you
specify necessary extra_ldflags (-rdynamic -ldl), build in pie mode
(-pie), and have necessary changes applied to load the modules.

But I think additional stage is needed now: to also build some
shared library with all the common-obj and block-obj.  I'm not
sure supporing plugins without that shared library is a good idea,
but if we do, there are just too many variants (shared lib or not,
plugins or not).  In case shared library is in use, there's no
need to specify -rdynamic, either.  But shared lib will require
quite some more changes in the build system.

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Stefan Hajnoczi]

On Tue, Jun 18, 2013 at 09:34:00PM +0400, Michael Tokarev wrote:
> The foo.cflags isn't really necessary, but when you specify one
> thing one way (target-specific variable), and another thing completely
> different way, resulting code does not look nice.  In particular, the
> two curl definitions in block/Makefile.objs look somewhat funky if
> curl.cflags isn't used.

Specifying per-object file or per-module LIBS/CFLAGS is great.  I wonder
if we can do it like kbuild though:
CFLAGS_i915_trace_points.o := -I$(src)

It only takes the object file name, not the path.  Could be a problem if
we have files with the same name in different directories I guess.

But I wonder if this approach would let you drop the $(obj)filename.o
change.  I didn't review closely enough to really understand how that
works though :-).

Stefan

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

19.06.2013 18:16, Stefan Hajnoczi wrote:
> On Tue, Jun 18, 2013 at 09:34:00PM +0400, Michael Tokarev wrote:
>> The foo.cflags isn't really necessary, but when you specify one
>> thing one way (target-specific variable), and another thing completely
>> different way, resulting code does not look nice.  In particular, the
>> two curl definitions in block/Makefile.objs look somewhat funky if
>> curl.cflags isn't used.
> 
> Specifying per-object file or per-module LIBS/CFLAGS is great.  I wonder
> if we can do it like kbuild though:
> CFLAGS_i915_trace_points.o := -I$(src)
> 
> It only takes the object file name, not the path.  Could be a problem if
> we have files with the same name in different directories I guess.

Yes, that's exactly the problem here -- duplicate file names.  Currently
we have some duplicates -- for example:

hw/bt/core.c
hw/usb/core.c
hw/acpi/core.c
hw/i2c/core.c
hw/ide/core.c

block.c
hw/block/block.c

hw/bt/hid.c
hw/input/hid.c

and so on.  These will have to be renamed for this approach to work, and
we will end up always risking having more duplicates.

More, it is difficult to watch for them, since we have roms/ and tests/
which are _full_ of dups :)

So I'm afraid this wont work.

> But I wonder if this approach would let you drop the $(obj)filename.o
> change.  I didn't review closely enough to really understand how that
> works though :-).

Well, it works, provided we don't forget to NOT put a slash after $(obj)
(which should be easy to verify btw, make can check for that like it
already does in my changes for missing $(foo.mod) variables).

And after thinking more about it -- it does not look that bad.

Alternative it to ALWAYS specify $(obj) in all Makefile.objs so we don't
need to expand just the special variables (common-obj-y, block-obj-y
etc) and it always works without additional expansion.

Thank you for the review!

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 19/06/2013 16:16, Stefan Hajnoczi ha scritto:
>> The foo.cflags isn't really necessary, but when you specify one
>> > thing one way (target-specific variable), and another thing completely
>> > different way, resulting code does not look nice.  In particular, the
>> > two curl definitions in block/Makefile.objs look somewhat funky if
>> > curl.cflags isn't used.
> Specifying per-object file or per-module LIBS/CFLAGS is great.  I wonder
> if we can do it like kbuild though:
> CFLAGS_i915_trace_points.o := -I$(src)

I think that works for kbuild because under the hood it is recursive
makefile (using the trick that foo/bar/ does a recursive invocation).
QEMU's makefile is non-recursive.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 18/06/2013 19:34, Michael Tokarev ha scritto:
> The following working patchset demonstrates a one step to plugins system:
> it moves various dependent libraries and stuff out from libs_softmmu or
> libs_tools to object-specific variables.  When that object is linked
> into final executable, corresponding libs are expanded and appended to
> the linking command line.
> 
> I took block/curl.o as an example (in the last patch).
> 
> This is a working example which can be used as is as a starting point
> to convert other similar cases.

I was thinking of this problem too while drafting the modules feature
page.  My solution had some duplication:

libs-$(CONFIG_CURL) += $(CURL_LIBS)
$(obj)/curl.so: libs-$(CONFIG_CURL) += $(CURL_LIBS)

where the .so rule would use $(libs-y) $(libs-m).

> There are a few questions still.
> 
> I'm not sure whenever this $(obj)foo.o syntax (instead of $(obj)/foo.o)
> is okay, using the slash is more natural, but when you realize that
> objects can be stored in current dir it may be okay.  However, it is
> easy to make mistakes here in new code -- probably trivially catchable.
> 
> The foo.cflags isn't really necessary, but when you specify one
> thing one way (target-specific variable), and another thing completely
> different way, resulting code does not look nice.  In particular, the
> two curl definitions in block/Makefile.objs look somewhat funky if
> curl.cflags isn't used.

I like this solution, and I agree that consistency between cflags and
libs is good.  However, it would be great if you could do it without
changing the meaning of $(obj).  It is not clear to me (from reading the
patches) why you need that.

Also, for the inevitable bikeshedding, I would prefer

   cflags-$(obj)/curl.o-y
   libs-$(obj)/curl.o-y

> It is quite a bit ugly to strip out ../ in the link line, but it is
> also ugly to add that ../ in the first place.  Maybe I should add a
> comment in Makefile.target where it adds ../ to also refer to
> rules.mak, and back, for clarity.

The ../ is ugly indeed.  Perhaps we can instead use something like

common.o: $(patsubst %,../%, $(common-obj-y))
	$(LD) -r -o $@ $^

and then link common.o into the QEMU target.  Libtool can also be used
to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
(we'd need it anyway for modules) as long as you do not need libtool to
start QEMU or gdb from the build directory.

> In configure, we may define $(obj)curl.libs directly, but for that
> to work we should know where exactly the curl.o is located.
> 
> At the same time, we may just as well use basenames (without paths)
> to define per-object variables -- this way we wont need to strip
> ./ from $(obj) or any other black magic, but we should ensure that
> all basenames are unique, or else bad things may happen.

This would be worse than the disease, I think.

> When you build a library from an object, its libs aren't propagated.
> It is fixable, by creating library.libs variables and expanding them
> at executable link time, but since not all objects from the lib may
> be necessary, this isn't nice.
> 
> Generally, is is expected that obj.libs variables will be used only
> for common optional objects like "plugins".

Not necessarily! :)

Paolo

> Michael Tokarev (4):
>   build-sys: strip leading ./ from $(obj)
>   build-sys: allow object-specific libraries to be used to link executables
>   build-sys: allow per-object foo.cflags variables
>   build-sys: move -lcurl out of libs and specify it for curl.o
> 
>  audio/Makefile.objs    |    4 ++--
>  backends/Makefile.objs |    2 +-
>  block/Makefile.objs    |    4 ++--
>  configure              |    3 +--
>  hw/audio/Makefile.objs |    2 +-
>  rules.mak              |   16 ++++++++++------
>  trace/Makefile.objs    |   26 +++++++++++++-------------
>  ui/Makefile.objs       |    6 +++---
>  8 files changed, 33 insertions(+), 30 deletions(-)
> 

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

19.06.2013 20:58, Paolo Bonzini wrote:
> Il 18/06/2013 19:34, Michael Tokarev ha scritto:
>> The following working patchset demonstrates a one step to plugins system:
>> it moves various dependent libraries and stuff out from libs_softmmu or
>> libs_tools to object-specific variables.  When that object is linked
>> into final executable, corresponding libs are expanded and appended to
>> the linking command line.
>>
>> I took block/curl.o as an example (in the last patch).
>>
>> This is a working example which can be used as is as a starting point
>> to convert other similar cases.
> 
> I was thinking of this problem too while drafting the modules feature
> page.  My solution had some duplication:
> 
> libs-$(CONFIG_CURL) += $(CURL_LIBS)
> $(obj)/curl.so: libs-$(CONFIG_CURL) += $(CURL_LIBS)
> 
> where the .so rule would use $(libs-y) $(libs-m).

/me dislikes duplication :)

[]
> libs is good.  However, it would be great if you could do it without
> changing the meaning of $(obj).  It is not clear to me (from reading the
> patches) why you need that.

I'm not really changing meaning of $(obj), it is still a subdir, but
it might be empty (for a top-level dir) too, which is different from
current-dir  (.).

Initially your $(obj) starts with `.', and expands with slashes at the
end.  So a object-specific variables -- like


 $(obj)/foo.cflags = -DFOOBAR

are - after expansion - named like  `./foo.cflags'.  Nothing is wrong
with that.

The only prob here is that when you expand lists of objects/modules to
process, it includes foo.o bar.mod, not ./foo.o ./bar.mod.

Currently I expand it like this:

  $(foreach m, $(filter %.o,$1), $($(m:%.o=%.libs)))

Probably I can change that to

  $(foreach m, $(filter %.o,$1), $($(m:%.o=./%.libs)))

(here and in other similar cases), and it will work without changing
anything around $(obj).

But maybe we can argee here that this is not really OBJect, it is
a path or dir, and name it $(d) or $(p) instead of $(obj) ?  To
include the slash when needed. just like I did for $(obj).

It really is a directory prefix (hence I prefer $(p) -- it is short
and can be referred to as either path or prefix), it is not a directory
with object files.  When you think about it this way, it becomes
much cleaner.

Also, it should be possible to add a test that all $(obj)- (or $(p)-)
prefixed paths don't include the slash, so in case of any mistakes
happens they will be detected early.

And since it looks like this mechanism is useful enough to be
used widely, I think the shorter the name, the better.

> Also, for the inevitable bikeshedding, I would prefer
> 
>    cflags-$(obj)/curl.o-y
>    libs-$(obj)/curl.o-y

What are all these -y suffixes for?  In existing variables and in
this new your invention?  It's already a bit too verbose.

BTW, can you take a look why your expand-nesting does not remove
the save- variables properly?  Run make with -p and see which
vars are defined.  (This is really BTW, you just reminded me
about something I've seen but had no chance to look at).

>> It is quite a bit ugly to strip out ../ in the link line, but it is
>> also ugly to add that ../ in the first place.  Maybe I should add a
>> comment in Makefile.target where it adds ../ to also refer to
>> rules.mak, and back, for clarity.
> 
> The ../ is ugly indeed.  Perhaps we can instead use something like

I already changed that part a bit, by adding a second argument
to the $(LINK) rule, but I don't think it is less ugly.

> common.o: $(patsubst %,../%, $(common-obj-y))
> 	$(LD) -r -o $@ $^
> 
> and then link common.o into the QEMU target.

Well, that might not be a bad idea.  I'll think about it.

That might actually be turned into a shared library (or two
or even 3 -- for tools, for softmmu targets and for user
emulation).

>        Libtool can also be used
> to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
> (we'd need it anyway for modules) as long as you do not need libtool to
> start QEMU or gdb from the build directory.

Do we really need it for modules?  I'm not sure.  Actually, in a modern
world, I'm not really sure libtool is needed if you only count "major"
operating systems.  We're basically stick with gcc and so with whole
toolchain (and with gnu make having in mind all the above tricks!),
even windows programs are build with mingw only.  In this restricted
environment it is easy to go stright into gcc command line instead of
using a wrapper which adds its own complexity.  And even if that is
not sufficient, we can just skip playing all these plugin/shared
games on "unsupported" systems.

I already looked at the gmodule wrapper around dlopen()/LoadLibrary() --
there's no need even in that wrapper, stright OS interface looks more
accurate.

(Speaking of running qemu from the build dir -- I think we'll need an
option, like -plugin-dir, to be able to speclfy plugins just built).

>> When you build a library from an object, its libs aren't propagated.
>> It is fixable, by creating library.libs variables and expanding them
>> at executable link time, but since not all objects from the lib may
>> be necessary, this isn't nice.
>>
>> Generally, is is expected that obj.libs variables will be used only
>> for common optional objects like "plugins".
> 
> Not necessarily! :)

Well, time will tell ;)
But I don't actually see a reason for that, -- if the mechanism will
be easy and works, there's no problem in using it.

Thanks!

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 19/06/2013 20:18, Michael Tokarev ha scritto:
> Currently I expand it like this:
> 
>   $(foreach m, $(filter %.o,$1), $($(m:%.o=%.libs)))
> 
> Probably I can change that to
> 
>   $(foreach m, $(filter %.o,$1), $($(m:%.o=./%.libs)))
> 
> (here and in other similar cases), and it will work without changing
> anything around $(obj).
> 
> But maybe we can argee here that this is not really OBJect, it is
> a path or dir, and name it $(d) or $(p) instead of $(obj) ?  To
> include the slash when needed. just like I did for $(obj).

I chose $(obj) because that's what Kbuild uses.

>> > Also, for the inevitable bikeshedding, I would prefer
>> > 
>> >    cflags-$(obj)/curl.o-y
>> >    libs-$(obj)/curl.o-y
> What are all these -y suffixes for?  In existing variables and in
> this new your invention?  It's already a bit too verbose.

It is so that you can do

foo-$(CONFIG_XYZ) += blah

instead of

ifeq ($(CONFIG_XYZ),y)
FOO += blah
endif

> BTW, can you take a look why your expand-nesting does not remove
> the save- variables properly?  Run make with -p and see which
> vars are defined.  (This is really BTW, you just reminded me
> about something I've seen but had no chance to look at).

Because I didn't bother. :)

>> common.o: $(patsubst %,../%, $(common-obj-y))
>> 	$(LD) -r -o $@ $^
>>
>> and then link common.o into the QEMU target.

I think that would make it a bit more complex to gather all the required
libs.  But it is probably not insurmountable.

>> >        Libtool can also be used
>> > to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
>> > (we'd need it anyway for modules) as long as you do not need libtool to
>> > start QEMU or gdb from the build directory.
> Do we really need it for modules?  I'm not sure.  Actually, in a modern
> world, I'm not really sure libtool is needed if you only count "major"
> operating systems.

Do you also count one major proprietary operating system?  Unfortunately
mingw still requires magic to create shared libraries.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Richard Henderson]

On 06/19/2013 11:52 AM, Paolo Bonzini wrote:
>> > Do we really need it for modules?  I'm not sure.  Actually, in a modern
>> > world, I'm not really sure libtool is needed if you only count "major"
>> > operating systems.
> Do you also count one major proprietary operating system?  Unfortunately
> mingw still requires magic to create shared libraries.

Does it?  It sure doesn't seem like it to me:

----

$ cat z.c
int __attribute__((dllexport)) foo(int x)
{
  return x + 1;
}

$ x86_64-w64-mingw32-gcc -shared -o z.dll z.c

$ file z.dll
z.dll: PE32+ executable (DLL) (console) x86-64, for MS Windows

$  x86_64-w64-mingw32-objdump.exe -p z.dll
...
There is an export table in .edata at 0x6788c000

The Export Tables (interpreted .edata section contents)

Export Flags                    0
Time/Date stamp                 51c20613
Major/Minor                     0/0
Name                            000000000000c032 z.dll
Ordinal Base                    1
Number in:
        Export Address Table            00000001
        [Name Pointer/Ordinal] Table    00000001
Table Addresses
        Export Address Table            000000000000c028
        Name Pointer Table              000000000000c02c
        Ordinal Table                   000000000000c030

Export Address Table -- Ordinal Base 1
        [   0] +base[   1] 1470 Export RVA

[Ordinal/Name Pointer] Table
        [   0] foo

----

I didn't happen to happen to have the 32-bit mingw installed in this VM, but I
don't imagine it's much different from the 64-bit version.


r~

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

19.06.2013 22:52, Paolo Bonzini wrote:
> Il 19/06/2013 20:18, Michael Tokarev ha scritto:
>> Currently I expand it like this:
>>
>>   $(foreach m, $(filter %.o,$1), $($(m:%.o=%.libs)))
>>
>> Probably I can change that to
>>
>>   $(foreach m, $(filter %.o,$1), $($(m:%.o=./%.libs)))
>>
>> (here and in other similar cases), and it will work without changing
>> anything around $(obj).
>>
>> But maybe we can argee here that this is not really OBJect, it is
>> a path or dir, and name it $(d) or $(p) instead of $(obj) ?  To
>> include the slash when needed. just like I did for $(obj).
> 
> I chose $(obj) because that's what Kbuild uses.

kbuild almost never requires this variable in "user" makefiles
(not counting kbuild internals), -- because of recursive way
of its working there's no need to prepend directory names like
qemu makefiles currently do, "user" makefiles refer to files
using bare (no path) names.

Contrary to that, qemu does not recurse, it tries to do everything
in one instance, so it can expand just a few "magic" variables,
and for everything else we have to explicitly use that prefix.
So that it becomes inconsistent (some vars require prefix, some
don't), and too verbose.

In this context, maybe it is good idea to actually get rid of
both issues -- being inconsistent (the more per-object flags
we add, the more inconsisten it becomes, since more and more
variables requires the prefix), and make it as short as possible
(so that using one-char variable name is better than 3-char).

(by "user" I mean regular makefiles used in various places in
the kernel, as opposed to kbuild internals).

>>>> Also, for the inevitable bikeshedding, I would prefer
>>>>
>>>>    cflags-$(obj)/curl.o-y
>>>>    libs-$(obj)/curl.o-y
>> What are all these -y suffixes for?  In existing variables and in
>> this new your invention?  It's already a bit too verbose.
> 
> It is so that you can do
> 
> foo-$(CONFIG_XYZ) += blah
> 
> instead of
> 
> ifeq ($(CONFIG_XYZ),y)
> FOO += blah
> endif

Yes, that sure I undertand -- obj-y, block-m etc.  I'm asking
about those new vars yuo propose -- why you want -y sufffix
*there*, like cflags-foo.o-y ?

Do you want to be able to specify different flags for -y and -m
builds?  Isn't it a bit too much?

>> BTW, can you take a look why your expand-nesting does not remove
>> the save- variables properly?  Run make with -p and see which
>> vars are defined.  (This is really BTW, you just reminded me
>> about something I've seen but had no chance to look at).
> 
> Because I didn't bother. :)

Actually you did - you tried to clean things up a bit, but some
dirt is still there.

>>> common.o: $(patsubst %,../%, $(common-obj-y))
>>> 	$(LD) -r -o $@ $^
>>>
>>> and then link common.o into the QEMU target.
> 
> I think that would make it a bit more complex to gather all the required
> libs.  But it is probably not insurmountable.

Yes, definitely.

>>>>        Libtool can also be used
>>>> to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
>>>> (we'd need it anyway for modules) as long as you do not need libtool to
>>>> start QEMU or gdb from the build directory.
>> Do we really need it for modules?  I'm not sure.  Actually, in a modern
>> world, I'm not really sure libtool is needed if you only count "major"
>> operating systems.
> 
> Do you also count one major proprietary operating system?  Unfortunately
> mingw still requires magic to create shared libraries.

Sure.  But it's not alot really.

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Peter Maydell]

On 20 June 2013 10:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This only leaves Darwin.  I have no idea about that, and I don't have
> anymore a machine to test it.  Andreas or Peter, can you shed light?

If you have something concrete you'd like me to test I can test it.

> But still, libtool wouldn't be a particularly problematic dependency.
> We're already using it for libcacard.

...and we're already disabling the probe for libtool in configure
on MacOSX because MacOS libtool is something completely different...

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 19/06/2013 22:00, Michael Tokarev ha scritto:
> 19.06.2013 22:52, Paolo Bonzini wrote:
>> Il 19/06/2013 20:18, Michael Tokarev ha scritto:
>>> Currently I expand it like this:
>>>
>>>   $(foreach m, $(filter %.o,$1), $($(m:%.o=%.libs)))
>>>
>>> Probably I can change that to
>>>
>>>   $(foreach m, $(filter %.o,$1), $($(m:%.o=./%.libs)))
>>>
>>> (here and in other similar cases), and it will work without changing
>>> anything around $(obj).
>>>
>>> But maybe we can argee here that this is not really OBJect, it is
>>> a path or dir, and name it $(d) or $(p) instead of $(obj) ?  To
>>> include the slash when needed. just like I did for $(obj).
>>
>> I chose $(obj) because that's what Kbuild uses.
> 
> kbuild almost never requires this variable in "user" makefiles
> (not counting kbuild internals), -- because of recursive way
> of its working there's no need to prepend directory names like
> qemu makefiles currently do, "user" makefiles refer to files
> using bare (no path) names.
> 
> Contrary to that, qemu does not recurse, it tries to do everything
> in one instance, so it can expand just a few "magic" variables,
> and for everything else we have to explicitly use that prefix.
> So that it becomes inconsistent (some vars require prefix, some
> don't), and too verbose.

I think something like this would still be messy:

common-obj-y += $(p)core.o $(p)smbus.o $(p)smbus_eeprom.o
common-obj-$(CONFIG_VERSATILE_I2C) += $(p)versatile_i2c.o
common-obj-$(CONFIG_ACPI) += $(p)smbus_ich9.o
common-obj-$(CONFIG_APM) += $(p)pm_smbus.o
common-obj-$(CONFIG_BITBANG_I2C) += $(p)bitbang_i2c.o
common-obj-$(CONFIG_EXYNOS4) += $(p)exynos4210_i2c.o
obj-$(CONFIG_OMAP) += $(p)omap_i2c.o

and this is why I preferred per-target variable values to magic variable
names.

I also disliked the duplication, but in fact you do not even have the
duplication if you use libtool and make a .la convenience library
(similar to "ld -r" but preserving library dependencies) for each
module, even if it is builtin.  Then you can link the .la either into a
.so module, or add it to the executable.

If you do this, the LIBS only goes into a $(obj)/foo.la target-specific
variable.

>>>>> Also, for the inevitable bikeshedding, I would prefer
>>>>>
>>>>>    cflags-$(obj)/curl.o-y
>>>>>    libs-$(obj)/curl.o-y
>>> What are all these -y suffixes for?  In existing variables and in
>>> this new your invention?  It's already a bit too verbose.
>>
>> It is so that you can do
>>
>> foo-$(CONFIG_XYZ) += blah
>>
>> instead of
>>
>> ifeq ($(CONFIG_XYZ),y)
>> FOO += blah
>> endif
> 
> Yes, that sure I undertand -- obj-y, block-m etc.  I'm asking
> about those new vars yuo propose -- why you want -y sufffix
> *there*, like cflags-foo.o-y ?
> 
> Do you want to be able to specify different flags for -y and -m
> builds?  Isn't it a bit too much?

I was thinking of a module that optionally uses a library.  For example
raw block I/O may optionally use the linux AIO library.  But you would do

module-raw-posix-$(CONFIG_LINUX_AIO) += linux-aio.o
libs-linux-aio.o = -laio

or something like that, not

module-raw-posix-$(CONFIG_LINUX_AIO) += linux-aio.o
libs-raw-posix-$(CONFIG_LINUX_AIO) += -laio

So it looks like there's no need for the -y here, indeed.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 20/06/2013 12:06, Peter Maydell ha scritto:
>> > This only leaves Darwin.  I have no idea about that, and I don't have
>> > anymore a machine to test it.  Andreas or Peter, can you shed light?
> If you have something concrete you'd like me to test I can test it.

You can modify the configure probe to detect GNU libtool (Mac OS X calls
it glibtool, IIRC), and attach the log of "make libcacard.la V=1".

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Peter Maydell]

On 20 June 2013 13:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 20/06/2013 12:06, Peter Maydell ha scritto:
>>> > This only leaves Darwin.  I have no idea about that, and I don't have
>>> > anymore a machine to test it.  Andreas or Peter, can you shed light?
>> If you have something concrete you'd like me to test I can test it.
>
> You can modify the configure probe to detect GNU libtool (Mac OS X calls
> it glibtool, IIRC), and attach the log of "make libcacard.la V=1".

Unfortunately MacOSX doesn't have the 'nss' prereq so
CONFIG_SMARTCARD_NSS isn't set and make just says
make: *** No rule to make target `libcacard.la'.  Stop.

-- PMM

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Richard Henderson]

On 06/20/2013 02:49 AM, Paolo Bonzini wrote:
> This only leaves Darwin.  I have no idea about that, and I don't have
> anymore a machine to test it.  Andreas or Peter, can you shed light?

I have an idea that -shared works there as well.

r~

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

[Rehashing a previous thread]

19.06.2013 20:58, Paolo Bonzini wrote:
> Il 18/06/2013 19:34, Michael Tokarev ha scritto:
>> The following working patchset demonstrates a one step to plugins system:
>> it moves various dependent libraries and stuff out from libs_softmmu or
>> libs_tools to object-specific variables.  When that object is linked
>> into final executable, corresponding libs are expanded and appended to
>> the linking command line.
>>
>> I took block/curl.o as an example (in the last patch).
>>
>> This is a working example which can be used as is as a starting point
>> to convert other similar cases.
> 
> I was thinking of this problem too while drafting the modules feature
> page.  My solution had some duplication:
> 
> libs-$(CONFIG_CURL) += $(CURL_LIBS)
> $(obj)/curl.so: libs-$(CONFIG_CURL) += $(CURL_LIBS)
> 
> where the .so rule would use $(libs-y) $(libs-m).
> 
>> There are a few questions still.
>>
>> I'm not sure whenever this $(obj)foo.o syntax (instead of $(obj)/foo.o)
>> is okay, using the slash is more natural, but when you realize that
>> objects can be stored in current dir it may be okay.  However, it is
>> easy to make mistakes here in new code -- probably trivially catchable.
>>
>> The foo.cflags isn't really necessary, but when you specify one
>> thing one way (target-specific variable), and another thing completely
>> different way, resulting code does not look nice.  In particular, the
>> two curl definitions in block/Makefile.objs look somewhat funky if
>> curl.cflags isn't used.
> 
> I like this solution, and I agree that consistency between cflags and
> libs is good.  However, it would be great if you could do it without
> changing the meaning of $(obj).  It is not clear to me (from reading the
> patches) why you need that.
> 
> Also, for the inevitable bikeshedding, I would prefer
> 
>    cflags-$(obj)/curl.o-y
>    libs-$(obj)/curl.o-y

There's no need to have -y or -n prefix here, at least for now,
because a) it does look ugly -- this is what we have to actually
specify in makefile, and b) we don't really need to have different
libs for static vs dynamic linking or flags.  I think anyway.

Also, my version (which uses $(obj)/curl.{libs,cflags,mod}) is safer
because the names will not clash with each other.

>> It is quite a bit ugly to strip out ../ in the link line, but it is
>> also ugly to add that ../ in the first place.  Maybe I should add a
>> comment in Makefile.target where it adds ../ to also refer to
>> rules.mak, and back, for clarity.
> 
> The ../ is ugly indeed.  Perhaps we can instead use something like
> 
> common.o: $(patsubst %,../%, $(common-obj-y))
> 	$(LD) -r -o $@ $^
> 
> and then link common.o into the QEMU target.  Libtool can also be used
> to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
> (we'd need it anyway for modules) as long as you do not need libtool to
> start QEMU or gdb from the build directory.

I think this will mean that whole common.o will have to be linked
into each executable which uses it, even if some executables are
only using just one function from whole lot of .o files.

And I think we don't really have a choice here BUT to actually build a
shared library with all this stuff, which will be loaded either by
qemu-system binary or by tools (or by qemu-user for that matter) --
it looks like the only sane way to get all required-for-plugins symbols
is to use a shared library.

The same mechanism can be used to build .a static library too, so there
will be no different rules for plugins-vs-non-plugins cases.

And if foo.so or foo.a library is built, all the foo.libs of all
objects/modules which are members of that library, can be combined
into ${fooo.libs} just as well.

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Andreas Färber]

Am 18.06.2013 19:34, schrieb Michael Tokarev:
> The following working patchset demonstrates a one step to plugins system:
> it moves various dependent libraries and stuff out from libs_softmmu or
> libs_tools to object-specific variables.

We did have a more elaborate Makefile variable system before, but Paolo
stashed most of that into common-obj-y and obj-y for simplicity.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

30.06.2013 19:28, Andreas Färber wrote:
> Am 18.06.2013 19:34, schrieb Michael Tokarev:
>> The following working patchset demonstrates a one step to plugins system:
>> it moves various dependent libraries and stuff out from libs_softmmu or
>> libs_tools to object-specific variables.
> 
> We did have a more elaborate Makefile variable system before, but Paolo
> stashed most of that into common-obj-y and obj-y for simplicity.

I don't understand.  I for one like to see a plugins system used in qemu,
and except of the build system everything else is easy (and even nice,
there's even no need to load all plugins at startup as was initially
suggested).  But for this to work, we really need to separate libs
used only by plugins from the main lot, -- or else there's just no
reason to build plugins in the first place.

So, are you saying we should abandom this whole idea?  Or that maybe
Paolo dislikes it (I think he expressed his interest here too)?

Count me confused.

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Peter Maydell]

On 30 June 2013 16:36, Michael Tokarev <mjt@tls.msk.ru> wrote:
> I don't understand.  I for one like to see a plugins system used in qemu,
> and except of the build system everything else is easy (and even nice,
> there's even no need to load all plugins at startup as was initially
> suggested).  But for this to work, we really need to separate libs
> used only by plugins from the main lot, -- or else there's just no
> reason to build plugins in the first place.

So, why do we want to build things as plugins? Neither of the
cover letters to your two patch series nor Anthony's wiki page
on modules actually give the rationale.

I'd like to see the reasons why we want this feature clearly laid
out, because it is a big load of extra faff and potential portability
issues (surrounding dlopen, possible use of libtool, etc), as
well as being really easy to misunderstand as some kind of promise
of API/ABI stability or third-party extension functionality.[*]

[*] by which I mean that although everybody here proposing patches
is clear that that's out of scope, end users don't generally read
patches and rationale emails, and "we dlopen device models" looks
really really like a plugin ABI...

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Andreas Färber]

Am 30.06.2013 17:36, schrieb Michael Tokarev:
> 30.06.2013 19:28, Andreas Färber wrote:
>> Am 18.06.2013 19:34, schrieb Michael Tokarev:
>>> The following working patchset demonstrates a one step to plugins system:
>>> it moves various dependent libraries and stuff out from libs_softmmu or
>>> libs_tools to object-specific variables.
>>
>> We did have a more elaborate Makefile variable system before, but Paolo
>> stashed most of that into common-obj-y and obj-y for simplicity.
> 
> I don't understand.  I for one like to see a plugins system used in qemu,
> and except of the build system everything else is easy (and even nice,
> there's even no need to load all plugins at startup as was initially
> suggested).  But for this to work, we really need to separate libs
> used only by plugins from the main lot, -- or else there's just no
> reason to build plugins in the first place.
> 
> So, are you saying we should abandom this whole idea?  Or that maybe
> Paolo dislikes it (I think he expressed his interest here too)?

I haven't read the whole thread yet, so count me confused too, including
that Paolo didn't reply to that part of the message at all.

Whenever the question of a plugin system came up, it was mostly about
desperate attempts to try to sneak GPL-incompatible code into QEMU.

I doubt that is the case here, so I am not generally opposed. I assume
your interest is rather reducing packaging dependencies for headless
installs etc.?

The only thing I was pointing out for now is that with regards to our
build system our ship seems to have a slingering course, with objects
originally being grouped by functionality/scope, then thrown into a big
pot, now apparently being picked apart again.

And implicitly I was hinting that there are people with out-of-tree
patchsets that constantly need to rebase on these Makefile changes, me
finding a whole Makefile.objs as conflict much more confusing to resolve
than a new file not compiling due to some CPUState or QOM code changes.

Not saying you shouldn't apply changes for cool features, please just
coordinate with Paolo to avoid unnecessary back-and-forth in the build
system.

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

30.06.2013 19:51, Peter Maydell wrote:
> On 30 June 2013 16:36, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> I don't understand.  I for one like to see a plugins system used in qemu,
>> and except of the build system everything else is easy (and even nice,
>> there's even no need to load all plugins at startup as was initially
>> suggested).  But for this to work, we really need to separate libs
>> used only by plugins from the main lot, -- or else there's just no
>> reason to build plugins in the first place.
> 
> So, why do we want to build things as plugins? Neither of the
> cover letters to your two patch series nor Anthony's wiki page
> on modules actually give the rationale.

I just updated the wiki page a bit (which is at
http://wiki.qemu.org/Features/Modules btw).

The main rationale so far is to allow separate packaging of separate
features for downstreams, to reduce the need to install all dependent
libraries/packages.

> I'd like to see the reasons why we want this feature clearly laid
> out, because it is a big load of extra faff and potential portability
> issues (surrounding dlopen, possible use of libtool, etc), as
> well as being really easy to misunderstand as some kind of promise
> of API/ABI stability or third-party extension functionality.[*]
> 
> [*] by which I mean that although everybody here proposing patches
> is clear that that's out of scope, end users don't generally read
> patches and rationale emails, and "we dlopen device models" looks
> really really like a plugin ABI...

This has been mentioned already, one of the easy ways to prevent this
is to add some "build id" to all main symbols, so that only modules
from the same build will be loadable.  I mentioned this in the wiki
page too.

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Stefan Hajnoczi]

On Sun, Jun 30, 2013 at 08:49:28PM +0400, Michael Tokarev wrote:
> 30.06.2013 19:51, Peter Maydell wrote:
> > On 30 June 2013 16:36, Michael Tokarev <mjt@tls.msk.ru> wrote:
> >> I don't understand.  I for one like to see a plugins system used in qemu,
> >> and except of the build system everything else is easy (and even nice,
> >> there's even no need to load all plugins at startup as was initially
> >> suggested).  But for this to work, we really need to separate libs
> >> used only by plugins from the main lot, -- or else there's just no
> >> reason to build plugins in the first place.
> > 
> > So, why do we want to build things as plugins? Neither of the
> > cover letters to your two patch series nor Anthony's wiki page
> > on modules actually give the rationale.
> 
> I just updated the wiki page a bit (which is at
> http://wiki.qemu.org/Features/Modules btw).
> 
> The main rationale so far is to allow separate packaging of separate
> features for downstreams, to reduce the need to install all dependent
> libraries/packages.

This is more than just a small packaging clean-up.  It makes the
difference between shipping and not shipping niche features.

For features like Ceph, Sheepdog, etc the majority of downstream users
may be against the dependency.  So the QEMU package gets built without
these features!

Modules allow downstreams to cleanly package QEMU features and users can
decide which features/dependencies they wish to bring in.

For anyone jumping into the thread here: modules are not a stable ABI
and out-of-tree modules will not be supported in any way.

Stefan

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 30/06/2013 17:23, Michael Tokarev ha scritto:
>> Perhaps we can instead use something like
>> > 
>> > common.o: $(patsubst %,../%, $(common-obj-y))
>> > 	$(LD) -r -o $@ $^
>> > 
>> > and then link common.o into the QEMU target.  Libtool can also be used
>> > to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
>> > (we'd need it anyway for modules) as long as you do not need libtool to
>> > start QEMU or gdb from the build directory.
> I think this will mean that whole common.o will have to be linked
> into each executable which uses it, even if some executables are
> only using just one function from whole lot of .o files.

This would be a different common.o for each directory.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

01.07.2013 14:08, Paolo Bonzini wrote:
> Il 30/06/2013 17:23, Michael Tokarev ha scritto:
>>> Perhaps we can instead use something like
>>>>
>>>> common.o: $(patsubst %,../%, $(common-obj-y))
>>>> 	$(LD) -r -o $@ $^
>>>>
>>>> and then link common.o into the QEMU target.  Libtool can also be used
>>>> to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
>>>> (we'd need it anyway for modules) as long as you do not need libtool to
>>>> start QEMU or gdb from the build directory.
>> I think this will mean that whole common.o will have to be linked
>> into each executable which uses it, even if some executables are
>> only using just one function from whole lot of .o files.
> 
> This would be a different common.o for each directory.

Like util/common.o which requires -lrt and -lutil even if a given
app does not use openpty() or timer_create() ?

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 01/07/2013 12:10, Michael Tokarev ha scritto:
>>>> >>> Perhaps we can instead use something like
>>>>> >>>>
>>>>> >>>> common.o: $(patsubst %,../%, $(common-obj-y))
>>>>> >>>> 	$(LD) -r -o $@ $^
>>>>> >>>>
>>>>> >>>> and then link common.o into the QEMU target.  Libtool can also be used
>>>>> >>>> to abstract "ld -r".  Making libtool mandatory wouldn't be a problem IMO
>>>>> >>>> (we'd need it anyway for modules) as long as you do not need libtool to
>>>>> >>>> start QEMU or gdb from the build directory.
>>> >> I think this will mean that whole common.o will have to be linked
>>> >> into each executable which uses it, even if some executables are
>>> >> only using just one function from whole lot of .o files.
>> > 
>> > This would be a different common.o for each directory.
> Like util/common.o which requires -lrt and -lutil even if a given
> app does not use openpty() or timer_create() ?

No, for each target directory.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 30/06/2013 17:36, Michael Tokarev ha scritto:
> 30.06.2013 19:28, Andreas Färber wrote:
>> Am 18.06.2013 19:34, schrieb Michael Tokarev:
>>> The following working patchset demonstrates a one step to plugins system:
>>> it moves various dependent libraries and stuff out from libs_softmmu or
>>> libs_tools to object-specific variables.
>>
>> We did have a more elaborate Makefile variable system before, but Paolo
>> stashed most of that into common-obj-y and obj-y for simplicity.
> 
> I don't understand.  I for one like to see a plugins system used in qemu,
> and except of the build system everything else is easy (and even nice,
> there's even no need to load all plugins at startup as was initially
> suggested).  But for this to work, we really need to separate libs
> used only by plugins from the main lot, -- or else there's just no
> reason to build plugins in the first place.

Right.

What used to be there was something like

   net-obj-y = foo.o bar.o
   common-obj-y += $(addprefix net/, $(net-obj-y))

and this has been replaced by net/Makefile.objs and friends.

This should not be undone, but it is completely different from what you
are trying to do.  You need a much more fine-grained assignment of
libraries to object files.

I think the build system parts are hard because we haven't found the
right design.

> So, are you saying we should abandom this whole idea?  Or that maybe
> Paolo dislikes it (I think he expressed his interest here too)?

Absolutely not!

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

01.07.2013 17:39, Paolo Bonzini wrote:
> What used to be there was something like
> 
>    net-obj-y = foo.o bar.o
>    common-obj-y += $(addprefix net/, $(net-obj-y))
> 
> and this has been replaced by net/Makefile.objs and friends.

Maybe we should just list them all in a single makefile after all,
with proper paths

   common-obj-y += net/foo.o net/bar.o block/baz.o

and be done with it, without the need to put them in
different makefiles and without re-merging them back
in a non-trivial way during make invocation?  It isn't
much typing really, and it is rather obvious this way
and easy to deal with.

And for every program we build,

 $(PROGRAM)-obj = program.o $(foo-objs-y) $(bar-objs-y)

etc.

Why do we re-merge them back making things shorter to
write (without paths), non-obvious and half-working?
(by "half" i mean that some variables gets prefixed
while some arent).

Something similar for a (softmmu|user) target too, with
maybe even all target variations listed in a single
top-level makefile.

> This should not be undone, but it is completely different from what you
> are trying to do.  You need a much more fine-grained assignment of
> libraries to object files.
> 
> I think the build system parts are hard because we haven't found the
> right design.

What's wrong with a single makefile for everything, except
of the need to use full pathnames from the top-level source
dir?

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Andreas Färber]

Am 01.07.2013 16:43, schrieb Michael Tokarev:
> 01.07.2013 17:39, Paolo Bonzini wrote:
>> What used to be there was something like
>>
>>    net-obj-y = foo.o bar.o
>>    common-obj-y += $(addprefix net/, $(net-obj-y))
>>
>> and this has been replaced by net/Makefile.objs and friends.
> 
> Maybe we should just list them all in a single makefile after all,
> with proper paths
> 
>    common-obj-y += net/foo.o net/bar.o block/baz.o
> 
> and be done with it, without the need to put them in
> different makefiles and without re-merging them back
> in a non-trivial way during make invocation?  It isn't
> much typing really, and it is rather obvious this way
> and easy to deal with.
> 
> And for every program we build,
> 
>  $(PROGRAM)-obj = program.o $(foo-objs-y) $(bar-objs-y)
> 
> etc.
> 
> Why do we re-merge them back making things shorter to
> write (without paths), non-obvious and half-working?
> (by "half" i mean that some variables gets prefixed
> while some arent).
> 
> Something similar for a (softmmu|user) target too, with
> maybe even all target variations listed in a single
> top-level makefile.
> 
>> This should not be undone, but it is completely different from what you
>> are trying to do.  You need a much more fine-grained assignment of
>> libraries to object files.
>>
>> I think the build system parts are hard because we haven't found the
>> right design.
> 
> What's wrong with a single makefile for everything, except
> of the need to use full pathnames from the top-level source
> dir?

In short the problem we ran into was missing *.d inclusions.
This got fixed by automatically expanding net/ to -include
net/Makefile.objs and -include net/*.d or so.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

01.07.2013 18:46, Andreas Färber wrote:
[]
>> What's wrong with a single makefile for everything, except
>> of the need to use full pathnames from the top-level source
>> dir?
> 
> In short the problem we ran into was missing *.d inclusions.
> This got fixed by automatically expanding net/ to -include
> net/Makefile.objs and -include net/*.d or so.

-include $(pathsubst %.o=%.d,$(all-objs))

?  With a requiriment that all actually compiled objects be
listed in $(all-objs) somehow, either by adding them to one
of the common $(obj-y) or $(common-obj-m), or explicitly adding
them to $(all-objs) if it is used in some custom way.

Also, it can be done even funnier, something like this:

  subdirs = custom_subdir1 custom_subdir2
  subdirs := $(sort $(subdirs) $(dirname $(objs)))
  -include *.d $(pathsubst %=%/*.d,$(dirs))

or just list all subdirs explicitly in $(subdirs).
There's nothing fancy in there really.

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 01/07/2013 16:46, Andreas Färber ha scritto:
>> > 
>> > What's wrong with a single makefile for everything, except
>> > of the need to use full pathnames from the top-level source
>> > dir?
> In short the problem we ran into was missing *.d inclusions.
> This got fixed by automatically expanding net/ to -include
> net/Makefile.objs and -include net/*.d or so.

I don't think that's the problem.  Simply I don't think that listing
1000 object files in a single makefile are manageable.  Choosing the
right directory per-target is also much easier if you can just do

	obj-y += hw/$(TARGET_BASE_ARCH)/

instead of long if-elseif-elseif-elseif-endif conditionals.

Conflicts in a small file are also way easier to solve, even if there
are more conflicting files.

If you prefer to have _everything_ in a single file, you just have to
post patches and justify them.  I just doubt that the result will be
better than what we have today, and the time would be better invested in
cleaning up what we have today.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

01.07.2013 18:53, Paolo Bonzini wrote:
[]
> I don't think that's the problem.  Simply I don't think that listing
> 1000 object files in a single makefile are manageable.  Choosing the
> right directory per-target is also much easier if you can just do
> 
> 	obj-y += hw/$(TARGET_BASE_ARCH)/
> 
> instead of long if-elseif-elseif-elseif-endif conditionals.

it's not this.  It is something like:

 obj-$(TARGET_BASE_ARCH) += $($(target)-base-arch)

instead, where $(target)-base-arch still has to be defined
very much the same like it is defined today, with the exception
that it doesn't require to be listed in a separate Makefile.

Ie, today, we have, say, in hw/i386/Makefile.objs:
   obj-y += msi.o irq.o kvm.o
   obj-y += msi2.o kvm2.o

So instead of this, we may have in the top-level Makefile:

  obj-i386-y += hw/i386/msi.o hw/i386/irq.o hw/i386/kvm.o

Or, if you prefer programmatic expansion,

  list-i386-y += msi.o irq.o kvm.o
  obj-i386-y += $(addprefix hw/i386/, $(list-i386-y))

So the difference is just the addition of the pathnames,
not the logic or ifdeffery.

> Conflicts in a small file are also way easier to solve, even if there
> are more conflicting files.

Conflicts?  Which conflicts?  You mean merge conflicts?
If yes, it does not really matter be it small file or large
file.  If you list everything in one line, you're much more
likely to have a conflict, be it small file or large file.
If you always add stuff to the end of a file, you have much
more chance for a conflict, be it large or small file.
Single makefile may have sections so, say, you add a new
block device into a "block" section, maybe near the end of
it or alphabetically, so you're avioding conflicts the
same way as if you had multiple files.

> If you prefer to have _everything_ in a single file, you just have to
> post patches and justify them.  I just doubt that the result will be
> better than what we have today, and the time would be better invested in
> cleaning up what we have today.

Well, the more I look into it, the more I dislike what we have
now, because of this non-obvious half-magic half-working stuff.
To me we should either do it recursively in submakes, or just
add the paths (be it single makefile or multiple small makefiles)
without the current half-working half-magic.

(Again, the "half" here refers to the fact that some variables
gets prefixed by the subdir automatically while some doesn't).

So before sending patches, can we at least agree (or not) that
specifying paths explicitly (either using dir/obj.o or by calling
addprefix) is not THAT bad or it should be avoided entirely?

(I dislike the recursive sub-make approach because when you have
everything in one make it is much easier to see all dependencies
and plan the work, instead of running a ton of submakes first,
each checking if its own subdir is up to date).

Thanks,

/mjt

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Paolo Bonzini]

Il 01/07/2013 17:06, Michael Tokarev ha scritto:
> So instead of this, we may have in the top-level Makefile:
> 
>   obj-i386-y += hw/i386/msi.o hw/i386/irq.o hw/i386/kvm.o
> 
> Or, if you prefer programmatic expansion,
> 
>   list-i386-y += msi.o irq.o kvm.o
>   obj-i386-y += $(addprefix hw/i386/, $(list-i386-y))

If I have to choose my poison, I prefer the directory multiple times.
But both seem worse than what we have now?

> So the difference is just the addition of the pathnames,
> not the logic or ifdeffery.

Even after you factor in user-level vs. softmmu, tools, KVM vs. Xen vs.
TCG, and all that?

>> Conflicts in a small file are also way easier to solve, even if there
>> are more conflicting files.
> 
> Conflicts?  Which conflicts?  You mean merge conflicts?
> If yes, it does not really matter be it small file or large
> file.

When diff3 goes haywire because you're merging features back to a
4-year-old version, it matters a lot.

> (Again, the "half" here refers to the fact that some variables
> gets prefixed by the subdir automatically while some doesn't).

Which *-obj-y variables do not get prefixed?

> So before sending patches, can we at least agree (or not) that
> specifying paths explicitly (either using dir/obj.o or by calling
> addprefix) is not THAT bad or it should be avoided entirely?

addprefix is bad.  Specifying paths explicitly is not bad _per se_, but
if a directory is used to group related code, I think the Makefile
should also be separated.  For example in the future we may have Kconfig
files in hw/*, and of these three possibilities, (1) and (2) would be
worse than (3):

1) use a single huge Kconfig file for all devices, use a single
Makefile.objs;

2) split Kconfig, keep a single Makefile.objs file;

3) split both the Kconfig and the Makefile.objs files.

> (I dislike the recursive sub-make approach because when you have
> everything in one make it is much easier to see all dependencies
> and plan the work, instead of running a ton of submakes first,
> each checking if its own subdir is up to date).

I agree.  And again QEMU is doing half-this half-that, but I don't see
any alternative.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

[Michael Tokarev]

01.07.2013 19:20, Paolo Bonzini wrote:
> Il 01/07/2013 17:06, Michael Tokarev ha scritto:
>> So instead of this, we may have in the top-level Makefile:
>>
>>   obj-i386-y += hw/i386/msi.o hw/i386/irq.o hw/i386/kvm.o
>>
>> Or, if you prefer programmatic expansion,
>>
>>   list-i386-y += msi.o irq.o kvm.o
>>   obj-i386-y += $(addprefix hw/i386/, $(list-i386-y))
> 
> If I have to choose my poison, I prefer the directory multiple times.

Actually I too - it is just more obvious this way, and the
typing isn't _that_ numerous.

> But both seem worse than what we have now?

It is.  Because of the half-magic-half-not which I mentioned
multiple times.

Currently we don't have lots of per-object (or per-module)
variables, it is the opposite: they're rare:

  $(obj)/foo.o: CFLAGS += -DFOO

When we have more of these (in one or another form - for extra
libs, for additional #defines like that), the makefiles becomes
much less clean:

  obj-y += foo.o
  $(obj)/foo.o: CFLAGS += -DFOO

Why for obj-y you write plain foo.o, while for CFLAGS you use
$(obj)/foo.o ?

This is exactly my half-magic-half-nonworking thing I'm referring
to.  The more individual variables like this you have, the more
non-obvious it becomes - where to use $(obj) and where not to.

So at the end, I think, it is better to just use it everywhere
(and at the same time just get rid of it entirely, and just specify
directory prefix directly), than to have this mix of magic and
explicity.

>> So the difference is just the addition of the pathnames,
>> not the logic or ifdeffery.
> 
> Even after you factor in user-level vs. softmmu, tools, KVM vs. Xen vs.
> TCG, and all that?

I think yes.  I don't _yet_ know it as a whole, but so far it sounds
doable and actually quite clean.  But only if done _carefully_.

>>> Conflicts in a small file are also way easier to solve, even if there
>>> are more conflicting files.
>>
>> Conflicts?  Which conflicts?  You mean merge conflicts?
>> If yes, it does not really matter be it small file or large
>> file.
> 
> When diff3 goes haywire because you're merging features back to a
> 4-year-old version, it matters a lot.

You can't merge a 4-year-old version regardless, because during
last 4 years lots of stuff has been rewritten, and some (like
the build system) even multiple times.

>> (Again, the "half" here refers to the fact that some variables
>> gets prefixed by the subdir automatically while some doesn't).
> 
> Which *-obj-y variables do not get prefixed?

For example, per-object variables like foo.cflags (or cflags-foo-y).
Actually anything except of a few "magic" variables.

It is not obvious to me why are you asking -- you know exactly what
gets prefixed and what not.

>> So before sending patches, can we at least agree (or not) that
>> specifying paths explicitly (either using dir/obj.o or by calling
>> addprefix) is not THAT bad or it should be avoided entirely?
> 
> addprefix is bad.  Specifying paths explicitly is not bad _per se_, but
> if a directory is used to group related code, I think the Makefile
> should also be separated.  For example in the future we may have Kconfig
> files in hw/*, and of these three possibilities, (1) and (2) would be
> worse than (3):
> 
> 1) use a single huge Kconfig file for all devices, use a single
> Makefile.objs;
> 
> 2) split Kconfig, keep a single Makefile.objs file;
> 
> 3) split both the Kconfig and the Makefile.objs files.

There's nothing that stops you from using multiple makefile fragments.
Nothing at all.  What I'm saying is to get rid of the half-magic
only, and just use plain include subdir/Makefile.objs (or whatever),
nothing more nothing less.

With all targets made this way -- something like this:

targets-$(CONFIG_TARGET_I386) += i386-softmmu/qemu-system-softmmu

i386-softmmu/qemu-system-softmmu: $(i386-softmmu-obj-y)
(or ...: $(addprefix i386-softmmu/,$(i386-softmmu-obj-y)))
...

-- be it in the single main makefile or in a submake, it doesn't
really matter at all at this point! -- but just without the magic,
it becomes rather interesting and strightforward.

Thanks,

/mjt

>> (I dislike the recursive sub-make approach because when you have
>> everything in one make it is much easier to see all dependencies
>> and plan the work, instead of running a ton of submakes first,
>> each checking if its own subdir is up to date).
> 
> I agree.  And again QEMU is doing half-this half-that, but I don't see
> any alternative.
> 
> Paolo
>