[v3,5/7] build/pkg-config: output driver libs first for static build
diff mbox series

Message ID 20200630141433.818517-6-bruce.richardson@intel.com
State Accepted
Delegated to: Thomas Monjalon
Headers show
Series
  • improve DPDK static builds with meson
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson June 30, 2020, 2:14 p.m. UTC
When calling pkg-config --static --libs, pkg-config will always output the
regular libs first, and then the extra libs from libraries.private field,
since the assumption is that those are additional dependencies for building
statically that the .a files depend upon.

However, for DPDK, we only link the driver files for static builds, and
those need to come *before* the regular libraries. To get this result, we
need two pkgconfig files for DPDK, one for the shared libs, and a second
for the static libs and drivers, which depends upon the first. Using a
dependency means that the shared libs are printed only after the
libraries.private field rather than before.

Without this patch, the linking works in DPDK because in all cases we
specify the libraries after the drivers in the Libs.private line, ensuring
that the references to the libs from the drivers can be resolved. The
current output is therefore of the form, "(shared)libs, drivers,
(static)libs", while after this patch the output is, "drivers,
(static)libs, (shared)libs". The former case will not work if we use the
--whole-archive flag on the static libs as it will lead to duplicate
definitions due to some references having been previously resolved from the
shared libraries. By ensuring the shared libraries come last in the link
link, this issue does not occur, as duplicate references when linking the
shared libs will be ignored.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Luca Boccassi <bluca@debian.org>
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
---
 buildtools/pkg-config/meson.build | 33 +++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

Comments

Thomas Monjalon July 1, 2020, 7:50 a.m. UTC | #1
30/06/2020 16:14, Bruce Richardson:
> When calling pkg-config --static --libs, pkg-config will always output the
> regular libs first, and then the extra libs from libraries.private field,
> since the assumption is that those are additional dependencies for building
> statically that the .a files depend upon.
> 
> However, for DPDK, we only link the driver files for static builds, and

Sorry, I'm lost here. Why "only" driver files?

> those need to come *before* the regular libraries.

Given whole libs are linked, is it really needed to have drivers first?

> To get this result, we
> need two pkgconfig files for DPDK, one for the shared libs, and a second
> for the static libs and drivers, which depends upon the first. Using a
> dependency means that the shared libs are printed only after the
> libraries.private field rather than before.
> 
> Without this patch, the linking works in DPDK because in all cases we
> specify the libraries after the drivers in the Libs.private line, ensuring
> that the references to the libs from the drivers can be resolved. The
> current output is therefore of the form, "(shared)libs, drivers,
> (static)libs", while after this patch the output is, "drivers,
> (static)libs, (shared)libs". The former case will not work if we use the
> --whole-archive flag on the static libs as it will lead to duplicate
> definitions due to some references having been previously resolved from the
> shared libraries.

I'm completely lost. In which case we link both static and shared libs?

> By ensuring the shared libraries come last in the link
> link, this issue does not occur, as duplicate references when linking the
> shared libs will be ignored.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Luca Boccassi <bluca@debian.org>
> Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
Bruce Richardson July 1, 2020, 8:43 a.m. UTC | #2
On Wed, Jul 01, 2020 at 09:50:20AM +0200, Thomas Monjalon wrote:
> 30/06/2020 16:14, Bruce Richardson:
> > When calling pkg-config --static --libs, pkg-config will always output the
> > regular libs first, and then the extra libs from libraries.private field,
> > since the assumption is that those are additional dependencies for building
> > statically that the .a files depend upon.
> > 
> > However, for DPDK, we only link the driver files for static builds, and
> 
> Sorry, I'm lost here. Why "only" driver files?

My fault for poor word ordering. Should be rewritten as "link the driver
files only for static builds".

> 
> > those need to come *before* the regular libraries.
> 
> Given whole libs are linked, is it really needed to have drivers first?

If we are considering only static libs, then I suspect not, but to avoid
having the shared libraries also linked into the static binaries they
definitely need to come last, so they can be linked with as-needed i.e.
skipped by the linker.

> 
> > To get this result, we
> > need two pkgconfig files for DPDK, one for the shared libs, and a second
> > for the static libs and drivers, which depends upon the first. Using a
> > dependency means that the shared libs are printed only after the
> > libraries.private field rather than before.
> > 
> > Without this patch, the linking works in DPDK because in all cases we
> > specify the libraries after the drivers in the Libs.private line, ensuring
> > that the references to the libs from the drivers can be resolved. The
> > current output is therefore of the form, "(shared)libs, drivers,
> > (static)libs", while after this patch the output is, "drivers,
> > (static)libs, (shared)libs". The former case will not work if we use the
> > --whole-archive flag on the static libs as it will lead to duplicate
> > definitions due to some references having been previously resolved from the
> > shared libraries.
> 
> I'm completely lost. In which case we link both static and shared libs?
> 
It will depend partially upon other linker flags used before the pkgconfig
insert. We would be relying upon the user integrating this to explicitly
pass in -Bstatic flag before putting the pkgconfig flags output. With the
current config, we can't add that ourselves because to go first it would
have to go (ironically) in the shared lib part of the pkg-config file. By
splitting things into two, it allows the static libs to come before the
shared ones, which gives us the ability to control things far better.

> > By ensuring the shared libraries come last in the link
> > link, this issue does not occur, as duplicate references when linking the
> > shared libs will be ignored.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Luca Boccassi <bluca@debian.org>
> > Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
> 
> 
>
Thomas Monjalon July 1, 2020, 2:42 p.m. UTC | #3
30/06/2020 16:14, Bruce Richardson:
> When calling pkg-config --static --libs, pkg-config will always output the
> regular libs first, and then the extra libs from libraries.private field,

s/libraries.private/Libs.private/

> since the assumption is that those are additional dependencies for building
> statically that the .a files depend upon.
> 
> However, for DPDK, we only link the driver files for static builds, and
> those need to come *before* the regular libraries. To get this result, we
> need two pkgconfig files for DPDK, one for the shared libs, and a second
> for the static libs and drivers, which depends upon the first. Using a
> dependency means that the shared libs are printed only after the
> libraries.private field rather than before.

s/libraries.private/Libs.private/

> Without this patch, the linking works in DPDK because in all cases we
> specify the libraries after the drivers in the Libs.private line, ensuring
> that the references to the libs from the drivers can be resolved. The
> current output is therefore of the form, "(shared)libs, drivers,
> (static)libs", while after this patch the output is, "drivers,
> (static)libs, (shared)libs". The former case will not work if we use the
> --whole-archive flag on the static libs as it will lead to duplicate
> definitions due to some references having been previously resolved from the
> shared libraries. By ensuring the shared libraries come last in the link
> link, this issue does not occur, as duplicate references when linking the
> shared libs will be ignored.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Luca Boccassi <bluca@debian.org>
> Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
> ---
> +# When calling pkg-config --static --libs, pkg-config will always output the
> +# regular libs first, and then the extra libs from libraries.private field,
> +# since the assumption is that those are additional dependencies for building
> +# statically that the .a files depend upon. However, for DPDK, we only link
> +# the driver files for static builds, and those need to come *before* the
> +# regular libraries. To get this result, we need two pkgconfig files for DPDK,
> +# one for the shared libs, and a second for the static libs and drivers, which
> +# depends upon the first. Using a dependency means that the shared libs are
> +# printed only after the libraries.private field rather than before.

This is not obvious. In order to avoid messing it up in future,
I suggest this longer reword:

# When calling pkg-config --static --libs, pkg-config will always output the
# regular libs first, and then the extra libs from Libs.private field,
# since the assumption is that those are additional dependencies for building
# statically that the .a files depend upon. The output order of .pc fields is:
#   Cflags   Libs   Libs.private   Requires   Requires.private
# The fields Requires* are for package names.
# The flags of the DPDK libraries must be defined in Libs* fields.
# However, the DPDK drivers are linked only in static builds (Libs.private),
# and those need to come *before* the regular libraries (Libs field).
# This requirement is satisfied by moving the regular libs in a separate file
# included in the field Requires (after Libs.private).
# Another requirement is to allow linking dependencies as shared libraries,
# while linking static DPDK libraries and drivers. It is satisfied by
# listing the static files in Libs.private with the explicit syntax -l:libfoo.a.
# As a consequence, the regular DPDK libraries are already listed as static
# in the field Libs.private. The second occurences of DPDK libraries,
# included from Requires and used for shared library linkage case,
# are skipped in the case of static linkage thanks to the flag --as-needed.

