Patchwork poco: not available for all architectures

login
register
mail settings
Submitter Gustavo Zacarias
Date May 16, 2013, 11:40 a.m.
Message ID <1368704401-1832-1-git-send-email-gustavo@zacarias.com.ar>
Download mbox | patch
Permalink /patch/244289/
State Accepted
Commit 1c3afe4bc14a55aa491be270c24f31d1c5d4d8f1
Headers show

Comments

Gustavo Zacarias - May 16, 2013, 11:40 a.m.
Missing defines for:
aarch64, arc, blackfin, microblaze & xtensa.
Not properly supported: mips - only defines generic mips as BE, doesn't
know about mipsel (LE) thus assuming it's BE.
Fixes:
http://autobuild.buildroot.net/results/9847702b046bed59b07f0e075a58b1f31e9236ce/

This should be pretty straightforward to fix in
Foundation/include/Poco/Platform.h for interested parties since it only
cares about endianness.

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/poco/Config.in | 2 ++
 1 file changed, 2 insertions(+)
Baruch Siach - May 16, 2013, 11:54 a.m.
Hi Gustavo,

On Thu, May 16, 2013 at 08:40:01AM -0300, Gustavo Zacarias wrote:
> Missing defines for:
> aarch64, arc, blackfin, microblaze & xtensa.
> Not properly supported: mips - only defines generic mips as BE, doesn't
> know about mipsel (LE) thus assuming it's BE.
> Fixes:
> http://autobuild.buildroot.net/results/9847702b046bed59b07f0e075a58b1f31e9236ce/
> 
> This should be pretty straightforward to fix in
> Foundation/include/Poco/Platform.h for interested parties since it only
> cares about endianness.
> 
> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> ---
>  package/poco/Config.in | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/poco/Config.in b/package/poco/Config.in
> index b823071..402f96d 100644
> --- a/package/poco/Config.in
> +++ b/package/poco/Config.in
> @@ -3,6 +3,7 @@ config BR2_PACKAGE_POCO
>  	depends on BR2_INSTALL_LIBSTDCPP
>  	depends on BR2_USE_WCHAR
>  	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
>  	select BR2_PACKAGE_ZLIB
>  	select BR2_PACKAGE_PCRE
>  	help
> @@ -61,3 +62,4 @@ endif # BR2_PACKAGE_POCO
>  comment "poco requires a toolchain with WCHAR, threads, and C++ support"
>  	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP \
>  		|| !BR2_TOOLCHAIN_HAS_THREADS
> +	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)

Enabling this comment when it is the architecture that is not supported might 
confuse the user. Maybe we should add another comment? How is this handled in 
other packages?

baruch
Gustavo Zacarias - May 16, 2013, 12:02 p.m.
On 05/16/2013 08:54 AM, Baruch Siach wrote:
>>  comment "poco requires a toolchain with WCHAR, threads, and C++ support"
>>  	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP \
>>  		|| !BR2_TOOLCHAIN_HAS_THREADS
>> +	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
> 
> Enabling this comment when it is the architecture that is not supported might 
> confuse the user. Maybe we should add another comment? How is this handled in 
> other packages?

I'm actually disabling the comment if it's not a supported architecture
with that line (same as the option itself).
Regards.
Baruch Siach - May 16, 2013, 12:05 p.m.
Hi Gustavo,

On Thu, May 16, 2013 at 09:02:01AM -0300, Gustavo Zacarias wrote:
> On 05/16/2013 08:54 AM, Baruch Siach wrote:
> >>  comment "poco requires a toolchain with WCHAR, threads, and C++ support"
> >>  	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP \
> >>  		|| !BR2_TOOLCHAIN_HAS_THREADS
> >> +	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
> > 
> > Enabling this comment when it is the architecture that is not supported might 
> > confuse the user. Maybe we should add another comment? How is this handled in 
> > other packages?
> 
> I'm actually disabling the comment if it's not a supported architecture
> with that line (same as the option itself).

Ah, right. I always get confused by reverse logic.

baruch
Gustavo Zacarias - May 16, 2013, 12:07 p.m.
On 05/16/2013 09:05 AM, Baruch Siach wrote:

>>> Enabling this comment when it is the architecture that is not supported might 
>>> confuse the user. Maybe we should add another comment? How is this handled in 
>>> other packages?
>>
>> I'm actually disabling the comment if it's not a supported architecture
>> with that line (same as the option itself).
> 
> Ah, right. I always get confused by reverse logic.

