diff mbox

[1/1] openpgm: disable on AVR32

Message ID 1383209956-31840-1-git-send-email-alexander.lukichev@gmail.com
State Superseded
Headers show

Commit Message

Alexander Lukichev Oct. 31, 2013, 8:59 a.m. UTC
openpgm doesn't build correctly on AVR32 using
gcc-4.2.2-avr32-2.1.5 toolchain: it is configured to call
intrinsic atomic functions not provided by the toolchain,
so they are propagated as unresolved external symbols in the
built openpgm libraries. This breaks programs that try to link
openpgm, because they do not know where to get those either. For
instance, it breaks building zeromq tests when PGM support is
selected.

This commit disables openpgm on AVR32 due to apparent absence of
interest in this package on that architecture and it breaking too
many test builds.

Fixes http://autobuild.buildroot.net/results/5a3261109ea63ba17375003eabd8b5d88757865f/
(at least)

Signed-off-by: Alexander Lukichev <alexander.lukichev@gmail.com>
---
 package/openpgm/Config.in | 5 +++++
 package/zeromq/Config.in  | 1 +
 2 files changed, 6 insertions(+)

Comments

Arnout Vandecappelle Oct. 31, 2013, 5:39 p.m. UTC | #1
On 31/10/13 09:59, Alexander Lukichev wrote:
> openpgm doesn't build correctly on AVR32 using
> gcc-4.2.2-avr32-2.1.5 toolchain: it is configured to call
> intrinsic atomic functions not provided by the toolchain,
> so they are propagated as unresolved external symbols in the
> built openpgm libraries. This breaks programs that try to link
> openpgm, because they do not know where to get those either. For
> instance, it breaks building zeromq tests when PGM support is
> selected.
>
> This commit disables openpgm on AVR32 due to apparent absence of
> interest in this package on that architecture and it breaking too
> many test builds.
>
> Fixes http://autobuild.buildroot.net/results/5a3261109ea63ba17375003eabd8b5d88757865f/
> (at least)
>
> Signed-off-by: Alexander Lukichev <alexander.lukichev@gmail.com>

  Good plan! However, your patch is incomplete.

> ---
>   package/openpgm/Config.in | 5 +++++
>   package/zeromq/Config.in  | 1 +
>   2 files changed, 6 insertions(+)
>
> diff --git a/package/openpgm/Config.in b/package/openpgm/Config.in
> index cae74f7..94733cd 100644
> --- a/package/openpgm/Config.in
> +++ b/package/openpgm/Config.in
> @@ -1,5 +1,6 @@
>   config BR2_PACKAGE_OPENPGM
>   	bool "openpgm"
> +	depends on !BR2_avr32
>   	depends on BR2_TOOLCHAIN_HAS_THREADS
>   	depends on BR2_INET_IPV6
>   	depends on BR2_USE_WCHAR
> @@ -14,3 +15,7 @@ config BR2_PACKAGE_OPENPGM
>
>   comment "openpgm needs a toolchain w/ wchar, threads, IPv6"
>   	depends on !(BR2_TOOLCHAIN_HAS_THREADS && BR2_INET_IPV6 && BR2_USE_WCHAR)
> +	depends on !BR2_avr32
> +
> +comment "openpgm is BROKEN on AVR32"
> +	depends on BR2_avr32

  This comment isn't needed. We don't do it for other packages, and 
adding it would make it even more work to disable architectures.

> diff --git a/package/zeromq/Config.in b/package/zeromq/Config.in
> index 42e13d2..3e8516c 100644
> --- a/package/zeromq/Config.in
> +++ b/package/zeromq/Config.in
> @@ -30,6 +30,7 @@ config BR2_PACKAGE_ZEROMQ
>   config BR2_PACKAGE_ZEROMQ_PGM
>   	bool "PGM/EPGM support"
>   	depends on BR2_PACKAGE_ZEROMQ
> +	depends on !BR2_avr32

  zeromq has a lot of reverse dependencies: cppzmq, czmq, filemq, 