# Link order summary:
#   libdpdk.Libs.private: whole-archive(static drivers/libs), drivers deps flags
#   libdpdk.Requires: libdpdk-libs package
#   libdpdk-libs.Libs: as-needed(shared libs)
#   libdpdk-libs.Libs.private: libs deps flags
#   libdpdk.pc.Requires.private: deps packages


If you agree, I could change this comment while merging.
I would add my Signed-off ;)
Bruce Richardson July 1, 2020, 3:16 p.m. UTC | #4
On Wed, Jul 01, 2020 at 04:42:33PM +0200, Thomas Monjalon wrote:
> 30/06/2020 16:14, Bruce Richardson:
> > When calling pkg-config --static --libs, pkg-config will always output the
> > regular libs first, and then the extra libs from libraries.private field,
> 
> s/libraries.private/Libs.private/
> 
> > since the assumption is that those are additional dependencies for building
> > statically that the .a files depend upon.
> > 
> > However, for DPDK, we only link the driver files for static builds, and
> > those need to come *before* the regular libraries. To get this result, we
> > need two pkgconfig files for DPDK, one for the shared libs, and a second
> > for the static libs and drivers, which depends upon the first. Using a
> > dependency means that the shared libs are printed only after the
> > libraries.private field rather than before.
> 
> s/libraries.private/Libs.private/
> 
> > Without this patch, the linking works in DPDK because in all cases we
> > specify the libraries after the drivers in the Libs.private line, ensuring
> > that the references to the libs from the drivers can be resolved. The
> > current output is therefore of the form, "(shared)libs, drivers,
> > (static)libs", while after this patch the output is, "drivers,
> > (static)libs, (shared)libs". The former case will not work if we use the
> > --whole-archive flag on the static libs as it will lead to duplicate
> > definitions due to some references having been previously resolved from the
> > shared libraries. By ensuring the shared libraries come last in the link
> > link, this issue does not occur, as duplicate references when linking the
> > shared libs will be ignored.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Luca Boccassi <bluca@debian.org>
> > Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
> > ---
> > +# When calling pkg-config --static --libs, pkg-config will always output the
> > +# regular libs first, and then the extra libs from libraries.private field,
> > +# since the assumption is that those are additional dependencies for building
> > +# statically that the .a files depend upon. However, for DPDK, we only link
> > +# the driver files for static builds, and those need to come *before* the
> > +# regular libraries. To get this result, we need two pkgconfig files for DPDK,
> > +# one for the shared libs, and a second for the static libs and drivers, which
> > +# depends upon the first. Using a dependency means that the shared libs are
> > +# printed only after the libraries.private field rather than before.
> 
> This is not obvious. In order to avoid messing it up in future,
> I suggest this longer reword:
> 
> # When calling pkg-config --static --libs, pkg-config will always output the
> # regular libs first, and then the extra libs from Libs.private field,
> # since the assumption is that those are additional dependencies for building
> # statically that the .a files depend upon. The output order of .pc fields is:
> #   Cflags   Libs   Libs.private   Requires   Requires.private
> # The fields Requires* are for package names.
> # The flags of the DPDK libraries must be defined in Libs* fields.
> # However, the DPDK drivers are linked only in static builds (Libs.private),
> # and those need to come *before* the regular libraries (Libs field).
> # This requirement is satisfied by moving the regular libs in a separate file
> # included in the field Requires (after Libs.private).
> # Another requirement is to allow linking dependencies as shared libraries,
> # while linking static DPDK libraries and drivers. It is satisfied by
> # listing the static files in Libs.private with the explicit syntax -l:libfoo.a.
> # As a consequence, the regular DPDK libraries are already listed as static
> # in the field Libs.private. The second occurences of DPDK libraries,
> # included from Requires and used for shared library linkage case,
> # are skipped in the case of static linkage thanks to the flag --as-needed.
> 
> # Link order summary:
> #   libdpdk.Libs.private: whole-archive(static drivers/libs), drivers deps flags
> #   libdpdk.Requires: libdpdk-libs package
> #   libdpdk-libs.Libs: as-needed(shared libs)
> #   libdpdk-libs.Libs.private: libs deps flags
> #   libdpdk.pc.Requires.private: deps packages
> 
> 
> If you agree, I could change this comment while merging.
> I would add my Signed-off ;)
>

