| Message ID | 1368704401-1832-1-git-send-email-gustavo@zacarias.com.ar |
|---|---|
| State | Accepted |
| Commit | 1c3afe4bc14a55aa491be270c24f31d1c5d4d8f1 |
| Headers | show |
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
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.
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
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.
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
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.
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
>>>>> "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.
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)
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(+)