diff mbox

[1/2] package/lldpd: disable use of libbsd

Message ID 1471171604-16162-1-git-send-email-yann.morin.1998@free.fr
State Changes Requested
Headers show

Commit Message

Yann E. MORIN Aug. 14, 2016, 10:46 a.m. UTC
libbsd installs .pc files that contain -isystem /usr/include/bsd, but
-isystem is not munged by pkg-config, so we end up using /usr/include/bsd
which are not suitable for the target. And breaks big time (see
failures, below).

But using libbsd is completely optional. In fact, our lldpd.mk did not
even express the dependency on libbsd, so we may well have had builds
without libbsd.

The functionality brought in by using libbsd is very minor, so we just
make lldpd not use libbsd at all. There is no --disable-libbsd or such,
so we just patch it out of configure.ac.

Fixes:
    http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
    http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
    http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
    [...]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 .../0002-configure-do-not-check-for-libbsd.patch   | 62 ++++++++++++++++++++++
 package/lldpd/lldpd.mk                             |  2 +-
 2 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 package/lldpd/0002-configure-do-not-check-for-libbsd.patch

Comments

Arnout Vandecappelle Aug. 14, 2016, 8:26 p.m. UTC | #1
On 14-08-16 12:46, Yann E. MORIN wrote:
> libbsd installs .pc files that contain -isystem /usr/include/bsd, but

 Actually, only libbsd-overlay.pc does that.

> -isystem is not munged by pkg-config, so we end up using /usr/include/bsd

 I'm not sure what pkg-config does, I can only say that it's not munged by pkgconf.

> which are not suitable for the target. And breaks big time (see
> failures, below).
> 
> But using libbsd is completely optional. In fact, our lldpd.mk did not
> even express the dependency on libbsd, so we may well have had builds
> without libbsd.
> 
> The functionality brought in by using libbsd is very minor, so we just

 It would be nice to mention _what_ functionality it brings.

> make lldpd not use libbsd at all. There is no --disable-libbsd or such,
> so we just patch it out of configure.ac.
> 
> Fixes:
>     http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
>     http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
>     http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
>     [...]
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

 Note that this fix leaves any other users of libbsd out to dry. However, we
only have netcat-openbsd and libedit at the moment that declare a dependency on
libbsd, and these don't use libbsd-overlay, so it seems to be OK.


> ---
>  .../0002-configure-do-not-check-for-libbsd.patch   | 62 ++++++++++++++++++++++
>  package/lldpd/lldpd.mk                             |  2 +-
>  2 files changed, 63 insertions(+), 1 deletion(-)
>  create mode 100644 package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> 
> diff --git a/package/lldpd/0002-configure-do-not-check-for-libbsd.patch b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> new file mode 100644
> index 0000000..b89b442
> --- /dev/null
> +++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> @@ -0,0 +1,62 @@
> +From 0801a066cd4a24a858ddfa7c62c7802e0f5533a8 Mon Sep 17 00:00:00 2001
> +From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> +Date: Sun, 14 Aug 2016 12:15:17 +0200
> +Subject: [PATCH] configure: do not check for libbsd
> +
> +libbsd causes build issues because it .pc file is borked: it contains

 libbsd -> libbsd-overlay
 it -> its

> +-isystem in CFLAGS, which is not munged by pkg-config, so we end up
> +using the headers of the build machine, causing all sorts of hard to
> +debug trouble at build time.
> +
> +lldpd uses libbsd to a few helper functions, but has fallback in case

 to -> for
 fallback -> fallbacks