This seems generally ok, but probably should just be added as part of patch
#7 when all parts of the above have been applied.

Couple of comments:
* One small nit is that cflags are not output as part of the --libs call, so
you can remove them from the list on line 5 of the comment. They aren't
really relevant to this comment/essay.

* I find the link-order summary to actually be more confusing than helpful.
I think the text block is explanatory enough. It just confuses things
introducing the extra details of what goes in the requires-private or
libs-private of the libdpdk.pc file. That's just regular stuff, unrelated
to the changes or to DPDK special-case of needing private libs first.

/Bruce
Thomas Monjalon July 1, 2020, 3:36 p.m. UTC | #5
01/07/2020 17:16, Bruce Richardson:
> On Wed, Jul 01, 2020 at 04:42:33PM +0200, Thomas Monjalon wrote:
> > 30/06/2020 16:14, Bruce Richardson:
> > > When calling pkg-config --static --libs, pkg-config will always output the
> > > regular libs first, and then the extra libs from libraries.private field,
> > 
> > s/libraries.private/Libs.private/
> > 
> > > since the assumption is that those are additional dependencies for building
> > > statically that the .a files depend upon.
> > > 
> > > However, for DPDK, we only link the driver files for static builds, and
> > > those need to come *before* the regular libraries. To get this result, we
> > > need two pkgconfig files for DPDK, one for the shared libs, and a second
> > > for the static libs and drivers, which depends upon the first. Using a
> > > dependency means that the shared libs are printed only after the
> > > libraries.private field rather than before.
> > 
> > s/libraries.private/Libs.private/
> > 
> > > Without this patch, the linking works in DPDK because in all cases we
> > > specify the libraries after the drivers in the Libs.private line, ensuring
> > > that the references to the libs from the drivers can be resolved. The
> > > current output is therefore of the form, "(shared)libs, drivers,
> > > (static)libs", while after this patch the output is, "drivers,
> > > (static)libs, (shared)libs". The former case will not work if we use the
> > > --whole-archive flag on the static libs as it will lead to duplicate
> > > definitions due to some references having been previously resolved from the
> > > shared libraries. By ensuring the shared libraries come last in the link
> > > link, this issue does not occur, as duplicate references when linking the
> > > shared libs will be ignored.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Acked-by: Luca Boccassi <bluca@debian.org>
> > > Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
> > > ---
> > > +# When calling pkg-config --static --libs, pkg-config will always output the
> > > +# regular libs first, and then the extra libs from libraries.private field,
> > > +# since the assumption is that those are additional dependencies for building
> > > +# statically that the .a files depend upon. However, for DPDK, we only link
> > > +# the driver files for static builds, and those need to come *before* the
> > > +# regular libraries. To get this result, we need two pkgconfig files for DPDK,
> > > +# one for the shared libs, and a second for the static libs and drivers, which
> > > +# depends upon the first. Using a dependency means that the shared libs are
> > > +# printed only after the libraries.private field rather than before.
> > 
> > This is not obvious. In order to avoid messing it up in future,
> > I suggest this longer reword:
> > 
> > # When calling pkg-config --static --libs, pkg-config will always output the
> > # regular libs first, and then the extra libs from Libs.private field,
> > # since the assumption is that those are additional dependencies for building
> > # statically that the .a files depend upon. The output order of .pc fields is:
> > #   Cflags   Libs   Libs.private   Requires   Requires.private
> > # The fields Requires* are for package names.
> > # The flags of the DPDK libraries must be defined in Libs* fields.
> > # However, the DPDK drivers are linked only in static builds (Libs.private),
> > # and those need to come *before* the regular libraries (Libs field).
> > # This requirement is satisfied by moving the regular libs in a separate file
> > # included in the field Requires (after Libs.private).
> > # Another requirement is to allow linking dependencies as shared libraries,
> > # while linking static DPDK libraries and drivers. It is satisfied by
> > # listing the static files in Libs.private with the explicit syntax -l:libfoo.a.
> > # As a consequence, the regular DPDK libraries are already listed as static
> > # in the field Libs.private. The second occurences of DPDK libraries,
> > # included from Requires and used for shared library linkage case,
> > # are skipped in the case of static linkage thanks to the flag --as-needed.
> > 
> > # Link order summary:
> > #   libdpdk.Libs.private: whole-archive(static drivers/libs), drivers deps flags
> > #   libdpdk.Requires: libdpdk-libs package
> > #   libdpdk-libs.Libs: as-needed(shared libs)
> > #   libdpdk-libs.Libs.private: libs deps flags
> > #   libdpdk.pc.Requires.private: deps packages
> > 
> > 
> > If you agree, I could change this comment while merging.
> > I would add my Signed-off ;)
> >
> 
> This seems generally ok, but probably should just be added as part of patch
> #7 when all parts of the above have been applied.
> 
> Couple of comments:
> * One small nit is that cflags are not output as part of the --libs call, so
> you can remove them from the list on line 5 of the comment. They aren't
> really relevant to this comment/essay.

