Christian Ehrhardt <[email protected]> writes:

> On Fri, Jan 25, 2019 at 5:53 PM Aaron Conole <[email protected]> wrote:
>>
>> Luca Boccassi <[email protected]> writes:
>>
>> > On Fri, 2019-01-25 at 16:39 +0000, Luca Boccassi wrote:
>> >> On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote:
>> >> > Christian Ehrhardt <[email protected]> writes:
>> >> >
>> >> > > DPDK 18.11 builds using the more modern meson build system no
>> >> > > more
>> >> > > provide the -ldpdk linker script. Instead it is expected to use
>> >> > > pkgconfig for linker options as well.
>> >> > >
>> >> > > This change will set DPDK_LIB from pkg-config (if pkg-config was
>> >> > > available) and since that already carries the whole-archive flags
>> >> > > around
>> >> > > the PMDs skips the further wrapping in more whole-archive if that
>> >> > > is
>> >> > > already part of DPDK_LIB.
>> >> > >
>> >> > > Signed-off-by: Christian Ehrhardt <[email protected]
>> >> > > om
>> >> > > >
>> >> > >
>> >> > > ---
>> >> > >  acinclude.m4 | 17 ++++++++++++-----
>> >> > >  1 file changed, 12 insertions(+), 5 deletions(-)
>> >> > >
>> >> > > diff --git a/acinclude.m4 b/acinclude.m4
>> >> > > index f038fd457..a45411860 100644
>> >> > > --- a/acinclude.m4
>> >> > > +++ b/acinclude.m4
>> >> > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>> >> > >      case "$with_dpdk" in
>> >> > >        yes)
>> >> > >          DPDK_AUTO_DISCOVER="true"
>> >> > > -        PKG_CHECK_MODULES([DPDK], [libdpdk],
>> >> > > -                          [DPDK_INCLUDE="$DPDK_CFLAGS"],
>> >> > > -                          [DPDK_INCLUDE="-
>> >> > > I/usr/local/include/dpdk
>> >> > > -I/usr/include/dpdk"])
>> >> > > +        PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk],
>> >> > > +                                 [DPDK_INCLUDE="$DPDK_CFLAGS",
>> >> > > DPDK_LIB="$DPDK_LIBS"],
>> >> > > +
>> >> > > [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk",
>> >> > > DPDK_LIB="-ldpdk"])
>> >> >
>> >> > Something about this autoconf check caused all of the travis builds
>> >> > to
>> >> > fail:
>> >> >
>> >> >   https://travis-ci.org/ovsrobot/ovs/builds/483986081
>> >> >
>> >> > The PKG_CHECK_MODULES_STATIC seems to have been added in a later
>> >> > version
>> >> > of autoconf than is shipped in the travis environment, so it might
>> >> > be
>> >> > something we need to test against and provide an alternative
>> >> > implementation, ala the following bug report:
>> >> >
>> >> >   https://bugs.freedesktop.org/show_bug.cgi?id=19541
>> >>
>> >> Hi,
>> >>
>> >> The PKG_CHECK* macros come from the pkg-config package, not autoconf,
>> >> in version 0.29. The version of pkg-config in Ubuntu 14.04 does not
>> >> indeed contain it, but 16.04 does.
>> >>
>> >> If you go down the reimplementation route, I would recommend checking
>> >> for the macro version first and add a comment, so that you can then
>> >> drop it when compatibility with Ubuntu 14.04 and Debian 8 is no
>> >> longer
>> >> required.
>> >
>> > Also, these macro files are all versioned - autoconf is smart enough to
>> > check the version if you have it locally, and if the system's is
>> > higher, it will use that one instead, so there's no drawback to
>> > backporting.
>>
>> Cool, I didn't know that!
>>
>> > In other words, you can drop pkg.m4 version 0.29 in the m4/ directory
>> > in the repository, add "m4_include([m4/pkg.m4])" to configure.ac and
>> > then not worry about it anymore until compatibility with Ubuntu 14.04
>> > and Debian 8 can be dropped.
>>
>> That sounds like a solution.
>
> Ben/Aaron, thanks for sorting out that aspect of it!
>
> That said, does it mean that the Travis failures now can be considered 
> passing?

Not quite yet.

> And if so what does it mean for the patch itself, do you consider it ok and
> would merge it or are there other challenges that we need to tackle?

I think if you made the adjustments suggested by Luca (copy pkg.m4 into
the m4/ directory in the ovs tree, add the m4_include([m4/pkg.m4])
directive to configure.ac) and then post a v2, it would be acceptable.

>> >> > Thinking about it, is there a reason to change from dynamic linking
>> >> > to
>> >> > static linking?  Especially since we can achieve something similar
>> >> > via
>> >> > the `PKG_CONFIG="$PKG_CONFIG --static"` override?
>> >>
>> >> Note that this is not doing static linking - it's just getting the
>> >> full
>> >> list of libs (which includes all the PMDs). The _STATIC macro is
>> >> implemented exactly as you suggested above.
>> >>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to