Skip to contentSkip to conversation

pulseview: fix build failures

PATCH New Bartosz Golaszewski 3 participants 8 messages Jun 30, 2015 – Jun 13, 2016

lore
Share & tools
lore
#0 lore raw

cover pulseview: fix build failures

Autobuilders frequently report the following issue with pulseview:
http://autobuild.buildroot.net/results/993/993622312239fe56946fdde4d3dfec73786393e5/build-end.log

It's caused by passing a nullptr as argument where boost::optional<std::pair<(...)> > is expected. Commit ec6cc07f fixed the issue in PulseView and the second patch makes buildroot use it when building PV. The first patch is needed too due to some API changes in libsigrokcxx.

NOTE: this builds fine on x86 and actually also used to build properly on other architectures at commit edbeeef. Something must have changed along the way that exposed the bug.

Bartosz Golaszewski (2):
libsigrok: bump commit hash
pulseview: bump commit hash

 package/libsigrok/libsigrok.mk | 2 +-
 package/pulseview/pulseview.mk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/2 libsigrok: bump commit hash

---
 package/libsigrok/libsigrok.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/libsigrok/libsigrok.mk +1 -1
index 21d8259..54d0482 100644
@@ -4,7 +4,7 @@
 #
 ################################################################################
 
-LIBSIGROK_VERSION = 55462b8ba9f48931de9b9a7bfbc182a1d113b3b5
+LIBSIGROK_VERSION = fe473123ba836445d477149f2e2c0ae372fc0c4c
 # No https access on upstream git
 LIBSIGROK_SITE = git://sigrok.org/libsigrok
 LIBSIGROK_LICENSE = GPLv3+
Signature · 1 line
2.1.4
#2 lore raw

PATCH 2/2 pulseview: bump commit hash

In reply to Bartosz Golaszewski

Include commit ec6cc07f which fixes an issue causing build failures on several architectures.

---
 package/pulseview/pulseview.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pulseview/pulseview.mk +1 -1
index 1c0e819..67f293f 100644
@@ -8,7 +8,7 @@
 
 # No https access on upstream git
 PULSEVIEW_SITE = git://sigrok.org/pulseview
-PULSEVIEW_VERSION = 198f9bc74d6c955a72e3e3282c2b0ba16ef39a62
+PULSEVIEW_VERSION = ec6cc07fed12f5070eee6b8cb11343e83d42533c
 PULSEVIEW_LICENSE = GPLv3+
 PULSEVIEW_LICENSE_FILES = COPYING
 PULSEVIEW_DEPENDENCIES = libsigrok qt5base qt5svg boost
Signature · 1 line
2.1.4
#3 lore raw

cover pulseview: fix build failures

In reply to Bartosz Golaszewski

Dear Bartosz Golaszewski,

Quoted 3 lines · Bartosz Golaszewski

Bartosz Golaszewski (2):
libsigrok: bump commit hash
pulseview: bump commit hash

Both applied, thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

cover pulseview: fix build failures

In reply to Bartosz Golaszewski

Hello Bartosz,

Quoted 9 lines · Bartosz Golaszewski

Autobuilders frequently report the following issue with pulseview:
http://autobuild.buildroot.net/results/993/993622312239fe56946fdde4d3dfec73786393e5/build-end.log

It's caused by passing a nullptr as argument where boost::optional<std::pair<(...)> > is expected. Commit ec6cc07f fixed the issue in PulseView and the second patch makes buildroot use it when building PV. The first patch is needed too due to some API changes in libsigrokcxx.

Quoted 3 lines

NOTE: this builds fine on x86 and actually also used to build properly on other architectures at commit edbeeef. Something must have changed along the way that exposed the bug.

We are still seeing some build issue with the pulseview package, on the SPARC architecture:

http://autobuild.buildroot.org/results/1e3/1e3101261252d5f30fdf842cc99604e4f4c25eef/build-end.log

On SPARC, when the __atomic intrinsics are used, you must link with the libatomic library (which is part of the toolchain). When you take the pulseview link command line, and just add "-latomic", it links fine. So I've tried to add some CMake logic to have pulseview link against libatomic when needed. This CMake logic is inspired (copied) from LLVM, and looks like this:

include(CheckCXXSourceCompiles)