Yes, I will remove Cflags.

> * I find the link-order summary to actually be more confusing than helpful.
> I think the text block is explanatory enough. It just confuses things
> introducing the extra details of what goes in the requires-private or
> libs-private of the libdpdk.pc file. That's just regular stuff, unrelated
> to the changes or to DPDK special-case of needing private libs first.

The intent of this summary was to help navigating
for future changes in this area.
Personnaly it helps me, but it is maybe more a developer note
that can be deduced from the rest.
I can remove it.
Bruce Richardson July 1, 2020, 3:45 p.m. UTC | #6
On Wed, Jul 01, 2020 at 05:36:07PM +0200, Thomas Monjalon wrote:
> 01/07/2020 17:16, Bruce Richardson:
> > On Wed, Jul 01, 2020 at 04:42:33PM +0200, Thomas Monjalon wrote:
> > > 30/06/2020 16:14, Bruce Richardson:
> > > > When calling pkg-config --static --libs, pkg-config will always output the
> > > > regular libs first, and then the extra libs from libraries.private field,
> > > 
> > > s/libraries.private/Libs.private/
> > > 
> > > > since the assumption is that those are additional dependencies for building
> > > > statically that the .a files depend upon.
> > > > 
> > > > However, for DPDK, we only link the driver files for static builds, and
> > > > those need to come *before* the regular libraries. To get this result, we
> > > > need two pkgconfig files for DPDK, one for the shared libs, and a second
> > > > for the static libs and drivers, which depends upon the first. Using a
> > > > dependency means that the shared libs are printed only after the
> > > > libraries.private field rather than before.
> > > 
> > > s/libraries.private/Libs.private/
> > > 
> > > > Without this patch, the linking works in DPDK because in all cases we
> > > > specify the libraries after the drivers in the Libs.private line, ensuring
> > > > that the references to the libs from the drivers can be resolved. The
> > > > current output is therefore of the form, "(shared)libs, drivers,
> > > > (static)libs", while after this patch the output is, "drivers,
> > > > (static)libs, (shared)libs". The former case will not work if we use the
> > > > --whole-archive flag on the static libs as it will lead to duplicate
> > > > definitions due to some references having been previously resolved from the
> > > > shared libraries. By ensuring the shared libraries come last in the link
> > > > link, this issue does not occur, as duplicate references when linking the
> > > > shared libs will be ignored.
> > > > 
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > Acked-by: Luca Boccassi <bluca@debian.org>
> > > > Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
> > > > ---
> > > > +# When calling pkg-config --static --libs, pkg-config will always output the
> > > > +# regular libs first, and then the extra libs from libraries.private field,
> > > > +# since the assumption is that those are additional dependencies for building
> > > > +# statically that the .a files depend upon. However, for DPDK, we only link
> > > > +# the driver files for static builds, and those need to come *before* the
> > > > +# regular libraries. To get this result, we need two pkgconfig files for DPDK,
> > > > +# one for the shared libs, and a second for the static libs and drivers, which
> > > > +# depends upon the first. Using a dependency means that the shared libs are
> > > > +# printed only after the libraries.private field rather than before.
> > > 
> > > This is not obvious. In order to avoid messing it up in future,
> > > I suggest this longer reword:
> > > 
> > > # When calling pkg-config --static --libs, pkg-config will always output the
> > > # regular libs first, and then the extra libs from Libs.private field,
> > > # since the assumption is that those are additional dependencies for building
> > > # statically that the .a files depend upon. The output order of .pc fields is:
> > > #   Cflags   Libs   Libs.private   Requires   Requires.private
> > > # The fields Requires* are for package names.
> > > # The flags of the DPDK libraries must be defined in Libs* fields.
> > > # However, the DPDK drivers are linked only in static builds (Libs.private),
> > > # and those need to come *before* the regular libraries (Libs field).
> > > # This requirement is satisfied by moving the regular libs in a separate file
> > > # included in the field Requires (after Libs.private).
> > > # Another requirement is to allow linking dependencies as shared libraries,
> > > # while linking static DPDK libraries and drivers. It is satisfied by
> > > # listing the static files in Libs.private with the explicit syntax -l:libfoo.a.
> > > # As a consequence, the regular DPDK libraries are already listed as static
> > > # in the field Libs.private. The second occurences of DPDK libraries,
> > > # included from Requires and used for shared library linkage case,
> > > # are skipped in the case of static linkage thanks to the flag --as-needed.
> > > 
> > > # Link order summary:
> > > #   libdpdk.Libs.private: whole-archive(static drivers/libs), drivers deps flags
> > > #   libdpdk.Requires: libdpdk-libs package
> > > #   libdpdk-libs.Libs: as-needed(shared libs)
> > > #   libdpdk-libs.Libs.private: libs deps flags
> > > #   libdpdk.pc.Requires.private: deps packages
> > > 
> > > 
> > > If you agree, I could change this comment while merging.
> > > I would add my Signed-off ;)
> > >
> > 
> > This seems generally ok, but probably should just be added as part of patch
> > #7 when all parts of the above have been applied.
> > 
> > Couple of comments:
> > * One small nit is that cflags are not output as part of the --libs call, so
> > you can remove them from the list on line 5 of the comment. They aren't
> > really relevant to this comment/essay.
> 
> Yes, I will remove Cflags.
> 
> > * I find the link-order summary to actually be more confusing than helpful.
> > I think the text block is explanatory enough. It just confuses things
> > introducing the extra details of what goes in the requires-private or
> > libs-private of the libdpdk.pc file. That's just regular stuff, unrelated
> > to the changes or to DPDK special-case of needing private libs first.
> 
> The intent of this summary was to help navigating
> for future changes in this area.
> Personnaly it helps me, but it is maybe more a developer note
> that can be deduced from the rest.
> I can remove it.
> 
Ok thanks. If you are happy to make these comment changes on apply, please
feel free to do so, thanks.

