[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Bernd Kuhls]

It uses __atomic_fetch_add_4 so libatomic must be pulled in if
necessary. Fixes
http://autobuild.buildroot.net/results/652/6521223b426c3cb4dfefab27bc6021900db6910e/

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/opencv3/Config.in  | 2 ++
 package/opencv3/opencv3.mk | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/package/opencv3/Config.in b/package/opencv3/Config.in
index 2f4aa7a..4c00bf1 100644
--- a/package/opencv3/Config.in
+++ b/package/opencv3/Config.in
@@ -1,6 +1,7 @@
 menuconfig BR2_PACKAGE_OPENCV3
 	bool "opencv3"
 	select BR2_PACKAGE_ZLIB
+	depends on BR2_TOOLCHAIN_HAS_ATOMIC
 	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL
 	depends on BR2_INSTALL_LIBSTDCPP
 	depends on BR2_USE_WCHAR
@@ -335,6 +336,7 @@ config BR2_PACKAGE_OPENCV3_INSTALL_DATA
 endif # BR2_PACKAGE_OPENCV3
 
 comment "opencv3 needs a toolchain w/ C++, NPTL, wchar, dynamic library"
+	depends on BR2_TOOLCHAIN_HAS_ATOMIC
 	depends on !BR2_INSTALL_LIBSTDCPP || \
 		!BR2_USE_WCHAR || \
 		!BR2_TOOLCHAIN_HAS_THREADS_NPTL || \
diff --git a/package/opencv3/opencv3.mk b/package/opencv3/opencv3.mk
index 91cdfe6..2529de9 100644
--- a/package/opencv3/opencv3.mk
+++ b/package/opencv3/opencv3.mk
@@ -10,6 +10,11 @@ OPENCV3_INSTALL_STAGING = YES
 OPENCV3_LICENSE = BSD-3c
 OPENCV3_LICENSE_FILES = LICENSE
 
+# Uses __atomic_fetch_add_4
+ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
+OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
+endif
+
 # OpenCV component options
 OPENCV3_CONF_OPTS += \
 	-DBUILD_DOCS=OFF \
-- 
2.9.3

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Samuel Martin]

Hi Bernd, all,

On Mon, Sep 5, 2016 at 11:00 PM, Bernd Kuhls <bernd.kuhls@t-online.de> wrote:
> It uses __atomic_fetch_add_4 so libatomic must be pulled in if
> necessary. Fixes
> http://autobuild.buildroot.net/results/652/6521223b426c3cb4dfefab27bc6021900db6910e/
>
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/opencv3/Config.in  | 2 ++
>  package/opencv3/opencv3.mk | 5 +++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/package/opencv3/Config.in b/package/opencv3/Config.in
> index 2f4aa7a..4c00bf1 100644
> --- a/package/opencv3/Config.in
> +++ b/package/opencv3/Config.in
> @@ -1,6 +1,7 @@
>  menuconfig BR2_PACKAGE_OPENCV3
>         bool "opencv3"
>         select BR2_PACKAGE_ZLIB
> +       depends on BR2_TOOLCHAIN_HAS_ATOMIC
>         depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL
>         depends on BR2_INSTALL_LIBSTDCPP
>         depends on BR2_USE_WCHAR
> @@ -335,6 +336,7 @@ config BR2_PACKAGE_OPENCV3_INSTALL_DATA
>  endif # BR2_PACKAGE_OPENCV3
>
>  comment "opencv3 needs a toolchain w/ C++, NPTL, wchar, dynamic library"
> +       depends on BR2_TOOLCHAIN_HAS_ATOMIC
>         depends on !BR2_INSTALL_LIBSTDCPP || \
>                 !BR2_USE_WCHAR || \
>                 !BR2_TOOLCHAIN_HAS_THREADS_NPTL || \
> diff --git a/package/opencv3/opencv3.mk b/package/opencv3/opencv3.mk
> index 91cdfe6..2529de9 100644
> --- a/package/opencv3/opencv3.mk
> +++ b/package/opencv3/opencv3.mk
> @@ -10,6 +10,11 @@ OPENCV3_INSTALL_STAGING = YES
>  OPENCV3_LICENSE = BSD-3c
>  OPENCV3_LICENSE_FILES = LICENSE
>
> +# Uses __atomic_fetch_add_4
> +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
> +endif

