[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Peter Korsgaard]

Fixes:
http://autobuild.buildroot.net/results/67ef520d82ea529a9fe593d83a3aeae5f8b0ee5d/
http://autobuild.buildroot.net/results/eafc3e4be571d5ecee549a11530ac4e508f31782/
http://autobuild.buildroot.net/results/ba7f30833fef54162a82f4b336a72d6599594526/

The netsurf build system mixes up host and target CFLAGS, so it isn't
compatible with architectures where we pass ABI specicif compiler flags (in
TARGET_ABI).

Add a _ARCH_SUPPORTS kconfig variable matching the TARGET_ABI logic we have
in package/Makefile.in, and use it to disallow netsurf for those
architectures.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 package/netsurf/Config.in | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/package/netsurf/Config.in b/package/netsurf/Config.in
index 30b2695309..4f7d5c2713 100644
--- a/package/netsurf/Config.in
+++ b/package/netsurf/Config.in
@@ -1,10 +1,20 @@
+# netsurf mixes up host and target CFLAGS, so it isn't compatible with
+# architectures where we pass ABI specific compiler flags (TARGET_ABI)
+config BR2_PACKAGE_NETSURF_ARCH_SUPPORTS
+	bool
+	default y if !((BR2_arc && BR2_ARC_ATOMIC_EXT) || \
+		BR2_powerpc_8540 || BR2_powerpc_8548 || BR2_powerpc_e500mc || \
+		BR2_xtensa)
+
 comment "netsurf needs a toolchain w/ dynamic library"
 	depends on BR2_STATIC_LIBS
+	depends on BR2_PACKAGE_NETSURF_ARCH_SUPPORTS
 
 config BR2_PACKAGE_NETSURF
 	bool "netsurf"
 	# static linking support is broken beyond repair
 	depends on !BR2_STATIC_LIBS
+	depends on BR2_PACKAGE_NETSURF_ARCH_SUPPORTS
 	select BR2_PACKAGE_EXPAT
 	select BR2_PACKAGE_JPEG
 	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
-- 
2.11.0

[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Thomas Huth]

Am Fri, 31 May 2019 23:40:52 +0200
schrieb Peter Korsgaard <peter@korsgaard.com>:

> Fixes:
> http://autobuild.buildroot.net/results/67ef520d82ea529a9fe593d83a3aeae5f8b0ee5d/
> http://autobuild.buildroot.net/results/eafc3e4be571d5ecee549a11530ac4e508f31782/
> http://autobuild.buildroot.net/results/ba7f30833fef54162a82f4b336a72d6599594526/
> 
> The netsurf build system mixes up host and target CFLAGS, so it isn't
> compatible with architectures where we pass ABI specicif compiler

s/specicif/specific/ please.

> flags (in TARGET_ABI).
> 
> Add a _ARCH_SUPPORTS kconfig variable matching the TARGET_ABI logic
> we have in package/Makefile.in, and use it to disallow netsurf for
> those architectures.
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>  package/netsurf/Config.in | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/package/netsurf/Config.in b/package/netsurf/Config.in
> index 30b2695309..4f7d5c2713 100644
> --- a/package/netsurf/Config.in
> +++ b/package/netsurf/Config.in
> @@ -1,10 +1,20 @@
> +# netsurf mixes up host and target CFLAGS, so it isn't compatible
> with +# architectures where we pass ABI specific compiler flags
> (TARGET_ABI) +config BR2_PACKAGE_NETSURF_ARCH_SUPPORTS
> +	bool
> +	default y if !((BR2_arc && BR2_ARC_ATOMIC_EXT) || \
> +		BR2_powerpc_8540 || BR2_powerpc_8548 ||
> BR2_powerpc_e500mc || \
> +		BR2_xtensa)
> +
>  comment "netsurf needs a toolchain w/ dynamic library"
>  	depends on BR2_STATIC_LIBS
> +	depends on BR2_PACKAGE_NETSURF_ARCH_SUPPORTS

First, the comment is now not accurate anymore. It should also say
something about the architectures where it is not available.

Then the logic below says that PACKAGE_NETSURF depends on "!STATIC_LIBS
*AND* NETSURF_ARCH_SUPPORT". The dependency of the comment above is
inverted, so I think this rather should be:

       depends on BR2_STATIC_LIBS || !BR2_PACKAGE_NETSURF_ARCH_SUPPORTS

?

>  config BR2_PACKAGE_NETSURF
>  	bool "netsurf"
>  	# static linking support is broken beyond repair
>  	depends on !BR2_STATIC_LIBS
> +	depends on BR2_PACKAGE_NETSURF_ARCH_SUPPORTS
>  	select BR2_PACKAGE_EXPAT
>  	select BR2_PACKAGE_JPEG
>  	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE

 Thomas H

[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Thomas Petazzoni]

Hello,

