qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
@ 2020-06-16 16:18 Christophe de Dinechin
  2020-06-23 14:44 ` Stefan Hajnoczi
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe de Dinechin @ 2020-06-16 16:18 UTC (permalink / raw)
  To: qemu-devel

In util/Makefile.objs, there is a setting for dbus.o-libs.
Trying to copy-paste that to add a library for module.o that was was
not otherwise linked yields link errors on a number of binaries,
e.g. qemu-ga, with unsatisfied symbols in libqemuutil.a(module.o).
The reason is that library dependencies are not propagated to the .a
files automatically.

Adding a call to extract-libs to get the libraries for the two .a
files that are used elsewhere.

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 2e93068894..5fb0c78a0b 100644
--- a/Makefile
+++ b/Makefile
@@ -598,6 +598,8 @@ Makefile: $(version-obj-y)
 ######################################################################
 # Build libraries
 
+libqemuutil.a-libs += $(call extract-libs, $(util-obj-y) $(trace-obj-y) $(stub-obj-y))
+libvhost-user.a-libs += $(call extract-libs, $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y))
 libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(stub-obj-y)
 libvhost-user.a: $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y)
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
  2020-06-16 16:18 [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a Christophe de Dinechin
@ 2020-06-23 14:44 ` Stefan Hajnoczi
  2020-07-01  9:12   ` Christophe de Dinechin
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2020-06-23 14:44 UTC (permalink / raw)
  To: Christophe de Dinechin; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

On Tue, Jun 16, 2020 at 06:18:14PM +0200, Christophe de Dinechin wrote:
> In util/Makefile.objs, there is a setting for dbus.o-libs.
> Trying to copy-paste that to add a library for module.o that was was
> not otherwise linked yields link errors on a number of binaries,
> e.g. qemu-ga, with unsatisfied symbols in libqemuutil.a(module.o).
> The reason is that library dependencies are not propagated to the .a
> files automatically.
> 
> Adding a call to extract-libs to get the libraries for the two .a
> files that are used elsewhere.
> 
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 2e93068894..5fb0c78a0b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -598,6 +598,8 @@ Makefile: $(version-obj-y)
>  ######################################################################
>  # Build libraries
>  
> +libqemuutil.a-libs += $(call extract-libs, $(util-obj-y) $(trace-obj-y) $(stub-obj-y))
> +libvhost-user.a-libs += $(call extract-libs, $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y))
>  libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(stub-obj-y)
>  libvhost-user.a: $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y)

I wonder if there is GNU Make syntax to avoid duplicating the
dependencies of libqemuutil.a and libvhost-user.a?

Another thing I wonder about: the purpose of the .a files is to compile
all object files and only link those .o files needed by the program
(i.e. a subset of the .a file).

Now that libqemuutil.a-libs is defined, do programs using libqemuutil.a
link libs required by .o files that aren't being used?

For example, say we had util/mp3encoder.o which depends on an MP3
encoder library. A utility like qemu-img does not depend on mp3encoder.o
from libqemuutil.a. Therefore -lmp3enc or whatever shouldn't be on
qemu-img's linker command-line.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
  2020-06-23 14:44 ` Stefan Hajnoczi
@ 2020-07-01  9:12   ` Christophe de Dinechin
  2020-07-02 13:06     ` Michael Tokarev
  0 siblings, 1 reply; 5+ messages in thread
From: Christophe de Dinechin @ 2020-07-01  9:12 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel


On 2020-06-23 at 16:44 CEST, Stefan Hajnoczi wrote...
> On Tue, Jun 16, 2020 at 06:18:14PM +0200, Christophe de Dinechin wrote:
>> In util/Makefile.objs, there is a setting for dbus.o-libs.
>> Trying to copy-paste that to add a library for module.o that was was
>> not otherwise linked yields link errors on a number of binaries,
>> e.g. qemu-ga, with unsatisfied symbols in libqemuutil.a(module.o).
>> The reason is that library dependencies are not propagated to the .a
>> files automatically.
>>
>> Adding a call to extract-libs to get the libraries for the two .a
>> files that are used elsewhere.
>>
>> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
>> ---
>>  Makefile | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index 2e93068894..5fb0c78a0b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -598,6 +598,8 @@ Makefile: $(version-obj-y)
>>  ######################################################################
>>  # Build libraries
>>
>> +libqemuutil.a-libs += $(call extract-libs, $(util-obj-y) $(trace-obj-y) $(stub-obj-y))
>> +libvhost-user.a-libs += $(call extract-libs, $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y))
>>  libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(stub-obj-y)
>>  libvhost-user.a: $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y)
>
> I wonder if there is GNU Make syntax to avoid duplicating the
> dependencies of libqemuutil.a and libvhost-user.a?

The dependencies are not identical. Would you want two lines, one with the
shared dependencies, i.e. something like:

  libqemuutils.a libhost-user.a: $(util-obj-y) $(stub-obj-y)
  libqemuutils.a: $(trace-obj-y)
  libvhost-user.a: $(libvhost-user-obj-y)

Or do you mean something else?

