All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH NEXT 1/1] Liblo - disable werror and link libatomic
@ 2018-02-12 21:17 Alex B
  2018-02-13  8:01 ` Thomas Petazzoni
  0 siblings, 1 reply; 2+ messages in thread
From: Alex B @ 2018-02-12 21:17 UTC (permalink / raw)
  To: buildroot

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

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))
--
2.7.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Buildroot] [PATCH NEXT 1/1] Liblo - disable werror and link libatomic
  2018-02-12 21:17 [Buildroot] [PATCH NEXT 1/1] Liblo - disable werror and link libatomic Alex B
@ 2018-02-13  8:01 ` Thomas Petazzoni
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Petazzoni @ 2018-02-13  8:01 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-02-13  8:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-12 21:17 [Buildroot] [PATCH NEXT 1/1] Liblo - disable werror and link libatomic Alex B
2018-02-13  8:01 ` Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.