[Buildroot] [PATCH 0/1] Fix for packages which use CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT and target is ARMv8

[Buildroot] [PATCH 0/1] Fix for packages which use CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT and target is ARMv8

[Adrian Perez de Castro]

Hello again,

Earlier today I sent a patch to update WebKitGTK+ to version 2.16.5, which
I tested by building for the RPi3 with a configuration which was incorrectly
using ARMv6 as target. Then later I noticed the mistake and switched over to
use ARMv8 as target, but then WebKitGTK+ wouldn't build and CMake would stop
with the following error:

   [...]
   -- Detecting CXX compiler ABI info - done
   -- Detecting CXX compile features
   -- Detecting CXX compile features - done
   CMake Error at CMakeLists.txt:87 (message):
     Unknown CPU 'l'

Applying this patch to pkg-cmake.mk fixes the issue above, which I suspect
might affect other packages as well.

Cheers,


Adrian Perez de Castro (1):
  pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT for ARMv8

 package/pkg-cmake.mk | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.13.2

[Buildroot] [PATCH 1/1] pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT for ARMv8

[Adrian Perez de Castro]

This is needed for correctly building some CMake-based packages which
use this variable. For example, this is needed for WebKitGTK+ 2.16.x
to build correctly when an ARMv8 target is configured.

Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>
---
 package/pkg-cmake.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index 401084fb12..914bda7482 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -219,6 +219,8 @@ else ifeq ($(BR2_ARM_CPU_ARMV6),y)
 CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv6
 else ifeq ($(BR2_ARM_CPU_ARMV7A),y)
 CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv7
+else ifeq ($(BR2_ARM_CPU_ARMV8),y)
+CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv8
 endif
 
 ifeq ($(BR2_arm),y)
-- 
2.13.2

[Buildroot] [PATCH 1/1] pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT for ARMv8

[Arnout Vandecappelle]

On 05-07-17 21:08, Adrian Perez de Castro wrote:
> This is needed for correctly building some CMake-based packages which
> use this variable. For example, this is needed for WebKitGTK+ 2.16.x
> to build correctly when an ARMv8 target is configured.

 Well, it's needed for any cmake package on ARMv8...


> Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

> ---
>  package/pkg-cmake.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 401084fb12..914bda7482 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -219,6 +219,8 @@ else ifeq ($(BR2_ARM_CPU_ARMV6),y)
>  CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv6
>  else ifeq ($(BR2_ARM_CPU_ARMV7A),y)

 I noticed that armv7m is also missing. Could you try to find the corresponding
CMake architecture and add it?

 Also, it could be useful to add

else
$(error ARM CPU variant not set for CMake)
endif

 Not *that* helpful for users, but better than the error you got :-)

 Regards,
 Arnout

>  CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv7
> +else ifeq ($(BR2_ARM_CPU_ARMV8),y)
> +CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv8
>  endif
>  
>  ifeq ($(BR2_arm),y)
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/1] pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT for ARMv8

[Adrian Perez de Castro]

Hi Arnout,

Thanks for the reviewing :-)

On Wed, 5 Jul 2017 22:23:26 +0200, Arnout Vandecappelle <arnout@mind.be> wrote:

> On 05-07-17 21:08, Adrian Perez de Castro wrote:
> > This is needed for correctly building some CMake-based packages which
> > use this variable. For example, this is needed for WebKitGTK+ 2.16.x
> > to build correctly when an ARMv8 target is configured.
> 
>  Well, it's needed for any cmake package on ARMv8...
> 
> 
> > Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> > ---
> >  package/pkg-cmake.mk | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> > index 401084fb12..914bda7482 100644
> > --- a/package/pkg-cmake.mk
> > +++ b/package/pkg-cmake.mk
> > @@ -219,6 +219,8 @@ else ifeq ($(BR2_ARM_CPU_ARMV6),y)
> >  CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT = armv6
> >  else ifeq ($(BR2_ARM_CPU_ARMV7A),y)
> 
>  I noticed that armv7m is also missing. Could you try to find the corresponding
> CMake architecture and add it?

Sure, I'll try and look into this.

>  Also, it could be useful to add
> 
> else
> $(error ARM CPU variant not set for CMake)
> endif
> 
>  Not *that* helpful for users, but better than the error you got :-)

I think that is a good idea. Even if for an user it is not so handy, it is
better to fail early. It took me a good while to figure out where was the root
of the issue was, and having an error message early would have been faster.
Even if a build succeeds without setting CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT,
not having a correct value here could potentially cause mysterious failures at
run-time. I'll be sending a patch for this later as well.

Cheers,

--
 Adri?n ?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170706/4d9a046e/attachment.asc>

[Buildroot] [PATCH 1/1] pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT for ARMv8

[Thomas Petazzoni]

Hello,

On Wed,  5 Jul 2017 22:08:40 +0300, Adrian Perez de Castro wrote:
> This is needed for correctly building some CMake-based packages which
> use this variable. For example, this is needed for WebKitGTK+ 2.16.x
> to build correctly when an ARMv8 target is configured.
> 
> Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>
> ---
>  package/pkg-cmake.mk | 2 ++
>  1 file changed, 2 insertions(+)

Applied to master, thanks.

It is worth mentioning that sending a cover letter for a one patch
patch series isn't really necessary. If you want to put additional
information besides the commit log, you can put them after the "---"
separator below the SoB line.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR_ARM_VARIANT for ARMv8

[Adrian Perez de Castro]

On Thu, 6 Jul 2017 23:24:13 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Hello,
> 
> On Wed,  5 Jul 2017 22:08:40 +0300, Adrian Perez de Castro wrote:
> > This is needed for correctly building some CMake-based packages which
> > use this variable. For example, this is needed for WebKitGTK+ 2.16.x
> > to build correctly when an ARMv8 target is configured.
> > 
> > Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>
> > ---
> >  package/pkg-cmake.mk | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Applied to master, thanks.
> 
> It is worth mentioning that sending a cover letter for a one patch
> patch series isn't really necessary. If you want to put additional
> information besides the commit log, you can put them after the "---"
> separator below the SoB line.

Ah, I thought that using the ?---? separator was being only used to write the
changes between different versions of the same patch. I will be using it from
now on when sending single patches. Thanks for the tip!

Cheers,

--
 Adri?n ?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170707/2df2fa89/attachment.asc>