> +libbsd is not available. The only feature that is lost when not using
> +libbsd is that the neighbour name is no longer displayed in the
> +/proc/self/cmdline. As the author of lldpd said on IRC: "people should
> +survive! ;-)"
> +
> +So we just remove the detection of libbsd altogether.
> +
> +Fixes:
> +    http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
> +    http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
> +    http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
> +    [...]
> +
> +Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> +---
> + configure.ac | 19 -------------------
> + 1 file changed, 19 deletions(-)
> +
> +diff --git a/configure.ac b/configure.ac
> +index dd723b0..45498ce 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -165,25 +165,6 @@ AC_FUNC_MALLOC
> + AC_FUNC_REALLOC
> + AC_FUNC_FORK
> + 
> +-# Some functions can be in libbsd
> +-PKG_CHECK_MODULES([libbsd], [libbsd-overlay], [
> +-  _save_CFLAGS="$CFLAGS"
> +-  _save_LIBS="$LIBS"
> +-  CFLAGS="$CFLAGS $libbsd_CFLAGS"
> +-  LIBS="$LIBS $libbsd_LIBS"
> +-  AC_MSG_CHECKING([if libbsd can be linked correctly])
> +-  AC_LINK_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],
> +-    [
> +-      AC_MSG_RESULT(yes)
> +-      LLDP_CFLAGS="$LLDP_CFLAGS $libbsd_CFLAGS"
> +-      LLDP_LDFLAGS="$LLDP_LDFLAGS $libbsd_LIBS"
> +-    ],[
> +-      AC_MSG_RESULT(no)
> +-      CFLAGS="$_save_CFLAGS"
> +-      LIBS="$_save_LIBS"
> +-    ])
> +-], [:])
> +-
> + # setproctitle may have an _init function
> + AC_REPLACE_FUNCS([setproctitle])
> + AC_CHECK_FUNCS([setproctitle_init])
> +-- 
> +2.7.4
> +
> diff --git a/package/lldpd/lldpd.mk b/package/lldpd/lldpd.mk
> index 924f2db..008d4bf 100644
> --- a/package/lldpd/lldpd.mk
> +++ b/package/lldpd/lldpd.mk
> @@ -9,7 +9,7 @@ LLDPD_SITE = http://media.luffy.cx/files/lldpd
>  LLDPD_DEPENDENCIES = host-pkgconf libevent
>  LLDPD_LICENSE = ISC
>  LLDPD_LICENSE_FILES = README.md
> -# 0001-build-make-generation-of-atom-glue-compatible-with-o.patch
> +# Touching configure.ac and Makefile.am in our patches:

 I actually find it quite convenient if the comment states which patch(es) touch
the autotools files, but ATM we don't do that very often, so OK.

 Only spelling mistakes so

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

>  LLDPD_AUTORECONF = YES
>  
>  # Detection of c99 support in configure fails without WCHAR. To enable
>
Yann E. MORIN Aug. 14, 2016, 9:49 p.m. UTC | #2
Arnout, All,

On 2016-08-14 22:26 +0200, Arnout Vandecappelle spake thusly:
> On 14-08-16 12:46, Yann E. MORIN wrote:
> > libbsd installs .pc files that contain -isystem /usr/include/bsd, but
>  Actually, only libbsd-overlay.pc does that.

Right.

> > -isystem is not munged by pkg-config, so we end up using /usr/include/bsd
>  I'm not sure what pkg-config does, I can only say that it's not munged by pkgconf.

Well, that's what I said, no?

> > which are not suitable for the target. And breaks big time (see
> > failures, below).
> > 
> > But using libbsd is completely optional. In fact, our lldpd.mk did not
> > even express the dependency on libbsd, so we may well have had builds
> > without libbsd.
> > 
> > The functionality brought in by using libbsd is very minor, so we just
>  It would be nice to mention _what_ functionality it brings.

It's described in the patch.

> > make lldpd not use libbsd at all. There is no --disable-libbsd or such,
> > so we just patch it out of configure.ac.
> > 
> > Fixes:
> >     http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
> >     http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
> >     http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
> >     [...]
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> 
>  Note that this fix leaves any other users of libbsd out to dry.

And we have no issue with those...

