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

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

[Yann E. MORIN]

libbsd installs at least one .pc file that contains a -isystem include
path (-isystem /usr.include/bsd), but -isystem is not munged by
pkg-config, so we end up using /usr/include/bsd which is 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 (just setting
the neighbour name in /proc/self/smdline), 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>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
Note: this is not for upstream as-is; it's just a workaround for
Buildroot, given how close next release is. After the release, we can
bump lldpd to use a newer upstream (they already worked on this problem,
just in another way).

---
Changes v1 -> v2:
  - slightly expand the commit log  (Arnout)
  - typoes  (Arnout)
---
 .../0002-configure-do-not-check-for-libbsd.patch   | 61 ++++++++++++++++++++++
 package/lldpd/lldpd.mk                             |  2 +-
 2 files changed, 62 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..4d63274
--- /dev/null
+++ b/package/lldpd/0002-configure-do-not-check-for-libbsd.patch
@@ -0,0 +1,61 @@
+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 its libbsd-overlay.pc file is borked:
+it contains -isystem in CFLAGS, which is not munged by pkgconf, 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-overlay for a few helper functions, but has fallbacks
+in case it is not available. The only feature that is lost when not using
+it is that the neighbour name is no longer displayed in /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..7613f9e 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
+# 0002-configure-do-not-check-for-libbsd.patch
 LLDPD_AUTORECONF = YES
 
 # Detection of c99 support in configure fails without WCHAR. To enable
-- 
2.7.4

[Buildroot] [PATCH 2/2] package/lldpd: explicit optional dependencies

[Yann E. MORIN]

check and valgrind are optional depenencies of lldpd, so we must depend
on them when either is enabled.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Changes v1 -> v2:
  - add optional dependency on valgrind  (Arnout)

---
PS. Arnout, I did not carry your reviewd-by tag, since I changed the
patch to also add the dependency on valgrind.
---
 package/lldpd/lldpd.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/package/lldpd/lldpd.mk b/package/lldpd/lldpd.mk
index 7613f9e..349c556 100644
--- a/package/lldpd/lldpd.mk
+++ b/package/lldpd/lldpd.mk
@@ -12,6 +12,14 @@ LLDPD_LICENSE_FILES = README.md
 # 0002-configure-do-not-check-for-libbsd.patch
 LLDPD_AUTORECONF = YES
 
+ifeq ($(BR2_PACKAGE_CHECK),y)
+LLDPD_DEPENDENCIES += check
+endif
+
+ifeq ($(BR2_PACKAGE_VALGRIND),y)
+LLDPD_DEPENDENCIES += valgrind
+endif
+
 # Detection of c99 support in configure fails without WCHAR. To enable
 # automatic detection of c99 support by configure, we need to enable
 # WCHAR in toolchain. But actually we do not need WCHAR at lldpd
-- 
2.7.4

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

[Thomas Petazzoni]

Hello,

On Thu, 18 Aug 2016 23:01:21 +0200, Yann E. MORIN wrote:
> libbsd installs at least one .pc file that contains a -isystem include
> path (-isystem /usr.include/bsd), but -isystem is not munged by
> pkg-config, so we end up using /usr/include/bsd which is 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 (just setting
> the neighbour name in /proc/self/smdline), 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.

I don't like this fix, for two reasons:

 * The patch is not upstreamable.

 * It works around the problem, living a potentially problematic
   libbsd.pc installed which can be used by other packages than lldpd.

Isn't the right solution to add support in pkg-config to mungle
-isystem flags?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] package/lldpd: explicit optional dependencies

[Thomas Petazzoni]

Hello,

On Thu, 18 Aug 2016 23:01:22 +0200, Yann E. MORIN wrote:
> check and valgrind are optional depenencies of lldpd, so we must depend
> on them when either is enabled.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> Changes v1 -> v2:
>   - add optional dependency on valgrind  (Arnout)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Arnout Vandecappelle]

On 18-08-16 23:24, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 18 Aug 2016 23:01:21 +0200, Yann E. MORIN wrote:
>> libbsd installs at least one .pc file that contains a -isystem include
>> path (-isystem /usr.include/bsd), but -isystem is not munged by
>> pkg-config, so we end up using /usr/include/bsd which is 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 (just setting
>> the neighbour name in /proc/self/smdline), 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.
> 
> I don't like this fix, for two reasons:
> 
>  * The patch is not upstreamable.
> 
>  * It works around the problem, living a potentially problematic
>    libbsd.pc installed which can be used by other packages than lldpd.
> 
> Isn't the right solution to add support in pkg-config to mungle
> -isystem flags?

 That is the right solution. However, it's not exactly trivial to do that in
pkg-config - they're using kind of messy parsing of the flags.

 So I propose to wait until the end of the week to see if kaniini [1] comes up
with a patch we can backport, and if not, apply this one.

 My Reviewed-by stands :-)

 Regards,
 Arnout


[1] https://github.com/pkgconf/pkgconf/issues/94


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF