Skip to content

Commit

Permalink
Merge #50: cmake: Add libatomic check
Browse files Browse the repository at this point in the history
b2cfe67 ci: Test CMake edge cases (Hennadii Stepanov)
ee2ac14 fixup! cmake: Redefine configuration flags (Hennadii Stepanov)
7f2d751 fixup! cmake: Check system symbols (Hennadii Stepanov)
b8a01cf fixup! cmake: Check system symbols (Hennadii Stepanov)
1340921 fixup! cmake: Check system symbols (Hennadii Stepanov)
4b198e4 fixup! cmake: Warn about not encapsulated build properties (Hennadii Stepanov)
c92c4c1 fixup! cmake: Warn about not encapsulated build properties (Hennadii Stepanov)

Pull request description:

  1. Added an analogue of https://github.com/bitcoin/bitcoin/blob/master/build-aux/m4/l_atomic.m4.
  2. Included changes from bitcoin#28777.
  3. Added (temporarily?) a test case to the GHA CI to avoid regressions in the future.

  Might be tested, for example, as follows:
  ```
  make -j $(nproc) -C depends HOST=i686-pc-linux-gnu CC="clang -m32" CXX="clang++ -m32" NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_NATPMP=1 NO_UPNP=1 NO_USDT=1
  cmake -B build --toolchain depends/i686-pc-linux-gnu/share/toolchain.cmake
  cmake --build build -j $(nproc) -t bitcoind
  ```

  Here is an excerpt from the CI [log](https://github.com/hebasto/bitcoin/actions/runs/6842095586/job/18603196018):
  ```
  -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
  -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Failed
  -- Performing Test STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC
  -- Performing Test STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC - Success
  ```

Top commit has no ACKs.

Tree-SHA512: 3af1524dc0f8e06c8b1f5398417c3d53cd7d1fb41ffd6602407b6c79d7d891784447832f443434998b524b9a7c3360ff07c5396d660aa6f9185718fa2af4a3e8
  • Loading branch information
hebasto committed Nov 15, 2023
2 parents 466bc36 + b2cfe67 commit 6174485
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 41 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:
- '**'

concurrency:
group: ${{ github.event_name != 'pull_request' && github.run_id || github.ref }}
group: ${{ github.workflow }}${{ github.event_name != 'pull_request' && github.run_id || github.ref }}
cancel-in-progress: true

env:
Expand Down
99 changes: 99 additions & 0 deletions .github/workflows/cmake.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

name: CMake
on:
# See: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request.
pull_request:
# See: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#push.
push:
branches:
- '**'
tags-ignore:
- '**'

concurrency:
group: ${{ github.workflow }}${{ github.event_name != 'pull_request' && github.run_id || github.ref }}
cancel-in-progress: true

jobs:
cross-build:
name: ${{ matrix.host.name }}
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
host:
- name: 'Linux 32-bit, Clang, link to libatomic'
packages: 'clang-14 g++-multilib'
triplet: 'i686-pc-linux-gnu'
c_compiler: 'clang-14 -m32'
cxx_compiler: 'clang++-14 -m32'
depends_options: 'NO_QT=1'
configure_options: '-DWERROR=ON'

env:
CCACHE_MAXSIZE: '120M'

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install dependency packages
run: |
sudo apt-get update
sudo apt-get install --no-install-recommends ccache ${{ matrix.host.packages }}
echo "CCACHE_DIR=${{ runner.temp }}/ccache" >> "$GITHUB_ENV"
- name: Depends fingerprint
id: depends_fingerprint
run: |
${{ matrix.host.c_compiler }} -v 2>&1 | tee depends/c_compiler_version
${{ matrix.host.cxx_compiler }} -v 2>&1 | tee depends/cxx_compiler_version
echo ${{ matrix.host.depends_options }} > depends/depends_options
echo "hash=${{ hashFiles('./depends/**') }}" >> "$GITHUB_OUTPUT"
- name: Depends cache
id: depends_cache
uses: actions/cache@v3
with:
path: |
depends/built
key: ${{ matrix.host.triplet }}-depends-${{ steps.depends_fingerprint.outputs.hash }}

- name: Build depends
run: |
cd depends
make -j$(nproc) HOST=${{ matrix.host.triplet }} CC='${{ matrix.host.c_compiler }}' CXX='${{ matrix.host.cxx_compiler }}' ${{ matrix.host.depends_options }} LOG=1
- name: Restore Ccache cache
uses: actions/cache/restore@v3
id: ccache-cache
with:
path: ${{ env.CCACHE_DIR }}
key: ${{ matrix.host.triplet }}-ccache-${{ github.run_id }}
restore-keys: ${{ matrix.host.triplet }}-ccache-

- name: Generate build system
run: |
cmake -B build --toolchain depends/${{ matrix.host.triplet }}/share/toolchain.cmake ${{ matrix.host.configure_options }}
- name: Build
run: |
ccache --zero-stats
cmake --build build -j $(nproc)
ccache --version | head -n 1 && ccache --show-stats
file build/src/bitcoind
- name: Save Ccache cache
uses: actions/cache/save@v3
if: github.event_name != 'pull_request' && steps.ccache-cache.outputs.cache-hit != 'true'
with:
path: ${{ env.CCACHE_DIR }}
key: ${{ matrix.host.triplet }}-ccache-${{ github.run_id }}

- name: Check
run: |
ctest --test-dir build -j $(nproc)
13 changes: 1 addition & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -382,15 +382,4 @@ if(configure_warnings)
endif()

# We want all build properties to be encapsulated properly.
get_directory_property(global_compile_definitions COMPILE_DEFINITIONS)
if(global_compile_definitions)
message(AUTHOR_WARNING "The directory's COMPILE_DEFINITIONS property is not empty: ${global_compile_definitions}")
endif()
get_directory_property(global_compile_options COMPILE_OPTIONS)
if(global_compile_options)
message(AUTHOR_WARNING "The directory's COMPILE_OPTIONS property is not empty: ${global_compile_options}")
endif()
get_directory_property(global_link_options LINK_OPTIONS)
if(global_link_options)
message(AUTHOR_WARNING "The directory's LINK_OPTIONS property is not empty: ${global_link_options}")
endif()
include(WarnAboutGlobalProperties)
27 changes: 5 additions & 22 deletions cmake/introspection.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -75,32 +75,15 @@ check_include_file_cxx(ifaddrs.h HAVE_IFADDRS_H)
if(HAVE_SYS_TYPES_H AND HAVE_IFADDRS_H)
check_cxx_symbol_exists(freeifaddrs "sys/types.h;ifaddrs.h" HAVE_DECL_FREEIFADDRS)
check_cxx_symbol_exists(getifaddrs "sys/types.h;ifaddrs.h" HAVE_DECL_GETIFADDRS)
# Illumos/SmartOS requires linking with -lsocket if
# using getifaddrs & freeifaddrs.
# See: https://github.com/bitcoin/bitcoin/pull/21486
if(HAVE_DECL_GETIFADDRS AND HAVE_DECL_FREEIFADDRS)
set(check_socket_source "
#include <sys/types.h>
#include <ifaddrs.h>
int main() {
struct ifaddrs* ifaddr;
getifaddrs(&ifaddr);
freeifaddrs(ifaddr);
}
")
check_cxx_source_links("${check_socket_source}" IFADDR_LINKS_WITHOUT_LIBSOCKET)
if(NOT IFADDR_LINKS_WITHOUT_LIBSOCKET)
check_cxx_source_links_with_libs(socket "${check_socket_source}" IFADDR_NEEDS_LINK_TO_LIBSOCKET)
if(IFADDR_NEEDS_LINK_TO_LIBSOCKET)
link_libraries(socket)
else()
message(FATAL_ERROR "Cannot figure out how to use getifaddrs/freeifaddrs.")
endif()
endif()
include(TestAppendRequiredLibraries)
test_append_socket_library(core)
endif()
endif()

include(TestAppendRequiredLibraries)
test_append_atomic_library(core)

# Check for gmtime_r(), fallback to gmtime_s() if that is unavailable.
# Fail if neither are available.
check_cxx_source_compiles("
Expand Down
8 changes: 4 additions & 4 deletions cmake/module/CheckSourceCompilesAndLinks.cmake
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

include(CheckCXXSourceCompiles)
include(CMakePushCheckState)
Expand All @@ -16,14 +16,14 @@ endmacro()

macro(check_cxx_source_compiles_with_flags flags source)
cmake_push_check_state(RESET)
string(REPLACE ";" " " CMAKE_REQUIRED_FLAGS "${flags}")
list(JOIN flags " " CMAKE_REQUIRED_FLAGS)
check_cxx_source_compiles("${source}" ${ARGN})
cmake_pop_check_state()
endmacro()

macro(check_cxx_source_links_with_flags flags source)
cmake_push_check_state(RESET)
string(REPLACE ";" " " CMAKE_REQUIRED_FLAGS "${flags}")
list(JOIN flags " " CMAKE_REQUIRED_FLAGS)
check_cxx_source_links("${source}" ${ARGN})
cmake_pop_check_state()
endmacro()
Expand Down
6 changes: 4 additions & 2 deletions cmake/module/ProcessConfigurations.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,10 @@ function(get_target_interface var target property)
get_target_property(dependencies ${target} INTERFACE_LINK_LIBRARIES)
if(dependencies)
foreach(dependency IN LISTS dependencies)
get_target_interface(dep_result ${dependency} ${property})
string(STRIP "${result} ${dep_result}" result)
if(TARGET ${dependency})
get_target_interface(dep_result ${dependency} ${property})
string(STRIP "${result} ${dep_result}" result)
endif()
endforeach()
endif()

Expand Down
77 changes: 77 additions & 0 deletions cmake/module/TestAppendRequiredLibraries.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

include_guard(GLOBAL)

# Illumos/SmartOS requires linking with -lsocket if
# using getifaddrs & freeifaddrs.
# See: https://github.com/bitcoin/bitcoin/pull/21486
function(test_append_socket_library target)
set(check_socket_source "
#include <sys/types.h>
#include <ifaddrs.h>
int main() {
struct ifaddrs* ifaddr;
getifaddrs(&ifaddr);
freeifaddrs(ifaddr);
}
")

include(CheckSourceCompilesAndLinks)
check_cxx_source_links("${check_socket_source}" IFADDR_LINKS_WITHOUT_LIBSOCKET)
if(IFADDR_LINKS_WITHOUT_LIBSOCKET)
return()
endif()

check_cxx_source_links_with_libs(socket "${check_socket_source}" IFADDR_NEEDS_LINK_TO_LIBSOCKET)
if(IFADDR_NEEDS_LINK_TO_LIBSOCKET)
target_link_libraries(${target} INTERFACE socket)
else()
message(FATAL_ERROR "Cannot figure out how to use getifaddrs/freeifaddrs.")
endif()
endfunction()

# Clang prior to version 15, when building for 32-bit,
# and linking against libstdc++, requires linking with
# -latomic if using the C++ atomic library.
# Can be tested with: clang++ test.cpp -m32
#
# Sourced from http://bugs.debian.org/797228
function(test_append_atomic_library target)
set(check_atomic_source "
#include <atomic>
#include <cstdint>
#include <chrono>
using namespace std::chrono_literals;
int main() {
std::atomic<bool> lock{true};
lock.exchange(false);
std::atomic<std::chrono::seconds> t{0s};
t.store(2s);
std::atomic<int64_t> a{};
int64_t v = 5;
int64_t r = a.fetch_add(v);
return static_cast<int>(r);
}
")

include(CheckSourceCompilesAndLinks)
check_cxx_source_links("${check_atomic_source}" STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC)
if(STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC)
return()
endif()

check_cxx_source_links_with_libs(atomic "${check_atomic_source}" STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC)
if(STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC)
target_link_libraries(${target} INTERFACE atomic)
else()
message(FATAL_ERROR "Cannot figure out how to use std::atomic.")
endif()
endfunction()
35 changes: 35 additions & 0 deletions cmake/module/WarnAboutGlobalProperties.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

include_guard(GLOBAL)

# Avoid the directory-wide add_definitions() and add_compile_definitions() commands.
# Instead, prefer the target-specific target_compile_definitions() one.
get_directory_property(global_compile_definitions COMPILE_DEFINITIONS)
if(global_compile_definitions)
message(AUTHOR_WARNING "The directory's COMPILE_DEFINITIONS property is not empty: ${global_compile_definitions}")
endif()

# Avoid the directory-wide add_compile_options() command.
# Instead, prefer the target-specific target_compile_options() one.
get_directory_property(global_compile_options COMPILE_OPTIONS)
if(global_compile_options)
message(AUTHOR_WARNING "The directory's COMPILE_OPTIONS property is not empty: ${global_compile_options}")
endif()

# Avoid the directory-wide add_link_options() command.
# Instead, prefer the target-specific target_link_options() one.
get_directory_property(global_link_options LINK_OPTIONS)
if(global_link_options)
message(AUTHOR_WARNING "The directory's LINK_OPTIONS property is not empty: ${global_link_options}")
endif()

# Avoid the directory-wide link_libraries() command.
# Instead, prefer the target-specific target_link_libraries() one.
file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/dummy_cxx_source.cpp "#error")
add_library(check_loose_linked_libraries OBJECT EXCLUDE_FROM_ALL ${CMAKE_CURRENT_BINARY_DIR}/dummy_cxx_source.cpp)
get_target_property(global_linked_libraries check_loose_linked_libraries LINK_LIBRARIES)
if(global_linked_libraries)
message(AUTHOR_WARNING "There are libraries linked with `link_libraries` commands: ${global_linked_libraries}")
endif()

0 comments on commit 6174485

Please sign in to comment.