> However, we
> only have netcat-openbsd and libedit at the moment that declare a dependency on
> libbsd, and these don't use libbsd-overlay, so it seems to be OK.

Yes.

However, I have no idea how to fix libbsd-overlay... -isystem is weird,
because it changes the ordering of include paths that gcc searches for.
We've had issue with it recently, so I don't really know what to do.

Better handle the issues when they arise. If we ever add another package
that uses libbsd-overlay, _then_ we can think of a proper fix.

But for now, the release is approaching fast enough that I think it is
better to just fix that one issue, espeocially since we don't lose much.

> > ---
> >  .../0002-configure-do-not-check-for-libbsd.patch   | 62 ++++++++++++++++++++++
> >  package/lldpd/lldpd.mk                             |  2 +-
> >  2 files changed, 63 insertions(+), 1 deletion(-)
> >  create mode 100644 package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> > 
> > diff --git a/package/lldpd/0002-configure-do-not-check-for-libbsd.patch b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> > new file mode 100644
> > index 0000000..b89b442
> > --- /dev/null
> > +++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> > @@ -0,0 +1,62 @@
> > +From 0801a066cd4a24a858ddfa7c62c7802e0f5533a8 Mon Sep 17 00:00:00 2001
> > +From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > +Date: Sun, 14 Aug 2016 12:15:17 +0200
> > +Subject: [PATCH] configure: do not check for libbsd
> > +
> > +libbsd causes build issues because it .pc file is borked: it contains
>  libbsd -> libbsd-overlay

Yes, I was speaking about the package, not the .pc file. But I can
change...

[--SNIP--]
> > diff --git a/package/lldpd/lldpd.mk b/package/lldpd/lldpd.mk
> > index 924f2db..008d4bf 100644
> > --- a/package/lldpd/lldpd.mk
> > +++ b/package/lldpd/lldpd.mk
> > @@ -9,7 +9,7 @@ LLDPD_SITE = http://media.luffy.cx/files/lldpd
> >  LLDPD_DEPENDENCIES = host-pkgconf libevent
> >  LLDPD_LICENSE = ISC
> >  LLDPD_LICENSE_FILES = README.md
> > -# 0001-build-make-generation-of-atom-glue-compatible-with-o.patch
> > +# Touching configure.ac and Makefile.am in our patches:
> 
>  I actually find it quite convenient if the comment states which patch(es) touch
> the autotools files, but ATM we don't do that very often, so OK.

Yes, can do.

>  Only spelling mistakes so
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Thanks! :-)

Regards,
Yann E. MORIN.
Vivien Didelot Aug. 15, 2016, 3:05 p.m. UTC | #3
Hi Yann,

"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