Since you add the dependency on BR2_TOOLCHAIN_HAS_ATOMIC in the
Config.in, I wonder whether nested conf. opts. update in a if-block
that will always be true is really necessary...

> +
>  # OpenCV component options
>  OPENCV3_CONF_OPTS += \
>         -DBUILD_DOCS=OFF \
> --
> 2.9.3
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot


Regards,

-- 
Samuel

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Thomas Petazzoni]

Hello,

On Mon, 5 Sep 2016 23:20:00 +0200, Samuel Martin wrote:

> > +# Uses __atomic_fetch_add_4
> > +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> > +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
> > +endif  
> 
> Since you add the dependency on BR2_TOOLCHAIN_HAS_ATOMIC in the
> Config.in, I wonder whether nested conf. opts. update in a if-block
> that will always be true is really necessary...

You're confusing BR2_TOOLCHAIN_HAS_ATOMIC with
BR2_TOOLCHAIN_HAS_LIBATOMIC. There's a subtle difference :)

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

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Thomas Petazzoni]

Hello,

On Mon,  5 Sep 2016 23:00:56 +0200, Bernd Kuhls wrote:

> +# Uses __atomic_fetch_add_4
> +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
> +endif

Isn't this overriding the flags already passed? Shouldn't it be:

	-DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS} -latomic"

or something like that?

Samuel?

Thanks,

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

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Samuel Martin]

On Mon, Sep 5, 2016 at 11:25 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Mon, 5 Sep 2016 23:20:00 +0200, Samuel Martin wrote:
>
>> > +# Uses __atomic_fetch_add_4
>> > +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
>> > +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
>> > +endif
>>
>> Since you add the dependency on BR2_TOOLCHAIN_HAS_ATOMIC in the
>> Config.in, I wonder whether nested conf. opts. update in a if-block
>> that will always be true is really necessary...
>
> You're confusing BR2_TOOLCHAIN_HAS_ATOMIC with
> BR2_TOOLCHAIN_HAS_LIBATOMIC. There's a subtle difference :)
>
gah! Right :-/

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



-- 
Samuel

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Samuel Martin]

On Mon, Sep 5, 2016 at 11:39 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Mon,  5 Sep 2016 23:00:56 +0200, Bernd Kuhls wrote:
>
>> +# Uses __atomic_fetch_add_4
>> +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
>> +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
>> +endif
>
> Isn't this overriding the flags already passed? Shouldn't it be:
>
>         -DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS} -latomic"
>
> or something like that?

Nope, this is correct because, by default, CMAKE_CXX_FLAGS is not set
by Buildroot (except for
host-package), and for the target ones, the toolchainfile.cmake allows
defining package specific
CMAKE_CXX_FLAGS (see [1]).

Besides, doing:
  -DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS} -latomic"
may allow take into account the CMAKE_CXX_FLAGS variable set in the
Buildroot environment; I'm
not sure we want this...

So, the original code from Bernd should behave as expected.

[1] https://github.com/buildroot/buildroot/blob/master/support/misc/toolchainfile.cmake.in#L17

Regards,

-- 
Samuel

[Buildroot] [PATCH 1/1] package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC

[Thomas Petazzoni]

Hello,

On Mon,  5 Sep 2016 23:00:56 +0200, Bernd Kuhls wrote:
> It uses __atomic_fetch_add_4 so libatomic must be pulled in if
> necessary. Fixes
> http://autobuild.buildroot.net/results/652/6521223b426c3cb4dfefab27bc6021900db6910e/
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/opencv3/Config.in  | 2 ++
>  package/opencv3/opencv3.mk | 5 +++++
>  2 files changed, 7 insertions(+)

Applied to master, thanks.

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