qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries
Date: Wed, 19 Jun 2013 18:58:29 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6

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(-)
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]