> libbsd installs .pc files that contain -isystem /usr/include/bsd, but
> -isystem is not munged by pkg-config, so we end up using /usr/include/bsd
> which are not suitable for the target. And breaks big time (see
> failures, below).
>
> But using libbsd is completely optional. In fact, our lldpd.mk did not
> even express the dependency on libbsd, so we may well have had builds
> without libbsd.
>
> The functionality brought in by using libbsd is very minor, so we just
> make lldpd not use libbsd at all. There is no --disable-libbsd or such,
> so we just patch it out of configure.ac.
>
> Fixes:
>     http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
>     http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
>     http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
>     [...]
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  .../0002-configure-do-not-check-for-libbsd.patch   | 62 ++++++++++++++++++++++
>  package/lldpd/lldpd.mk                             |  2 +-
>  2 files changed, 63 insertions(+), 1 deletion(-)
>  create mode 100644 package/lldpd/0002-configure-do-not-check-for-libbsd.patch
>
> diff --git a/package/lldpd/0002-configure-do-not-check-for-libbsd.patch b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> new file mode 100644
> index 0000000..b89b442
> --- /dev/null
> +++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> @@ -0,0 +1,62 @@
> +From 0801a066cd4a24a858ddfa7c62c7802e0f5533a8 Mon Sep 17 00:00:00 2001
> +From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> +Date: Sun, 14 Aug 2016 12:15:17 +0200
> +Subject: [PATCH] configure: do not check for libbsd
> +
> +libbsd causes build issues because it .pc file is borked: it contains
> +-isystem in CFLAGS, which is not munged by pkg-config, so we end up
> +using the headers of the build machine, causing all sorts of hard to
> +debug trouble at build time.
> +
> +lldpd uses libbsd to a few helper functions, but has fallback in case
> +libbsd is not available. The only feature that is lost when not using
> +libbsd is that the neighbour name is no longer displayed in the
> +/proc/self/cmdline. As the author of lldpd said on IRC: "people should
> +survive! ;-)"
> +
> +So we just remove the detection of libbsd altogether.
> +
> +Fixes:
> +    http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
> +    http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
> +    http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
> +    [...]
> +
> +Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> +---
> + configure.ac | 19 -------------------
> + 1 file changed, 19 deletions(-)
> +
> +diff --git a/configure.ac b/configure.ac
> +index dd723b0..45498ce 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -165,25 +165,6 @@ AC_FUNC_MALLOC
> + AC_FUNC_REALLOC
> + AC_FUNC_FORK
> + 
> +-# Some functions can be in libbsd
> +-PKG_CHECK_MODULES([libbsd], [libbsd-overlay], [
> +-  _save_CFLAGS="$CFLAGS"
> +-  _save_LIBS="$LIBS"
> +-  CFLAGS="$CFLAGS $libbsd_CFLAGS"
> +-  LIBS="$LIBS $libbsd_LIBS"
> +-  AC_MSG_CHECKING([if libbsd can be linked correctly])
> +-  AC_LINK_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],
> +-    [
> +-      AC_MSG_RESULT(yes)
> +-      LLDP_CFLAGS="$LLDP_CFLAGS $libbsd_CFLAGS"
> +-      LLDP_LDFLAGS="$LLDP_LDFLAGS $libbsd_LIBS"
> +-    ],[
> +-      AC_MSG_RESULT(no)
> +-      CFLAGS="$_save_CFLAGS"
> +-      LIBS="$_save_LIBS"
> +-    ])
> +-], [:])
> +-
> + # setproctitle may have an _init function
> + AC_REPLACE_FUNCS([setproctitle])
> + AC_CHECK_FUNCS([setproctitle_init])
> +-- 
> +2.7.4
> +

Did you send this patch upstream to lldpd?

I've seen that Vincent pushed something already for that -isystem
issue. No risk to conflict?

https://github.com/vincentbernat/lldpd/commit/11c1b6a2ffb3f66a54a43413cb894844bcb38f14

Adding Vincent in the loop.

Thanks,

        Vivien
Yann E. MORIN Aug. 15, 2016, 3:23 p.m. UTC | #4
Viven, All,

