All of lore.kernel.org
 help / color / mirror / Atom feed
From: Romain Naour <romain.naour@gmail.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2] package/pulseview: fix build when linking against libatomic is needed
Date: Sat, 28 May 2016 12:19:36 +0200	[thread overview]
Message-ID: <da1f45c7-e463-8e31-e8f8-ce7ddfc3f0fe@gmail.com> (raw)
In-Reply-To: <20160526220044.13808-1-s.martin49@gmail.com>

Hi Samuel, Thomas, All,

Le 27/05/2016 ? 00:00, Samuel Martin a ?crit :
> With some toolchains, using atomics requires to explicitly add -latomic
> to the linker flags.
> 
> This change adds a patch to pulseview adding this detection and updating
> the LDFLAGS when appropriate.
> 
> This patch has be sent upstream:
>   http://article.gmane.org/gmane.comp.debugging.sigrok.devel/2097
> 
> Fixes:
>   http://autobuild.buildroot.org/results/1e3/1e3101261252d5f30fdf842cc99604e4f4c25eef/build-end.log
> 
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Romain Naour <romain.naour@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v1->v2:
> - update/rework pulseview patch
> ---
>  ...heck-for-explicit-linking-against-libatom.patch | 117 +++++++++++++++++++++
>  1 file changed, 117 insertions(+)
>  create mode 100644 package/pulseview/0002-cmake-add-check-for-explicit-linking-against-libatom.patch
> 
> diff --git a/package/pulseview/0002-cmake-add-check-for-explicit-linking-against-libatom.patch b/package/pulseview/0002-cmake-add-check-for-explicit-linking-against-libatom.patch
> new file mode 100644
> index 0000000..70549e6
> --- /dev/null
> +++ b/package/pulseview/0002-cmake-add-check-for-explicit-linking-against-libatom.patch
> @@ -0,0 +1,117 @@
> +From 71830c804be76cf6abe913ac2fe584947b7a91ea Mon Sep 17 00:00:00 2001
> +From: Samuel Martin <s.martin49@gmail.com>
> +Date: Tue, 24 May 2016 23:08:40 +0200
> +Subject: [PATCH] cmake: add check for explicit linking against libatomic
> +
> +To use atomics functions, some toolchains requires to explicitly add
> +-latomic to the linker flags (because they are not provided by libc,
> +but libatomic).
> +
> +This change adds a helper function trying to build/link a test program
> +using atomics, then calls it to:
> +* first check if atomics are directly available in the libc;
> +* if not and libatomic has been found, then run the same test with
> +  "-latomic" added to the linker flags.
> +The pulseview link library list is updated according to the results of
> +these tests.
> +
> +This issue was triggered by the Buildroot farms:
> +  http://autobuild.buildroot.org/results/1e3/1e3101261252d5f30fdf842cc99604e4f4c25eef/build-end.log
> +
> +Notes:
> +1- CMAKE_REQUIRED_* variables are only used in check functions. They
> +   are not automatically forwarded to/handled by the target commands
> +   (such as target_link_library), because the check functions are
> +   implemented as macro in CMake code, whereas many target commands
> +   are native.
> +2- Because of note #1, CMAKE_REQUIRED_LIBRARIES (or its value) must be
> +   explicitly passed to the target_link_library command when this is
> +   needed.
> +3- In this implementation, LIBATOMIC_LIBRARY is only set when it is
> +   needed; so, unconditionally appending it to PULSEVIEW_LINK_LIBS
> +   will produce the expected behavior.
> +
> +Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> +
> +---
> +changes v1->v2:
> +- use std::atomic_fetch_add_explicit function instead of
> +  __atomic_fetch_add_4;
> +- rework code using cmake_*_check_state and find_library helpers;
> +- quiet-ize checks and clean outputs
> +- extend the commit log
> +---
> + CMakeLists.txt | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> + 1 file changed, 50 insertions(+)
> +
> +diff --git a/CMakeLists.txt b/CMakeLists.txt
> +index 9dac69f..44f810e 100644
> +--- a/CMakeLists.txt
> ++++ b/CMakeLists.txt
> +@@ -107,6 +107,55 @@ endif()
> + # This will set ${CMAKE_THREAD_LIBS_INIT} to the correct, OS-specific value.
> + find_package(Threads REQUIRED)
> + 
> ++