[BTW: I think the shebang line on the new python script needs a "python"
changed to "python3" also. I missed that in the latest revision, sorry!]
Thomas Monjalon July 1, 2020, 4:04 p.m. UTC | #7
01/07/2020 17:45, Bruce Richardson:
> On Wed, Jul 01, 2020 at 05:36:07PM +0200, Thomas Monjalon wrote:
> > 01/07/2020 17:16, Bruce Richardson:
> > > On Wed, Jul 01, 2020 at 04:42:33PM +0200, Thomas Monjalon wrote:
> > > > 30/06/2020 16:14, Bruce Richardson:
> > > > > When calling pkg-config --static --libs, pkg-config will always output the
> > > > > regular libs first, and then the extra libs from libraries.private field,
> > > > 
> > > > s/libraries.private/Libs.private/
> > > > 
> > > > > since the assumption is that those are additional dependencies for building
> > > > > statically that the .a files depend upon.
> > > > > 
> > > > > However, for DPDK, we only link the driver files for static builds, and
> > > > > those need to come *before* the regular libraries. To get this result, we
> > > > > need two pkgconfig files for DPDK, one for the shared libs, and a second
> > > > > for the static libs and drivers, which depends upon the first. Using a
> > > > > dependency means that the shared libs are printed only after the
> > > > > libraries.private field rather than before.
> > > > 
> > > > s/libraries.private/Libs.private/
> > > > 
> > > > > Without this patch, the linking works in DPDK because in all cases we
> > > > > specify the libraries after the drivers in the Libs.private line, ensuring
> > > > > that the references to the libs from the drivers can be resolved. The
> > > > > current output is therefore of the form, "(shared)libs, drivers,
> > > > > (static)libs", while after this patch the output is, "drivers,
> > > > > (static)libs, (shared)libs". The former case will not work if we use the
> > > > > --whole-archive flag on the static libs as it will lead to duplicate
> > > > > definitions due to some references having been previously resolved from the
> > > > > shared libraries. By ensuring the shared libraries come last in the link
> > > > > link, this issue does not occur, as duplicate references when linking the
> > > > > shared libs will be ignored.
> > > > > 
> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Acked-by: Luca Boccassi <bluca@debian.org>
> > > > > Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
> > > > > ---
> > > > > +# When calling pkg-config --static --libs, pkg-config will always output the
> > > > > +# regular libs first, and then the extra libs from libraries.private field,
> > > > > +# since the assumption is that those are additional dependencies for building
> > > > > +# statically that the .a files depend upon. However, for DPDK, we only link
> > > > > +# the driver files for static builds, and those need to come *before* the
> > > > > +# regular libraries. To get this result, we need two pkgconfig files for DPDK,
> > > > > +# one for the shared libs, and a second for the static libs and drivers, which
> > > > > +# depends upon the first. Using a dependency means that the shared libs are
> > > > > +# printed only after the libraries.private field rather than before.
> > > > 
> > > > This is not obvious. In order to avoid messing it up in future,
> > > > I suggest this longer reword:
> > > > 
> > > > # When calling pkg-config --static --libs, pkg-config will always output the
> > > > # regular libs first, and then the extra libs from Libs.private field,
> > > > # since the assumption is that those are additional dependencies for building
> > > > # statically that the .a files depend upon. The output order of .pc fields is:
> > > > #   Cflags   Libs   Libs.private   Requires   Requires.private
> > > > # The fields Requires* are for package names.
> > > > # The flags of the DPDK libraries must be defined in Libs* fields.
> > > > # However, the DPDK drivers are linked only in static builds (Libs.private),
> > > > # and those need to come *before* the regular libraries (Libs field).
> > > > # This requirement is satisfied by moving the regular libs in a separate file
> > > > # included in the field Requires (after Libs.private).
> > > > # Another requirement is to allow linking dependencies as shared libraries,
> > > > # while linking static DPDK libraries and drivers. It is satisfied by
> > > > # listing the static files in Libs.private with the explicit syntax -l:libfoo.a.
> > > > # As a consequence, the regular DPDK libraries are already listed as static
> > > > # in the field Libs.private. The second occurences of DPDK libraries,
> > > > # included from Requires and used for shared library linkage case,
> > > > # are skipped in the case of static linkage thanks to the flag --as-needed.
> > > > 
> > > > # Link order summary:
> > > > #   libdpdk.Libs.private: whole-archive(static drivers/libs), drivers deps flags
> > > > #   libdpdk.Requires: libdpdk-libs package
> > > > #   libdpdk-libs.Libs: as-needed(shared libs)
> > > > #   libdpdk-libs.Libs.private: libs deps flags
> > > > #   libdpdk.pc.Requires.private: deps packages
> > > > 
> > > > 
> > > > If you agree, I could change this comment while merging.
> > > > I would add my Signed-off ;)
> > > >
> > > 
> > > This seems generally ok, but probably should just be added as part of patch
> > > #7 when all parts of the above have been applied.
> > > 
> > > Couple of comments:
> > > * One small nit is that cflags are not output as part of the --libs call, so
> > > you can remove them from the list on line 5 of the comment. They aren't
> > > really relevant to this comment/essay.
> > 
> > Yes, I will remove Cflags.
> > 
> > > * I find the link-order summary to actually be more confusing than helpful.
> > > I think the text block is explanatory enough. It just confuses things
> > > introducing the extra details of what goes in the requires-private or
> > > libs-private of the libdpdk.pc file. That's just regular stuff, unrelated
> > > to the changes or to DPDK special-case of needing private libs first.
> > 
> > The intent of this summary was to help navigating
> > for future changes in this area.
> > Personnaly it helps me, but it is maybe more a developer note
> > that can be deduced from the rest.
> > I can remove it.
> > 
> Ok thanks. If you are happy to make these comment changes on apply, please
> feel free to do so, thanks.
> 
> [BTW: I think the shebang line on the new python script needs a "python"
> changed to "python3" also. I missed that in the latest revision, sorry!]