mongrel2, [python-pyzmq not necessary because python needs MMU], [zmqpp 
already disabled for avr32]. And there's one more transitive dependency 
to zyre. So all these should also be disabled.

  Regards,
  Arnout

>   	select BR2_PACKAGE_OPENPGM
>   	help
>   	  Add support for Pragmatic General Multicast protocol (RFC 3208)
>
Thomas Petazzoni Nov. 1, 2013, 2:37 p.m. UTC | #2
Dear Alexander Lukichev,

On Thu, 31 Oct 2013 10:59:16 +0200, Alexander Lukichev wrote:

>  config BR2_PACKAGE_OPENPGM
>  	bool "openpgm"
> +	depends on !BR2_avr32

Would be good to add a comment explaining why we have this (compiler
too old, missing intrinsics)

>  	depends on BR2_TOOLCHAIN_HAS_THREADS
>  	depends on BR2_INET_IPV6
>  	depends on BR2_USE_WCHAR
> @@ -14,3 +15,7 @@ config BR2_PACKAGE_OPENPGM
>  
>  comment "openpgm needs a toolchain w/ wchar, threads, IPv6"
>  	depends on !(BR2_TOOLCHAIN_HAS_THREADS && BR2_INET_IPV6 && BR2_USE_WCHAR)
> +	depends on !BR2_avr32
> +
> +comment "openpgm is BROKEN on AVR32"
> +	depends on BR2_avr32

We generally don't add comments for such cases (i.e package not
available on the selected architecture).

> diff --git a/package/zeromq/Config.in b/package/zeromq/Config.in
> index 42e13d2..3e8516c 100644
> --- a/package/zeromq/Config.in
> +++ b/package/zeromq/Config.in
> @@ -30,6 +30,7 @@ config BR2_PACKAGE_ZEROMQ
>  config BR2_PACKAGE_ZEROMQ_PGM
>  	bool "PGM/EPGM support"
>  	depends on BR2_PACKAGE_ZEROMQ
> +	depends on !BR2_avr32

Maybe:

	depends on !BR2_avr32 # openpgm

>  	select BR2_PACKAGE_OPENPGM
>  	help
>  	  Add support for Pragmatic General Multicast protocol (RFC 3208)

Can you fix these minor comments and resend an updated version?

Thanks a lot!

Thomas Petazzoni
Alexander Lukichev Nov. 3, 2013, 11:10 a.m. UTC | #3
Hi Arnout,

10/31/2013 07:39 PM, Arnout Vandecappelle wrote:
>> +comment "openpgm is BROKEN on AVR32"
>> +    depends on BR2_avr32
>  This comment isn't needed. We don't do it for other packages, and
> adding it would make it even more work to disable architectures.
Understood. I was angry at openpgm :)

>>   config BR2_PACKAGE_ZEROMQ_PGM
>>       bool "PGM/EPGM support"
>>       depends on BR2_PACKAGE_ZEROMQ
>> +    depends on !BR2_avr32
>  zeromq has a lot of reverse dependencies: cppzmq, czmq, filemq,
> mongrel2, [python-pyzmq not necessary because python needs MMU], [zmqpp
> already disabled for avr32]. And there's one more transitive dependency
> to zyre. So all these should also be disabled.

I do not understand. Why should they be disabled? This patch does not
disable the zeromq library, merely PGM support in it. I may be mistaken
but "users" of zeromq that we have in Buildroot do not actually require
this feature (PGM support). Or am I missing the point?

Thanks for your comments!

--
Best regards,
  Alexander Lukichev
Thomas Petazzoni Nov. 3, 2013, 11:18 a.m. UTC | #4
Dear Alexander Lukichev,

On Sun, 03 Nov 2013 13:10:27 +0200, Alexander Lukichev wrote:

> >>   config BR2_PACKAGE_ZEROMQ_PGM
> >>       bool "PGM/EPGM support"
> >>       depends on BR2_PACKAGE_ZEROMQ
> >> +    depends on !BR2_avr32
> >  zeromq has a lot of reverse dependencies: cppzmq, czmq, filemq,
> > mongrel2, [python-pyzmq not necessary because python needs MMU], [zmqpp
> > already disabled for avr32]. And there's one more transitive dependency
> > to zyre. So all these should also be disabled.
> 
> I do not understand. Why should they be disabled? This patch does not
> disable the zeromq library, merely PGM support in it. I may be mistaken
> but "users" of zeromq that we have in Buildroot do not actually require
> this feature (PGM support). Or am I missing the point?

You're correct, I believe Arnout missed the fact that the dependency is
added on a sub-option of the ZeroMQ package, and not on the ZeroMQ
package itself, and there is nothing that selects
BR2_PACKAGE_ZEROMQ_PGM.

Thomas
Arnout Vandecappelle Nov. 3, 2013, 5:46 p.m. UTC | #5
On 03/11/13 12:18, Thomas Petazzoni wrote:
> Dear Alexander Lukichev,
>
> On Sun, 03 Nov 2013 13:10:27 +0200, Alexander Lukichev wrote:
>
>>>>    config BR2_PACKAGE_ZEROMQ_PGM
>>>>        bool "PGM/EPGM support"
>>>>        depends on BR2_PACKAGE_ZEROMQ
>>>> +    depends on !BR2_avr32
>>>   zeromq has a lot of reverse dependencies: cppzmq, czmq, filemq,
>>> mongrel2, [python-pyzmq not necessary because python needs MMU], [zmqpp
>>> already disabled for avr32]. And there's one more transitive dependency
>>> to zyre. So all these should also be disabled.
>>
>> I do not understand. Why should they be disabled? This patch does not
>> disable the zeromq library, merely PGM support in it. I may be mistaken
>> but "users" of zeromq that we have in Buildroot do not actually require
>> this feature (PGM support). Or am I missing the point?
>
> You're correct, I believe Arnout missed the fact that the dependency is
> added on a sub-option of the ZeroMQ package, and not on the ZeroMQ
> package itself, and there is nothing that selects
> BR2_PACKAGE_ZEROMQ_PGM.

  Indeed, sorry for the noise.

  Regards,
  Arnout
diff mbox

Patch

diff --git a/package/openpgm/Config.in b/package/openpgm/Config.in
index cae74f7..94733cd 100644
--- a/package/openpgm/Config.in
+++ b/package/openpgm/Config.in
@@ -1,5 +1,6 @@ 
 config BR2_PACKAGE_OPENPGM
 	bool "openpgm"
+	depends on !BR2_avr32
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on BR2_INET_IPV6
 	depends on BR2_USE_WCHAR
@@ -14,3 +15,7 @@  config BR2_PACKAGE_OPENPGM
 
 comment "openpgm needs a toolchain w/ wchar, threads, IPv6"
 	depends on !(BR2_TOOLCHAIN_HAS_THREADS && BR2_INET_IPV6 && BR2_USE_WCHAR)
+	depends on !BR2_avr32
+
+comment "openpgm is BROKEN on AVR32"
+	depends on BR2_avr32
diff --git a/package/zeromq/Config.in b/package/zeromq/Config.in
index 42e13d2..3e8516c 100644
--- a/package/zeromq/Config.in
+++ b/package/zeromq/Config.in
@@ -30,6 +30,7 @@  config BR2_PACKAGE_ZEROMQ
 config BR2_PACKAGE_ZEROMQ_PGM
 	bool "PGM/EPGM support"
 	depends on BR2_PACKAGE_ZEROMQ
+	depends on !BR2_avr32
 	select BR2_PACKAGE_OPENPGM
 	help
 	  Add support for Pragmatic General Multicast protocol (RFC 3208)