On Sat, 1 Jun 2019 07:33:51 +0200
Thomas Huth <huth@tuxfamily.org> wrote:

> >  comment "netsurf needs a toolchain w/ dynamic library"
> >  	depends on BR2_STATIC_LIBS
> > +	depends on BR2_PACKAGE_NETSURF_ARCH_SUPPORTS  
> 
> First, the comment is now not accurate anymore. It should also say
> something about the architectures where it is not available.

No, we never put comments about architecture dependencies, because
there's nothing the user can do about it.

> Then the logic below says that PACKAGE_NETSURF depends on "!STATIC_LIBS
> *AND* NETSURF_ARCH_SUPPORT". The dependency of the comment above is
> inverted, so I think this rather should be:
> 
>        depends on BR2_STATIC_LIBS || !BR2_PACKAGE_NETSURF_ARCH_SUPPORTS

No, because we precisely don't want the comment to show up when we're
on an architecture that cannot support the package.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Thomas Petazzoni]

Hello,

On Fri, 31 May 2019 23:40:52 +0200
Peter Korsgaard <peter@korsgaard.com> wrote:

> Fixes:
> http://autobuild.buildroot.net/results/67ef520d82ea529a9fe593d83a3aeae5f8b0ee5d/
> http://autobuild.buildroot.net/results/eafc3e4be571d5ecee549a11530ac4e508f31782/
> http://autobuild.buildroot.net/results/ba7f30833fef54162a82f4b336a72d6599594526/
> 
> The netsurf build system mixes up host and target CFLAGS, so it isn't
> compatible with architectures where we pass ABI specicif compiler flags (in
> TARGET_ABI).
> 
> Add a _ARCH_SUPPORTS kconfig variable matching the TARGET_ABI logic we have
> in package/Makefile.in, and use it to disallow netsurf for those
> architectures.
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

I understand the idea, but we need to realize it has some drawbacks:

 - If we add additional flags for some architecture, this will have to
   be updated.

 - If some custom flags are passed in BR2_TARGET_OPTIMIZATION, I would
   assume it would also cause a build failure, no ?

If we really can't remove the package, then I'm fine with this patch as
a stop-gap measure. But I think removing the package is the sanest
thing to do. If anything, removing the package might wake up the people
using it so that they fix it.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > Hello,
 > On Fri, 31 May 2019 23:40:52 +0200
 > Peter Korsgaard <peter@korsgaard.com> wrote:

 >> Fixes:
 >> http://autobuild.buildroot.net/results/67ef520d82ea529a9fe593d83a3aeae5f8b0ee5d/
 >> http://autobuild.buildroot.net/results/eafc3e4be571d5ecee549a11530ac4e508f31782/
 >> http://autobuild.buildroot.net/results/ba7f30833fef54162a82f4b336a72d6599594526/
 >> 
 >> The netsurf build system mixes up host and target CFLAGS, so it isn't
 >> compatible with architectures where we pass ABI specicif compiler flags (in
 >> TARGET_ABI).
 >> 
 >> Add a _ARCH_SUPPORTS kconfig variable matching the TARGET_ABI logic we have
 >> in package/Makefile.in, and use it to disallow netsurf for those
 >> architectures.
 >> 
 >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

 > I understand the idea, but we need to realize it has some drawbacks:

 >  - If we add additional flags for some architecture, this will have to
 >    be updated.

Correct, but the autobuilders will presumably show us pretty fast.


 >  - If some custom flags are passed in BR2_TARGET_OPTIMIZATION, I would
 >    assume it would also cause a build failure, no ?

Yes - If they are ABI specific, E.G. not understood by the host compiler.


 > If we really can't remove the package, then I'm fine with this patch as
 > a stop-gap measure. But I think removing the package is the sanest
 > thing to do. If anything, removing the package might wake up the people
 > using it so that they fix it.

I agree, I only created this patch as there was pushback on IRC against
removing netsurf this close to the release.

I'll apply this for master and then revisit the removal once we have next
merged.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package/netsurf: disallow on archs requiring ABI specific CFLAGS

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

 > Fixes:
 > http://autobuild.buildroot.net/results/67ef520d82ea529a9fe593d83a3aeae5f8b0ee5d/
 > http://autobuild.buildroot.net/results/eafc3e4be571d5ecee549a11530ac4e508f31782/
 > http://autobuild.buildroot.net/results/ba7f30833fef54162a82f4b336a72d6599594526/

 > The netsurf build system mixes up host and target CFLAGS, so it isn't
 > compatible with architectures where we pass ABI specicif compiler flags (in
 > TARGET_ABI).

 > Add a _ARCH_SUPPORTS kconfig variable matching the TARGET_ABI logic we have
 > in package/Makefile.in, and use it to disallow netsurf for those
 > architectures.

 > Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

Committed to 2019.02.x, thanks.

-- 
Bye, Peter Korsgaard