[NEXT,1/1] Liblo - disable werror and link libatomic

Message ID 1518470275-10997-1-git-send-email-alexbaldwinmusic@gmail.com
State Changes Requested
Headers show
Series
  • [NEXT,1/1] Liblo - disable werror and link libatomic
Related show

Commit Message

Alex B Feb. 12, 2018, 9:17 p.m.
From: Alex <alexbaldwinmusic@gmail.com>

This patch fixes 2 errors that were discovered by the autobuild:

1) -werror is still present, which threw up errors regarding redirecting sys/poll.h
to poll.h in the file server.c
To fix this -werror has been disbled with a conf_opt and a patch applied to
server.c that fixes the redirect.

2) On some architectures libatomic must be linked. A conf_env has been added
that links libatomic if it is present in the toolchain.

Signed-off-by: Alex <alexbaldwinmusic@gmail.com>
---
 package/liblo/0000-server.c-fixHeaderRedirects.patch | 13 +++++++++++++
 package/liblo/liblo.mk                               | 13 +++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 package/liblo/0000-server.c-fixHeaderRedirects.patch

--
2.7.4

Comments

Thomas Petazzoni Feb. 13, 2018, 8:01 a.m. | #1
Hello,

Thanks a lot for this updated patch! It looks better, but there are
still a few things that could be improved. Hopefully you won't get
annoyed by all those comments, and keep sending new iterations. See
below.

First, the commit title should always be:

	<package>: <description>

with <package> being the package name, in lower-case. So in your case:

	liblo: disable werror
	liblo: link with libatomic

(see below why two titles)

On Mon, 12 Feb 2018 22:17:55 +0100, Alex B wrote:
> From: Alex <alexbaldwinmusic@gmail.com>
> 
> This patch fixes 2 errors that were discovered by the autobuild:
> 
> 1) -werror is still present, which threw up errors regarding redirecting sys/poll.h
> to poll.h in the file server.c
> To fix this -werror has been disbled with a conf_opt and a patch applied to
> server.c that fixes the redirect.
> 
> 2) On some architectures libatomic must be linked. A conf_env has been added
> that links libatomic if it is present in the toolchain.

There are two separate issues, so this calls for two separate patches.

For each issue, we always add a reference to the autobuilder failure
being fixed, which has proven to be very useful when we get back to
such commits in the future, and wonder what the problem really was.
Something like:

===
Fixes:

  http://autobuild.buildroot.net/results/f0e47d43a2d49821ce2e16dff4ec9b3ef565bc6c/
===

is sufficient.

> diff --git a/package/liblo/0000-server.c-fixHeaderRedirects.patch b/package/liblo/0000-server.c-fixHeaderRedirects.patch
> new file mode 100644
> index 0000000..ae284d2
> --- /dev/null
> +++ b/package/liblo/0000-server.c-fixHeaderRedirects.patch

This patch should have a description and a Signed-off-by. Also, since
upstream uses Git, we like to have a Git formatted patch as well. So,
to create the patch:

 (1) Clone the upstream repository
     git clone https://github.com/radarsat1/liblo.git

 (2) Create a branch starting at the tag we're using in Buildroot
     git checkout -b build-fix 0.29

 (3) Do your changes

 (4) Commit

 (5) Generate the patch
     git format-patch HEAD^

> -# IPv6 support broken, issue known upstream
> -LIBLO_CONF_OPTS = --disable-ipv6
> +# IPv6 support broken, issue known upstream.

The addition of the final dot is not really necessary/related.

> +# wError - not needed for release.
> +LIBLO_CONF_OPTS += \
> +       --disable-ipv6 \
> +       --disable-werror

Do we need both the --disable-werror *and* the patch? Perhaps adding
--disable-werror is sufficient. The patch can however be submitted
upstream, so that the warning disappears when we update to a newer
upstream release.

Thanks a lot!

Thomas

Patch

diff --git a/package/liblo/0000-server.c-fixHeaderRedirects.patch b/package/liblo/0000-server.c-fixHeaderRedirects.patch
new file mode 100644
index 0000000..ae284d2
--- /dev/null
+++ b/package/liblo/0000-server.c-fixHeaderRedirects.patch
@@ -0,0 +1,13 @@ 
+diff --git a/src/server.c b/src/server.c
+index 01fa08f..11c62d2 100644
+--- a/src/server.c
++++ b/src/server.c
+@@ -51,7 +51,7 @@
+ #include <netdb.h>
+ #include <sys/socket.h>
+ #ifdef HAVE_POLL
+-#include <sys/poll.h>
++#include <poll.h>
+ #endif
+ #include <sys/un.h>
+ #include <arpa/inet.h>
diff --git a/package/liblo/liblo.mk b/package/liblo/liblo.mk
index 14b0527..7332334 100644
--- a/package/liblo/liblo.mk
+++ b/package/liblo/liblo.mk
@@ -11,7 +11,16 @@  LIBLO_LICENSE = LGPL-2.1+
 LIBLO_LICENSE_FILES = COPYING
 LIBLO_INSTALL_STAGING = YES

-# IPv6 support broken, issue known upstream
-LIBLO_CONF_OPTS = --disable-ipv6
+# IPv6 support broken, issue known upstream.
+# wError - not needed for release.
+LIBLO_CONF_OPTS += \
+       --disable-ipv6 \
+       --disable-werror
+
+# Liblo uses atomics, so we need to link with
+# libatomic for the architectures who explicitly need libatomic.
+ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
+	LIBLO_CONF_ENV += LIBS="-latomic"
+endif

 $(eval $(autotools-package))