On 2016-08-15 11:05 -0400, Vivien Didelot spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> writes:
[--SNIP--]
> > diff --git a/package/lldpd/0002-configure-do-not-check-for-libbsd.patch b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> > new file mode 100644
> > index 0000000..b89b442
> > --- /dev/null
> > +++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
> > @@ -0,0 +1,62 @@
> > +From 0801a066cd4a24a858ddfa7c62c7802e0f5533a8 Mon Sep 17 00:00:00 2001
> > +From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > +Date: Sun, 14 Aug 2016 12:15:17 +0200
> > +Subject: [PATCH] configure: do not check for libbsd
> > +
> > +libbsd causes build issues because it .pc file is borked: it contains
> > +-isystem in CFLAGS, which is not munged by pkg-config, so we end up
> > +using the headers of the build machine, causing all sorts of hard to
> > +debug trouble at build time.
> > +
> > +lldpd uses libbsd to a few helper functions, but has fallback in case
> > +libbsd is not available. The only feature that is lost when not using
> > +libbsd is that the neighbour name is no longer displayed in the
> > +/proc/self/cmdline. As the author of lldpd said on IRC: "people should
> > +survive! ;-)"
> > +
> > +So we just remove the detection of libbsd altogether.
> > +
> > +Fixes:
> > +    http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
> > +    http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
> > +    http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
> > +    [...]
> > +
> > +Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > +---
> > + configure.ac | 19 -------------------
> > + 1 file changed, 19 deletions(-)
> > +
> > +diff --git a/configure.ac b/configure.ac
> > +index dd723b0..45498ce 100644
> > +--- a/configure.ac
> > ++++ b/configure.ac
> > +@@ -165,25 +165,6 @@ AC_FUNC_MALLOC
> > + AC_FUNC_REALLOC
> > + AC_FUNC_FORK
> > + 
> > +-# Some functions can be in libbsd
> > +-PKG_CHECK_MODULES([libbsd], [libbsd-overlay], [
> > +-  _save_CFLAGS="$CFLAGS"
> > +-  _save_LIBS="$LIBS"
> > +-  CFLAGS="$CFLAGS $libbsd_CFLAGS"
> > +-  LIBS="$LIBS $libbsd_LIBS"
> > +-  AC_MSG_CHECKING([if libbsd can be linked correctly])
> > +-  AC_LINK_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],
> > +-    [
> > +-      AC_MSG_RESULT(yes)
> > +-      LLDP_CFLAGS="$LLDP_CFLAGS $libbsd_CFLAGS"
> > +-      LLDP_LDFLAGS="$LLDP_LDFLAGS $libbsd_LIBS"
> > +-    ],[
> > +-      AC_MSG_RESULT(no)
> > +-      CFLAGS="$_save_CFLAGS"
> > +-      LIBS="$_save_LIBS"
> > +-    ])
> > +-], [:])
> > +-
> > + # setproctitle may have an _init function
> > + AC_REPLACE_FUNCS([setproctitle])
> > + AC_CHECK_FUNCS([setproctitle_init])
> > +-- 
> > +2.7.4
> > +
> 
> Did you send this patch upstream to lldpd?

No. I don't think this patch is correct for upstream, see below.

> I've seen that Vincent pushed something already for that -isystem
> issue.

Yes, we discussed it on IRC with Vincent the other day. I'm still unsure
of the implications (I haven't tested it, in fact), but for Buildroot,
we need a quick patch for the next release. The bove patch is just that:
a woarkaround for us to have a stable release.

We can work after the release to update lldpd with a newer upstream
release or an upstream patch, though.

> No risk to conflict?

Oh, probably, yes, since both touch the exact same code! ;-)

Regards,
Yann E. MORIN.
Arnout Vandecappelle Aug. 15, 2016, 9:54 p.m. UTC | #5
On 14-08-16 23:49, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2016-08-14 22:26 +0200, Arnout Vandecappelle spake thusly:
>> On 14-08-16 12:46, Yann E. MORIN wrote:
>>> libbsd installs .pc files that contain -isystem /usr/include/bsd, but
>>  Actually, only libbsd-overlay.pc does that.
> 
> Right.
> 
>>> -isystem is not munged by pkg-config, so we end up using /usr/include/bsd
>>  I'm not sure what pkg-config does, I can only say that it's not munged by pkgconf.
> 
> Well, that's what I said, no?

 What I mean is: we use pkgconf instead of pkg-config. It's possible that the
full pkg-config does the correct thing with -isystem (i.e., prepending it with
the cross-sysroot). The only thing we know for sure is that pkgconf doesn't.

>>> which are not suitable for the target. And breaks big time (see
>>> failures, below).
>>>
>>> But using libbsd is completely optional. In fact, our lldpd.mk did not
>>> even express the dependency on libbsd, so we may well have had builds
>>> without libbsd.
>>>
>>> The functionality brought in by using libbsd is very minor, so we just
>>  It would be nice to mention _what_ functionality it brings.
> 
> It's described in the patch.

 Sorry I forgot to delete that comment when I came to the patch description.

> 
>>> make lldpd not use libbsd at all. There is no --disable-libbsd or such,
>>> so we just patch it out of configure.ac.
>>>
>>> Fixes:
>>>     http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
>>>     http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
>>>     http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
>>>     [...]
>>>
>>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>>
>>  Note that this fix leaves any other users of libbsd out to dry.
> 
> And we have no issue with those...
> 
>> However, we
>> only have netcat-openbsd and libedit at the moment that declare a dependency on
>> libbsd, and these don't use libbsd-overlay, so it seems to be OK.
> 
> Yes.
> 
> However, I have no idea how to fix libbsd-overlay... -isystem is weird,
> because it changes the ordering of include paths that gcc searches for.

 -isystem is not really weird, it just inserts the directory in a different
place in the search path. Oh yes, it's a little bit weird, because it overrides
a preceding -I with the same path. However, what pkgconf needs to do with it is
pretty simple: it needs to add the sysroot path to it, just like for -I.

 Now, I've taken a look at the source of pkgconf, and it's not that trivial to
implement, since ATM it really only considers single-letter flags.

> We've had issue with it recently, so I don't really know what to do.
> 
> Better handle the issues when they arise. If we ever add another package
> that uses libbsd-overlay, _then_ we can think of a proper fix.
> 
> But for now, the release is approaching fast enough that I think it is
> better to just fix that one issue, espeocially since we don't lose much.

 Fair enough.


 Regards,
 Arnout

> 
>>> ---
>>>  .../0002-configure-do-not-check-for-libbsd.patch   | 62 ++++++++++++++++++++++
>>>  package/lldpd/lldpd.mk                             |  2 +-
>>>  2 files changed, 63 insertions(+), 1 deletion(-)
>>>  create mode 100644 package/lldpd/0002-configure-do-not-check-for-libbsd.patch
>>>
>>> diff --git a/package/lldpd/0002-configure-do-not-check-for-libbsd.patch b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
>>> new file mode 100644
>>> index 0000000..b89b442
>>> --- /dev/null
>>> +++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
>>> @@ -0,0 +1,62 @@
>>> +From 0801a066cd4a24a858ddfa7c62c7802e0f5533a8 Mon Sep 17 00:00:00 2001
>>> +From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> +Date: Sun, 14 Aug 2016 12:15:17 +0200
>>> +Subject: [PATCH] configure: do not check for libbsd
>>> +
>>> +libbsd causes build issues because it .pc file is borked: it contains
>>  libbsd -> libbsd-overlay
> 
> Yes, I was speaking about the package, not the .pc file. But I can
> change...
> 
> [--SNIP--]
>>> diff --git a/package/lldpd/lldpd.mk b/package/lldpd/lldpd.mk
>>> index 924f2db..008d4bf 100644
>>> --- a/package/lldpd/lldpd.mk
>>> +++ b/package/lldpd/lldpd.mk
>>> @@ -9,7 +9,7 @@ LLDPD_SITE = http://media.luffy.cx/files/lldpd
>>>  LLDPD_DEPENDENCIES = host-pkgconf libevent
>>>  LLDPD_LICENSE = ISC
>>>  LLDPD_LICENSE_FILES = README.md
>>> -# 0001-build-make-generation-of-atom-glue-compatible-with-o.patch
>>> +# Touching configure.ac and Makefile.am in our patches:
>>
>>  I actually find it quite convenient if the comment states which patch(es) touch
>> the autotools files, but ATM we don't do that very often, so OK.
> 
> Yes, can do.
> 
>>  Only spelling mistakes so
>>
>> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> Thanks! :-)
> 
> Regards,
> Yann E. MORIN.
>
diff mbox