Normally we don't mention "your architecture isn't supported" because
there's not much the user can do about it ("hey, change your hardware!"
wouldn't be nice hehe).
Regards.
Thomas Petazzoni - May 16, 2013, 12:23 p.m.
Dear Gustavo Zacarias,

On Thu, 16 May 2013 09:02:01 -0300, Gustavo Zacarias wrote:
> On 05/16/2013 08:54 AM, Baruch Siach wrote:
> >>  comment "poco requires a toolchain with WCHAR, threads, and C++ support"
> >>  	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP \
> >>  		|| !BR2_TOOLCHAIN_HAS_THREADS
> >> +	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
> > 
> > Enabling this comment when it is the architecture that is not supported might 
> > confuse the user. Maybe we should add another comment? How is this handled in 
> > other packages?
> 
> I'm actually disabling the comment if it's not a supported architecture
> with that line (same as the option itself).

I believe that what Baruch says is that the comment is useless when
you're on an architecture that poco doesn't support.

So it should actually be:

	depends on (BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)

No?

Thomas
Gustavo Zacarias - May 16, 2013, 12:25 p.m.
On 05/16/2013 09:23 AM, Thomas Petazzoni wrote:

>> I'm actually disabling the comment if it's not a supported architecture
>> with that line (same as the option itself).
> 
> I believe that what Baruch says is that the comment is useless when
> you're on an architecture that poco doesn't support.
> 
> So it should actually be:
> 
> 	depends on (BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
> 
> No?

Maybe it's because i'm out of coffee, but i don't see that holding.
So comment "hey you lack toolchain X" when ARCH = aarch64 || arc .. ?
So it shows up if any of those arches == true ?
Regards.
Thomas Petazzoni - May 16, 2013, 12:41 p.m.
Dear Gustavo Zacarias,

On Thu, 16 May 2013 09:25:51 -0300, Gustavo Zacarias wrote:
> On 05/16/2013 09:23 AM, Thomas Petazzoni wrote:
> 
> >> I'm actually disabling the comment if it's not a supported architecture
> >> with that line (same as the option itself).
> > 
> > I believe that what Baruch says is that the comment is useless when
> > you're on an architecture that poco doesn't support.
> > 
> > So it should actually be:
> > 
> > 	depends on (BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
> > 
> > No?
> 
> Maybe it's because i'm out of coffee, but i don't see that holding.
> So comment "hey you lack toolchain X" when ARCH = aarch64 || arc .. ?
> So it shows up if any of those arches == true ?

Doh, I must be the one needing more coffee it seems. Yeah, if the
package is not available on some arches, we need to do "depends
on !(some arches)" on both the package itself and the comment, which is
what you did.

Got confused, sorry.

Confusingly-acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Thomas
Peter Korsgaard - May 23, 2013, 9:36 p.m.
>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

 Gustavo> Missing defines for:
 Gustavo> aarch64, arc, blackfin, microblaze & xtensa.
 Gustavo> Not properly supported: mips - only defines generic mips as BE, doesn't
 Gustavo> know about mipsel (LE) thus assuming it's BE.
 Gustavo> Fixes:
 Gustavo> http://autobuild.buildroot.net/results/9847702b046bed59b07f0e075a58b1f31e9236ce/

 Gustavo> This should be pretty straightforward to fix in
 Gustavo> Foundation/include/Poco/Platform.h for interested parties
 Gustavo> since it only cares about endianness.

Committed, thanks.

Patch

diff --git a/package/poco/Config.in b/package/poco/Config.in
index b823071..402f96d 100644
--- a/package/poco/Config.in
+++ b/package/poco/Config.in
@@ -3,6 +3,7 @@  config BR2_PACKAGE_POCO
 	depends on BR2_INSTALL_LIBSTDCPP
 	depends on BR2_USE_WCHAR
 	depends on BR2_TOOLCHAIN_HAS_THREADS
+	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)
 	select BR2_PACKAGE_ZLIB
 	select BR2_PACKAGE_PCRE
 	help
@@ -61,3 +62,4 @@  endif # BR2_PACKAGE_POCO
 comment "poco requires a toolchain with WCHAR, threads, and C++ support"
 	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP \
 		|| !BR2_TOOLCHAIN_HAS_THREADS
+	depends on !(BR2_aarch64 || BR2_arc || BR2_bfin || BR2_microblaze || BR2_mipsel || BR2_xtensa)