function(check_working_cxx_atomics varname)
set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) set(CMAKE_REQUIRED_FLAGS "-std=c++11") CHECK_CXX_SOURCE_COMPILES("
#include <atomic>
std::atomic<int> x;
int main() {
return x;
}
" ${varname})
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
endfunction(check_working_cxx_atomics)

# First check if atomics work without the library. check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITHOUT_LIB)
# If not, check if the library exists, and atomics work with it.
if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB)
check_library_exists(atomic __atomic_fetch_add_4 "" HAVE_LIBATOMIC)
if( HAVE_LIBATOMIC )
list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic") check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITH_LIB)
if (NOT HAVE_CXX_ATOMICS_WITH_LIB)
message(FATAL_ERROR "Host compiler must support std::atomic!")
endif()
else()
message(FATAL_ERROR "Host compiler appears to require libatomic, but cannot find it.")
endif()
endif()

Unfortunately, for some reason the sample code in this check builds just fine without -latomic, and therefore HAVE_CXX_ATOMICS_WITHOUT_LIB gets defined and -latomic is not added to the libraries to link with.

Could you investigate this issue, and see which CMake check can be written to make sure we link against libatomic when needed?

Here is a minimal defconfig that reproduces the problem:

BR2_sparc=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-sparc-uclibc-2016.05-rc1-2-g44038df.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_14=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_QT5=y
BR2_PACKAGE_QT5BASE_LICENSE_APPROVED=y
BR2_PACKAGE_PULSEVIEW=y
# BR2_TARGET_ROOTFS_TAR is not set

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

cover pulseview: fix build failures

Hi Thomas, all,

Quoted 86 lines · Thomas Petazzoni

Hello Bartosz,

Quoted 9 lines · Bartosz Golaszewski

Autobuilders frequently report the following issue with pulseview:
http://autobuild.buildroot.net/results/993/993622312239fe56946fdde4d3dfec73786393e5/build-end.log

It's caused by passing a nullptr as argument where boost::optional<std::pair<(...)> > is expected. Commit ec6cc07f fixed the issue in PulseView and the second patch makes buildroot use it when building PV. The first patch is needed too due to some API changes in libsigrokcxx.

Quoted 3 lines

NOTE: this builds fine on x86 and actually also used to build properly on other architectures at commit edbeeef. Something must have changed along the way that exposed the bug.

We are still seeing some build issue with the pulseview package, on the SPARC architecture:

http://autobuild.buildroot.org/results/1e3/1e3101261252d5f30fdf842cc99604e4f4c25eef/build-end.log

On SPARC, when the __atomic intrinsics are used, you must link with the libatomic library (which is part of the toolchain). When you take the pulseview link command line, and just add "-latomic", it links fine. So I've tried to add some CMake logic to have pulseview link against libatomic when needed. This CMake logic is inspired (copied) from LLVM, and looks like this:

include(CheckCXXSourceCompiles)

function(check_working_cxx_atomics varname)
set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) set(CMAKE_REQUIRED_FLAGS "-std=c++11") CHECK_CXX_SOURCE_COMPILES("
#include <atomic>
std::atomic<int> x;
int main() {
return x;
}
" ${varname})
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
endfunction(check_working_cxx_atomics)

# First check if atomics work without the library. check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITHOUT_LIB)
# If not, check if the library exists, and atomics work with it.
if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB)
check_library_exists(atomic __atomic_fetch_add_4 "" HAVE_LIBATOMIC)
if( HAVE_LIBATOMIC )
list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic") check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITH_LIB)
if (NOT HAVE_CXX_ATOMICS_WITH_LIB)
message(FATAL_ERROR "Host compiler must support std::atomic!")
endif()
else()
message(FATAL_ERROR "Host compiler appears to require libatomic, but cannot find it.")
endif()
endif()

Unfortunately, for some reason the sample code in this check builds just fine without -latomic, and therefore HAVE_CXX_ATOMICS_WITHOUT_LIB gets defined and -latomic is not added to the libraries to link with.

Could you investigate this issue, and see which CMake check can be written to make sure we link against libatomic when needed?

Here is a minimal defconfig that reproduces the problem:

BR2_sparc=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-sparc-uclibc-2016.05-rc1-2-g44038df.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_14=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_QT5=y
BR2_PACKAGE_QT5BASE_LICENSE_APPROVED=y
BR2_PACKAGE_PULSEVIEW=y
# BR2_TARGET_ROOTFS_TAR is not set

Quoted 1 line

Thanks a lot!

Thanks a lot for these materials, it helped me a lot cooking a working patch [1].

I still need to polish it, and I'm also wondering whether __atomic_fetch_add_4 is available on all architectures that have atomic support?
(sorry for this silly question, I don't know much about atomic support, and their architecture dependency).
If __atomic_fetch_add_4 is ok, then I'll post the patch shortly.

[1] http://code.bulix.org/f3zjiv-99265?raw

Regards,

Signature · 1 line
Samuel
#6 lore raw

cover pulseview: fix build failures

In reply to Thomas Petazzoni

2016-05-24 22:10 GMT+02:00 Thomas Petazzoni
<[email protected]>:

Quoted 86 lines

Hello Bartosz,

Quoted 9 lines · Bartosz Golaszewski

Autobuilders frequently report the following issue with pulseview:
http://autobuild.buildroot.net/results/993/993622312239fe56946fdde4d3dfec73786393e5/build-end.log

It's caused by passing a nullptr as argument where boost::optional<std::pair<(...)> > is expected. Commit ec6cc07f fixed the issue in PulseView and the second patch makes buildroot use it when building PV. The first patch is needed too due to some API changes in libsigrokcxx.

Quoted 3 lines

NOTE: this builds fine on x86 and actually also used to build properly on other architectures at commit edbeeef. Something must have changed along the way that exposed the bug.

We are still seeing some build issue with the pulseview package, on the SPARC architecture:

http://autobuild.buildroot.org/results/1e3/1e3101261252d5f30fdf842cc99604e4f4c25eef/build-end.log

On SPARC, when the __atomic intrinsics are used, you must link with the libatomic library (which is part of the toolchain). When you take the pulseview link command line, and just add "-latomic", it links fine. So I've tried to add some CMake logic to have pulseview link against libatomic when needed. This CMake logic is inspired (copied) from LLVM, and looks like this:

include(CheckCXXSourceCompiles)

function(check_working_cxx_atomics varname)
set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) set(CMAKE_REQUIRED_FLAGS "-std=c++11") CHECK_CXX_SOURCE_COMPILES("
#include <atomic>
std::atomic<int> x;
int main() {
return x;
}
" ${varname})
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
endfunction(check_working_cxx_atomics)

# First check if atomics work without the library. check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITHOUT_LIB)
# If not, check if the library exists, and atomics work with it.
if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB)
check_library_exists(atomic __atomic_fetch_add_4 "" HAVE_LIBATOMIC)
if( HAVE_LIBATOMIC )
list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic") check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITH_LIB)
if (NOT HAVE_CXX_ATOMICS_WITH_LIB)
message(FATAL_ERROR "Host compiler must support std::atomic!")
endif()
else()
message(FATAL_ERROR "Host compiler appears to require libatomic, but cannot find it.")
endif()
endif()

Unfortunately, for some reason the sample code in this check builds just fine without -latomic, and therefore HAVE_CXX_ATOMICS_WITHOUT_LIB gets defined and -latomic is not added to the libraries to link with.

Could you investigate this issue, and see which CMake check can be written to make sure we link against libatomic when needed?

Here is a minimal defconfig that reproduces the problem:

BR2_sparc=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-sparc-uclibc-2016.05-rc1-2-g44038df.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_14=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_QT5=y
BR2_PACKAGE_QT5BASE_LICENSE_APPROVED=y
BR2_PACKAGE_PULSEVIEW=y
# BR2_TARGET_ROOTFS_TAR is not set

Quoted 7 lines

Thanks a lot!

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

Hello Thomas,

I'm sorry for not responding before, but I was busy and lost this e-mail somehow. I see it's fixed now with commit b385d13 by Samuel. Let me know if there are still some issues with PulseView or any sigrok packages.

Best regards,
Bartosz Golaszewski

#7 lore raw

cover pulseview: fix build failures

Hi all,

On Mon, Jun 13, 2016 at 9:19 AM, Bartosz Golaszewski <[email protected]> wrote:
[...]

Quoted 6 lines

Hello Thomas,

I'm sorry for not responding before, but I was busy and lost this e-mail somehow. I see it's fixed now with commit b385d13 by Samuel. Let me know if there are still some issues with PulseView or any sigrok packages.

FYI, the patch got merged upstream:
https://sourceforge.net/p/sigrok/mailman/message/35154129/

Regards,

Signature · 1 line
Samuel