Maybe an extra new line here :)

Otherwise:
Reviewed-by: Romain Naour <romain.naour@gmail.com>

Best regards,
Romain

> ++# Check for explicit link against libatomic
> ++#
> ++# Depending on the toolchain, linking a program using atomic functions may need
> ++# "-latomic" explicitly passed to the linker
> ++#
> ++# This check first tests if atomics are available in the C-library, if not and
> ++# libatomic exists, then it runs the same test with -latomic added to the
> ++# linker flags.
> ++
> ++# Helper for checking for atomics
> ++function(check_working_cxx_atomics varname additional_lib)
> ++  include(CheckCXXSourceCompiles)
> ++  include(CMakePushCheckState)
> ++  cmake_push_check_state()
> ++  set(CMAKE_REQUIRED_FLAGS "-std=c++11")
> ++  set(CMAKE_REQUIRED_LIBRARIES "${additional_lib}")
> ++  set(CMAKE_REQUIRED_QUIET 1)
> ++  CHECK_CXX_SOURCE_COMPILES("
> ++#include <atomic>
> ++std::atomic<int> x;
> ++int main() {
> ++  return std::atomic_fetch_add_explicit(&x, 1, std::memory_order_seq_cst);
> ++}
> ++" ${varname})
> ++  cmake_pop_check_state()
> ++endfunction(check_working_cxx_atomics)
> ++
> ++# First check if atomics work without the library.
> ++# If not, check if the library exists, and atomics work with it.
> ++check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITHOUT_LIB "")
> ++if(HAVE_CXX_ATOMICS_WITHOUT_LIB)
> ++  message(STATUS "Atomics provided by the C-library - yes")
> ++else()
> ++  message(STATUS "Atomics provided by the C-library - no")
> ++  find_library(LIBATOMIC_LIBRARY NAMES atomic PATH_SUFFIXES lib)
> ++  if(LIBATOMIC_LIBRARY)
> ++    check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITH_LIB "${LIBATOMIC_LIBRARY}")
> ++    if (HAVE_CXX_ATOMICS_WITH_LIB)
> ++      message(STATUS "Atomics provided by libatomic - yes")
> ++    else()
> ++      message(STATUS "Atomics provided by libatomic - no")
> ++      message(FATAL_ERROR "Compiler must support std::atomic!")
> ++    endif()
> ++  else()
> ++    message(FATAL_ERROR "Compiler appears to require libatomic, but cannot find it.")
> ++  endif()
> ++endif()
> ++
> + #===============================================================================
> + #= System Introspection
> + #-------------------------------------------------------------------------------
> +@@ -387,6 +436,7 @@ set(PULSEVIEW_LINK_LIBS
> + 	${Boost_LIBRARIES}
> + 	${QT_LIBRARIES}
> + 	${CMAKE_THREAD_LIBS_INIT}
> ++	${LIBATOMIC_LIBRARY}
> + )
> + 
> + if(STATIC_PKGDEPS_LIBS)
> +-- 
> +2.8.3
> +
> 

  reply	other threads:[~2016-05-28 10:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-26 22:00 [Buildroot] [PATCH v2] package/pulseview: fix build when linking against libatomic is needed Samuel Martin
2016-05-28 10:19 ` Romain Naour [this message]
2016-05-29 21:12 ` Peter Korsgaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=da1f45c7-e463-8e31-e8f8-ce7ddfc3f0fe@gmail.com \
    --to=romain.naour@gmail.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.