Patch

diff --git a/package/lldpd/0002-configure-do-not-check-for-libbsd.patch b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
new file mode 100644
index 0000000..b89b442
--- /dev/null
+++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
@@ -0,0 +1,62 @@ 
+From 0801a066cd4a24a858ddfa7c62c7802e0f5533a8 Mon Sep 17 00:00:00 2001
+From: "Yann E. MORIN" <yann.morin.1998@free.fr>
+Date: Sun, 14 Aug 2016 12:15:17 +0200
+Subject: [PATCH] configure: do not check for libbsd
+
+libbsd causes build issues because it .pc file is borked: it contains
+-isystem in CFLAGS, which is not munged by pkg-config, so we end up
+using the headers of the build machine, causing all sorts of hard to
+debug trouble at build time.
+
+lldpd uses libbsd to a few helper functions, but has fallback in case
+libbsd is not available. The only feature that is lost when not using
+libbsd is that the neighbour name is no longer displayed in the
+/proc/self/cmdline. As the author of lldpd said on IRC: "people should
+survive! ;-)"
+
+So we just remove the detection of libbsd altogether.
+
+Fixes:
+    http://autobuild.buildroot.org/results/6b7/6b70fa379e834ec71cc260ba6af771b531ca3511/
+    http://autobuild.buildroot.org/results/769/769074c4bb67336ae6679f2c1cd2a8220d2bec24/
+    http://autobuild.buildroot.org/results/c8a/c8a6001f437701ecc75f6c9252935645bda8a8c8/
+    [...]
+
+Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
+---
+ configure.ac | 19 -------------------
+ 1 file changed, 19 deletions(-)
+
+diff --git a/configure.ac b/configure.ac
+index dd723b0..45498ce 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -165,25 +165,6 @@ AC_FUNC_MALLOC
+ AC_FUNC_REALLOC
+ AC_FUNC_FORK
+ 
+-# Some functions can be in libbsd
+-PKG_CHECK_MODULES([libbsd], [libbsd-overlay], [
+-  _save_CFLAGS="$CFLAGS"
+-  _save_LIBS="$LIBS"
+-  CFLAGS="$CFLAGS $libbsd_CFLAGS"
+-  LIBS="$LIBS $libbsd_LIBS"
+-  AC_MSG_CHECKING([if libbsd can be linked correctly])
+-  AC_LINK_IFELSE([AC_LANG_SOURCE([[int main() { return 0; }]])],
+-    [
+-      AC_MSG_RESULT(yes)
+-      LLDP_CFLAGS="$LLDP_CFLAGS $libbsd_CFLAGS"
+-      LLDP_LDFLAGS="$LLDP_LDFLAGS $libbsd_LIBS"
+-    ],[
+-      AC_MSG_RESULT(no)
+-      CFLAGS="$_save_CFLAGS"
+-      LIBS="$_save_LIBS"
+-    ])
+-], [:])
+-
+ # setproctitle may have an _init function
+ AC_REPLACE_FUNCS([setproctitle])
+ AC_CHECK_FUNCS([setproctitle_init])
+-- 
+2.7.4
+
diff --git a/package/lldpd/lldpd.mk b/package/lldpd/lldpd.mk
index 924f2db..008d4bf 100644
--- a/package/lldpd/lldpd.mk
+++ b/package/lldpd/lldpd.mk
@@ -9,7 +9,7 @@  LLDPD_SITE = http://media.luffy.cx/files/lldpd
 LLDPD_DEPENDENCIES = host-pkgconf libevent
 LLDPD_LICENSE = ISC
 LLDPD_LICENSE_FILES = README.md
-# 0001-build-make-generation-of-atom-glue-compatible-with-o.patch
+# Touching configure.ac and Makefile.am in our patches:
 LLDPD_AUTORECONF = YES
 
 # Detection of c99 support in configure fails without WCHAR. To enable