OK will do.

Patch
diff mbox series

diff --git a/buildtools/pkg-config/meson.build b/buildtools/pkg-config/meson.build
index 85d59972d..aabd00eed 100644
--- a/buildtools/pkg-config/meson.build
+++ b/buildtools/pkg-config/meson.build
@@ -10,17 +10,34 @@  pkg_extra_cflags = ['-include', 'rte_config.h'] + machine_args
 if is_freebsd
 	pkg_extra_cflags += ['-D__BSD_VISIBLE']
 endif
-pkg.generate(name: meson.project_name(),
-	filebase: 'lib' + meson.project_name().to_lower(),
+
+# When calling pkg-config --static --libs, pkg-config will always output the
+# regular libs first, and then the extra libs from libraries.private field,
+# since the assumption is that those are additional dependencies for building
+# statically that the .a files depend upon. However, for DPDK, we only link
+# the driver files for static builds, and those need to come *before* the
+# regular libraries. To get this result, we need two pkgconfig files for DPDK,
+# one for the shared libs, and a second for the static libs and drivers, which
+# depends upon the first. Using a dependency means that the shared libs are
+# printed only after the libraries.private field rather than before.
+pkg.generate(name: 'dpdk-libs',
+	filebase: 'libdpdk-libs',
+	description: '''Internal-only DPDK pkgconfig file. Not for direct use.
+Use libdpdk.pc instead of this file to query DPDK compile/link arguments''',
 	version: meson.project_version(),
+	subdirs: [get_option('include_subdir_arch'), '.'],
+	extra_cflags: pkg_extra_cflags,
 	libraries: dpdk_libraries,
-	libraries_private: dpdk_drivers + dpdk_static_libraries +
-			['-Wl,-Bdynamic'] + dpdk_extra_ldflags,
-	requires: libbsd, # apps using rte_string_fns.h may need this if enabled
-	                  # if libbsd is not enabled, then this is blank
+	libraries_private: dpdk_extra_ldflags)
+
+pkg.generate(name: 'DPDK', # main DPDK pkgconfig file
+	filebase: 'libdpdk',
+	version: meson.project_version(),
 	description: '''The Data Plane Development Kit (DPDK).
 Note that CFLAGS might contain an -march flag higher than typical baseline.
 This is required for a number of static inline functions in the public headers.''',
-	subdirs: [get_option('include_subdir_arch'), '.'],
-	extra_cflags: pkg_extra_cflags
+	requires: ['libdpdk-libs', libbsd], # may need libbsd for string funcs
+	                  # if libbsd is not enabled, then this is blank
+	libraries_private: dpdk_drivers + dpdk_static_libraries +
+			['-Wl,-Bdynamic']
 )