>
> Another thing I wonder about: the purpose of the .a files is to compile
> all object files and only link those .o files needed by the program
> (i.e. a subset of the .a file).

I believe that what you are saying is that by passing the required libraries
automatically, the binaries that use libqemuutil.a will inherit undesired
ldd dependencies. Indeed, a quick experiment shows that if you pass a -l
option, the library dependency is recorded even if no symbol in that library
is used. I saw no obvious linker option to address that.

Maybe add a comment, then, because otherwise it is surprising to have an
unsat despite something like:

  foo.o-libs = -lbar

As I pointed out in my commit comment, there is at least one case where this
is used, dbus.o-libs, suggesting that this is expected to work. This seems
to be the only case that goes in util-obj-y today. Apparently, this works
because the link of qemu-system-x86_64 already gets $(GIO_LIBS) from
some .o file that is not going through libqemuutil.a

>
> Now that libqemuutil.a-libs is defined, do programs using libqemuutil.a
> link libs required by .o files that aren't being used?

Yes. The alternative being that you get unsatisfied symbols if you do use
the .o file.

>
> For example, say we had util/mp3encoder.o which depends on an MP3
> encoder library. A utility like qemu-img does not depend on mp3encoder.o
> from libqemuutil.a. Therefore -lmp3enc or whatever shouldn't be on
> qemu-img's linker command-line.

If that's the intent, then util/mp3encoder.o should ideally not be lumped
into the library. Otherwise, you will get unsatisfied symbols when you use
it through the library, but not when you use it directly.

One can certainly argue that it's better to have an explicit "unsatisfied
symbol" error than a silent addition of an unwanted library dependency. If
that's the consensus, then, just add a comment explaining how this works.

Given that discussion, I'm now inclined to believe that the correct solution
is:

a) add a comment in the makefile explaining that .o-libs flags are not
   passed through .a files with a pointer to this discussion.

b) remove the existing dbus.o-libs line which has no effect, to avoid
   monkey-do copy-paste inheritance

--
Cheers,
Christophe de Dinechin (IRC c3d)



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
  2020-07-01  9:12   ` Christophe de Dinechin
@ 2020-07-02 13:06     ` Michael Tokarev
  2020-07-28 10:14       ` Stefan Hajnoczi
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Tokarev @ 2020-07-02 13:06 UTC (permalink / raw)
  To: Christophe de Dinechin, Stefan Hajnoczi; +Cc: qemu-devel

01.07.2020 12:12, Christophe de Dinechin wrote:
..
>>>  # Build libraries
>>>
>>> +libqemuutil.a-libs += $(call extract-libs, $(util-obj-y) $(trace-obj-y) $(stub-obj-y))
>>> +libvhost-user.a-libs += $(call extract-libs, $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y))
..
>> Another thing I wonder about: the purpose of the .a files is to compile
>> all object files and only link those .o files needed by the program
>> (i.e. a subset of the .a file).
> 
> I believe that what you are saying is that by passing the required libraries
> automatically, the binaries that use libqemuutil.a will inherit undesired
> ldd dependencies. Indeed, a quick experiment shows that if you pass a -l
> option, the library dependency is recorded even if no symbol in that library
> is used. I saw no obvious linker option to address that.

There's --as-needed and --no-as-needed ld flag (used with cc as -Wl,--as-needed),
which is designed for this very case.

JFYI.

/mjt


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
  2020-07-02 13:06     ` Michael Tokarev
@ 2020-07-28 10:14       ` Stefan Hajnoczi
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2020-07-28 10:14 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: Christophe de Dinechin, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

On Thu, Jul 02, 2020 at 04:06:32PM +0300, Michael Tokarev wrote:
> 01.07.2020 12:12, Christophe de Dinechin wrote:
> ..
> >>>  # Build libraries
> >>>
> >>> +libqemuutil.a-libs += $(call extract-libs, $(util-obj-y) $(trace-obj-y) $(stub-obj-y))
> >>> +libvhost-user.a-libs += $(call extract-libs, $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y))
> ..
> >> Another thing I wonder about: the purpose of the .a files is to compile
> >> all object files and only link those .o files needed by the program
> >> (i.e. a subset of the .a file).
> > 
> > I believe that what you are saying is that by passing the required libraries
> > automatically, the binaries that use libqemuutil.a will inherit undesired
> > ldd dependencies. Indeed, a quick experiment shows that if you pass a -l
> > option, the library dependency is recorded even if no symbol in that library
> > is used. I saw no obvious linker option to address that.
> 
> There's --as-needed and --no-as-needed ld flag (used with cc as -Wl,--as-needed),
> which is designed for this very case.

Nice, sounds like a solution. So -Wl,--as-needed should be at the end of
the command-line followed by $(libqemuutil.a-libs).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-28 10:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 16:18 [PATCH] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a Christophe de Dinechin
2020-06-23 14:44 ` Stefan Hajnoczi
2020-07-01  9:12   ` Christophe de Dinechin
2020-07-02 13:06     ` Michael Tokarev
2020-07-28 10:14       ` Stefan Hajnoczi

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