Skip to content

Commit

Permalink
fix: Use consistent CMake settings for all modules (#5228)
Browse files Browse the repository at this point in the history
* Resolves an issue introduced in #5111, which inadvertently removed the
  -Wno-maybe-uninitialized compiler option from some xrpl.libxrpl
  modules. This resulted in new "may be used uninitialized" build
  warnings, first noticed in the "protocol" module. When compiling with
  derr=TRUE, those warnings became errors, which made the build fail.
* Github CI actions will build with the assert and werr options turned
  on. This will cause CI jobs to fail if a developer introduces a new
  compiler warning, or causes an assert to fail in release builds.
* Includes the OS and compiler version in the linux dependencies jobs in
  the "check environment" step.
* Translates the `unity` build option into `CMAKE_UNITY_BUILD` setting.
  • Loading branch information
ximinez authored Jan 16, 2025
1 parent 7be5c31 commit 839d17e
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 37 deletions.
2 changes: 2 additions & 0 deletions .github/actions/build/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ runs:
${{ inputs.generator && format('-G "{0}"', inputs.generator) || '' }} \
-DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
-DCMAKE_BUILD_TYPE=${{ inputs.configuration }} \
-Dassert=TRUE \
-Dwerr=TRUE \
-Dtests=TRUE \
-Dxrpld=TRUE \
${{ inputs.cmake-args }} \
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/nix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ jobs:
- name: check environment
run: |
echo ${PATH} | tr ':' '\n'
lsb_release -a || true
${{ matrix.profile.cc }} --version
conan --version
cmake --version
env | sort
Expand Down
41 changes: 6 additions & 35 deletions cmake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ include(target_protobuf_sources)
# define a bunch of `static const` variables with the same names,
# so we just build them as a separate library.
add_library(xrpl.libpb)
set_target_properties(xrpl.libpb PROPERTIES UNITY_BUILD OFF)
target_protobuf_sources(xrpl.libpb xrpl/proto
LANGUAGE cpp
IMPORT_DIRS include/xrpl/proto
Expand Down Expand Up @@ -49,7 +50,9 @@ target_link_libraries(xrpl.libpb

# TODO: Clean up the number of library targets later.
add_library(xrpl.imports.main INTERFACE)
target_link_libraries(xrpl.imports.main INTERFACE

target_link_libraries(xrpl.imports.main
INTERFACE
LibArchive::LibArchive
OpenSSL::Crypto
Ripple::boost
Expand All @@ -59,7 +62,9 @@ target_link_libraries(xrpl.imports.main INTERFACE
date::date
ed25519::ed25519
secp256k1::secp256k1
xrpl.libpb
xxHash::xxhash
$<$<BOOL:${voidstar}>:antithesis-sdk-cpp>
)

include(add_module)
Expand Down Expand Up @@ -100,9 +105,6 @@ target_link_libraries(xrpl.libxrpl.server PUBLIC xrpl.libxrpl.protocol)

add_library(xrpl.libxrpl)
set_target_properties(xrpl.libxrpl PROPERTIES OUTPUT_NAME xrpl)
if(unity)
set_target_properties(xrpl.libxrpl PROPERTIES UNITY_BUILD ON)
endif()

add_library(xrpl::libxrpl ALIAS xrpl.libxrpl)

Expand Down Expand Up @@ -130,39 +132,8 @@ target_link_modules(xrpl PUBLIC
# $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
# $<INSTALL_INTERFACE:include>)

target_compile_definitions(xrpl.libxrpl
PUBLIC
BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT
BOOST_CONTAINER_FWD_BAD_DEQUE
HAS_UNCAUGHT_EXCEPTIONS=1)

target_compile_options(xrpl.libxrpl
PUBLIC
$<$<BOOL:${is_gcc}>:-Wno-maybe-uninitialized>
$<$<BOOL:${voidstar}>:-DENABLE_VOIDSTAR>
)

target_link_libraries(xrpl.libxrpl
PUBLIC
LibArchive::LibArchive
OpenSSL::Crypto
Ripple::boost
Ripple::opts
Ripple::syslibs
absl::random_random
date::date
ed25519::ed25519
secp256k1::secp256k1
xrpl.libpb
xxHash::xxhash
$<$<BOOL:${voidstar}>:antithesis-sdk-cpp>
)

if(xrpld)
add_executable(rippled)
if(unity)
set_target_properties(rippled PROPERTIES UNITY_BUILD ON)
endif()
if(tests)
target_compile_definitions(rippled PUBLIC ENABLE_TESTS)
endif()
Expand Down
7 changes: 6 additions & 1 deletion cmake/RippledInterface.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ add_library (Ripple::opts ALIAS opts)
target_compile_definitions (opts
INTERFACE
BOOST_ASIO_DISABLE_HANDLER_TYPE_REQUIREMENTS
BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT
BOOST_CONTAINER_FWD_BAD_DEQUE
HAS_UNCAUGHT_EXCEPTIONS=1
$<$<BOOL:${boost_show_deprecated}>:
BOOST_ASIO_NO_DEPRECATED
BOOST_FILESYSTEM_NO_DEPRECATED
Expand All @@ -18,10 +21,12 @@ target_compile_definitions (opts
>
$<$<BOOL:${beast_no_unit_test_inline}>:BEAST_NO_UNIT_TEST_INLINE=1>
$<$<BOOL:${beast_disable_autolink}>:BEAST_DONT_AUTOLINK_TO_WIN32_LIBRARIES=1>
$<$<BOOL:${single_io_service_thread}>:RIPPLE_SINGLE_IO_SERVICE_THREAD=1>)
$<$<BOOL:${single_io_service_thread}>:RIPPLE_SINGLE_IO_SERVICE_THREAD=1>
$<$<BOOL:${voidstar}>:ENABLE_VOIDSTAR>)
target_compile_options (opts
INTERFACE
$<$<AND:$<BOOL:${is_gcc}>,$<COMPILE_LANGUAGE:CXX>>:-Wsuggest-override>
$<$<BOOL:${is_gcc}>:-Wno-maybe-uninitialized>
$<$<BOOL:${perf}>:-fno-omit-frame-pointer>
$<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${coverage}>>:-g --coverage -fprofile-abs-path>
$<$<AND:$<BOOL:${is_clang}>,$<BOOL:${coverage}>>:-g --coverage>
Expand Down
1 change: 1 addition & 0 deletions cmake/RippledSettings.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ if(unity)
if(NOT is_ci)
set(CMAKE_UNITY_BUILD_BATCH_SIZE 15 CACHE STRING "")
endif()
set(CMAKE_UNITY_BUILD ON CACHE BOOL "Do a unity build")
endif()
if(is_clang AND is_linux)
option(voidstar "Enable Antithesis instrumentation." OFF)
Expand Down
2 changes: 1 addition & 1 deletion src/xrpld/app/paths/detail/DirectStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
if (!sle)
return QUALITY_ONE;

auto const& field = [this, qDir]() -> SF_UINT32 const& {
auto const& field = [&, this]() -> SF_UINT32 const& {
if (qDir == QualityDirection::in)
{
// compute dst quality in
Expand Down

0 comments on commit 839d17e

Please sign in to comment.