From 0d89cd8d0a73247060386e2678bec5ef910820a8 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 4 Jun 2025 11:05:38 +0800 Subject: [PATCH] cmake: enable out-of-source build of breakpad Previously, Breakpad was built in its source tree instead of the user-specified build directory, inconsistent with other external projects and potentially causing source tree pollution. Include path fix: - Add ${INSTALL_DIR}/include/breakpad to include directories to fix FTBFS on Jammy builders Build system improvements: - Replace dedicated LSS submodule symlink target with PATCH_COMMAND to simplify the build process - Use user-specified make command instead of hardcoded "make" - Skip building unused process library and tools - Link against breakpad with PRIVATE visibility unless required Compiler flag cleanups: - Remove -Wno-array-bounds from CFLAGS (Breakpad uses C++/CXXFLAGS) - Remove compile-time flags incorrectly placed in LDFLAGS - Remove '-fPIC' from CFLAGS, as it is already included by breakpad when building on linux hosts. - Replace the individual -Wno-* flags with -Wno-error to cancel -Werror option specified by breakpad. This is more future-proof. CMake target modernization: - Rename libbreakpad_client to Breakpad::client following modern conventions - Add Breakpad::breakpad header-only target to minimize dependencies - Install library to enable proper include path prefixes (breakpad/client/... vs client/...) Header dependency optimization: - Remove Breakpad includes from popular headers, use forward declarations - Include Breakpad headers before internal headers for better readability Signed-off-by: Kefu Chai --- src/CMakeLists.txt | 97 +++++++++++++----------- src/common/CMakeLists.txt | 6 +- src/common/ceph_context.cc | 4 + src/common/ceph_context.h | 9 ++- src/crimson/CMakeLists.txt | 2 +- src/crimson/os/alienstore/CMakeLists.txt | 3 + src/global/CMakeLists.txt | 4 + src/global/global_init.cc | 10 ++- src/rgw/CMakeLists.txt | 5 -- src/test/CMakeLists.txt | 2 +- 10 files changed, 82 insertions(+), 60 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3a52ba047c7f4..1e6ce38037c44 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -327,46 +327,62 @@ endif(WITH_BLKIN) ## breakpad if(WITH_BREAKPAD) + include(FindMake) + find_make("MAKE_EXECUTABLE" "make_cmd") + set(breakpad_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/breakpad) set(lss_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/lss) - add_custom_target(breakpad_lss_symlink) - add_custom_command( - TARGET breakpad_lss_symlink - PRE_BUILD - COMMAND ${CMAKE_COMMAND} -E create_symlink ${lss_SOURCE_DIR} ${breakpad_SOURCE_DIR}/src/third_party/lss - COMMENT "Creating symbolic link lss -> breakpad third party" - ) ExternalProject_Add( breakpad_project SOURCE_DIR "${breakpad_SOURCE_DIR}" - CONFIGURE_COMMAND cd "${breakpad_SOURCE_DIR}" - COMMAND "${breakpad_SOURCE_DIR}/configure" - "CC=${CMAKE_C_COMPILER}" - "CXX=${CMAKE_CXX_COMPILER}" - "CFLAGS=${CMAKE_C_FLAGS} -fPIC -Wno-unknown-warning-option -Wno-array-bounds -Wno-maybe-uninitialized" - "CXXFLAGS=${CMAKE_CXX_FLAGS} -fPIC -Wno-unknown-warning-option -Wno-ignored-qualifiers -Wno-array-bounds -Wno-maybe-uninitialized" - "LDFLAGS=${CMAKE_EXE_LINKER_FLAGS} -fPIC -Wno-unknown-warning-option -Wno-array-bounds -Wno-maybe-uninitialized" - BUILD_COMMAND - /bin/sh -cx "cd ${breakpad_SOURCE_DIR} && make" - INSTALL_COMMAND "" + PATCH_COMMAND ${CMAKE_COMMAND} -E create_symlink ${lss_SOURCE_DIR} ${breakpad_SOURCE_DIR}/src/third_party/lss + # the minidump processor is used when processing the minidump, we only + # use the breakpad's client for generating the minidump at this moment. + # + # also cancel the -Werror used by breakpad, as new compilers are more + # picky, and might fail the build because of warnings. + CONFIGURE_COMMAND ${breakpad_SOURCE_DIR}/configure --disable-processor --disable-tools --prefix= + "CC=${CMAKE_C_COMPILER}" + "CFLAGS=${CMAKE_C_FLAGS} -fPIC" + "CXX=${CMAKE_CXX_COMPILER}" + "CXXFLAGS=${CMAKE_CXX_FLAGS} -fPIC -Wno-error" + BUILD_COMMAND ${make_cmd} + # Install this library to ensure headers are included with proper prefixed paths + # (e.g., "breakpad/client/linux/handler/minidump_descriptor.h") rather than + # unprefixed paths (e.g., "client/linux/handler/minidump_descriptor.h"). + # Prefixed paths make the library dependency explicit and avoid ambiguity. + # + # Note: DESTDIR is unset to prevent Debian packaging from overriding install + # paths, which would break header/library discovery during build. This is safe + # since breakpad artifacts are not redistributed in the final package. + INSTALL_COMMAND ${CMAKE_COMMAND} -E env --unset=DESTDIR ${make_cmd} install UPDATE_DISCONNECTED ON - BUILD_IN_SOURCE ON - DEPENDS breakpad_lss_symlink - BUILD_BYPRODUCTS "${breakpad_SOURCE_DIR}/src/libbreakpad.a;${breakpad_SOURCE_DIR}/src/client/linux/libbreakpad_client.a" - ) - - add_library(libbreakpad STATIC IMPORTED GLOBAL) - set_property(TARGET libbreakpad PROPERTY IMPORTED_LOCATION ${breakpad_SOURCE_DIR}/src/libbreakpad.a) - add_library(libbreakpad_client STATIC IMPORTED GLOBAL) - set_property(TARGET libbreakpad_client PROPERTY IMPORTED_LOCATION ${breakpad_SOURCE_DIR}/src/client/linux/libbreakpad_client.a) + BUILD_BYPRODUCTS "/lib/libbreakpad_client.a") - include_directories(SYSTEM "${breakpad_SOURCE_DIR}/src") - add_dependencies(libbreakpad breakpad_project) - add_dependencies(libbreakpad_client breakpad_project) - - add_library(breakpad INTERFACE) - target_link_libraries(breakpad INTERFACE libbreakpad libbreakpad_client) + ExternalProject_Get_Property(breakpad_project INSTALL_DIR) + # create the directory so cmake won't complain when looking at the imported + # target + file(MAKE_DIRECTORY ${INSTALL_DIR}/include/breakpad) + + add_library(Breakpad::client STATIC IMPORTED GLOBAL) + add_dependencies(Breakpad::client breakpad_project) + set_target_properties(Breakpad::client PROPERTIES + # unfortunately, breakpad's public headers use internal include paths + # (e.g., "client/linux/..." rather than the installed include path + # (e.g., "breakpad/client/linux/..."), because the library's source + # structure differs between build time and install time. so we have to + # add ${INSTALL_DIR}/include/breakpad as well. + INTERFACE_INCLUDE_DIRECTORIES "${INSTALL_DIR}/include;${INSTALL_DIR}/include/breakpad" + IMPORTED_LINK_INTERFACE_LANGUAGES "CXX" + IMPORTED_LOCATION "${INSTALL_DIR}/lib/libbreakpad_client.a") + + # for header-only library + add_library(Breakpad::breakpad INTERFACE IMPORTED GLOBAL) + add_dependencies(Breakpad::breakpad breakpad_project) + set_target_properties(Breakpad::breakpad PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${INSTALL_DIR}/include" + IMPORTED_LINK_INTERFACE_LANGUAGES "CXX") endif(WITH_BREAKPAD) if(WITH_JAEGER) @@ -525,11 +541,6 @@ if(WITH_JAEGER) target_link_libraries(common-objs jaeger_base) endif() -if(WITH_BREAKPAD) - add_dependencies(common-objs breakpad_project) - target_link_libraries(common-objs breakpad) -endif() - CHECK_C_COMPILER_FLAG("-fvar-tracking-assignments" HAS_VTA) add_subdirectory(auth) add_subdirectory(common) @@ -591,10 +602,6 @@ if(WITH_JAEGER) list(APPEND ceph_common_deps jaeger_base) endif() -if(WITH_BREAKPAD) - list(APPEND ceph_common_deps breakpad) -endif() - if(WIN32) list(APPEND ceph_common_deps ws2_32 mswsock iphlpapi bcrypt) list(APPEND ceph_common_deps dlfcn_win32) @@ -623,6 +630,10 @@ if(WITH_BLUESTORE_PMEM OR WITH_RBD_RWL) endif() endif() +if(WITH_BREAKPAD) + list(APPEND ceph_common_deps Breakpad::client) +endif() + add_library(common STATIC ${ceph_common_objs}) target_link_libraries(common ${ceph_common_deps} @@ -631,10 +642,6 @@ if(WITH_JAEGER) add_dependencies(common jaeger_base) endif() -if(WITH_BREAKPAD) -add_dependencies(common breakpad_project) -endif() - if (WIN32) # Statically building ceph-common on Windows fails. We're temporarily # reverting this: 22fefb2338cfc4fcb03ece3cbf77aa964a7f17f2 diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index ae34d330813f3..947c8c7a2de47 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -196,7 +196,11 @@ target_compile_definitions(common-common-objs PRIVATE "CEPH_INSTALL_FULL_PKGLIBDIR=\"${CEPH_INSTALL_FULL_PKGLIBDIR}\"" "CEPH_INSTALL_DATADIR=\"${CEPH_INSTALL_DATADIR}\"" $) -target_link_libraries(common-common-objs legacy-option-headers) +target_link_libraries(common-common-objs PUBLIC legacy-option-headers) +if(WITH_BREAKPAD) + target_link_libraries(common-common-objs PUBLIC + Breakpad::client) +endif() set(common_mountcephfs_srcs armor.c diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index c37045337cce8..20ccb42a42758 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -22,6 +22,10 @@ #include +#ifdef HAVE_BREAKPAD +#include +#endif + #include "include/common_fwd.h" #include "include/mempool.h" #include "include/stringify.h" diff --git a/src/common/ceph_context.h b/src/common/ceph_context.h index df96ae68dcb99..d015f7eee46f2 100644 --- a/src/common/ceph_context.h +++ b/src/common/ceph_context.h @@ -34,9 +34,6 @@ #include "common/cmdparse.h" #include "common/code_environment.h" #include "msg/msg_types.h" -#ifdef HAVE_BREAKPAD -#include "breakpad/src/client/linux/handler/exception_handler.h" -#endif #ifdef WITH_CRIMSON #include "crimson/common/config_proxy.h" #include "crimson/common/perf_counters_collection.h" @@ -49,6 +46,12 @@ #include "crush/CrushLocation.h" +#ifdef HAVE_BREAKPAD +namespace google_breakpad { + class ExceptionHandler; +} +#endif + class AdminSocket; class AdminSocketHook; class CryptoHandler; diff --git a/src/crimson/CMakeLists.txt b/src/crimson/CMakeLists.txt index 09903ff795442..c86ac5ae12afe 100644 --- a/src/crimson/CMakeLists.txt +++ b/src/crimson/CMakeLists.txt @@ -138,7 +138,7 @@ if(WITH_JAEGER) endif() if(WITH_BREAKPAD) - list(APPEND crimson_common_public_deps breakpad) + list(APPEND crimson_common_deps Breakpad::client) endif() if(NOT WITH_SYSTEM_BOOST) diff --git a/src/crimson/os/alienstore/CMakeLists.txt b/src/crimson/os/alienstore/CMakeLists.txt index 2ba396d93fa85..3037bc74bbc3f 100644 --- a/src/crimson/os/alienstore/CMakeLists.txt +++ b/src/crimson/os/alienstore/CMakeLists.txt @@ -32,6 +32,9 @@ if(WITH_CEPH_DEBUG_MUTEX) endif() add_library(crimson-alien-common STATIC ${crimson_alien_common_srcs}) +if(WITH_BREAKPAD) + target_link_libraries(crimson-alien-common Breakpad::client) +endif() add_library(alien::cflags INTERFACE IMPORTED) set_target_properties(alien::cflags PROPERTIES diff --git a/src/global/CMakeLists.txt b/src/global/CMakeLists.txt index a6d3449e7173e..b9c0add329e6f 100644 --- a/src/global/CMakeLists.txt +++ b/src/global/CMakeLists.txt @@ -9,6 +9,10 @@ endif() add_library(libglobal_objs OBJECT ${libglobal_srcs}) target_link_libraries(libglobal_objs legacy-option-headers) +if(WITH_BREAKPAD) + target_link_libraries(libglobal_objs + Breakpad::client) +endif() add_library(global-static STATIC $) diff --git a/src/global/global_init.cc b/src/global/global_init.cc index 46e05916bd4cc..06bb9f3739c6e 100644 --- a/src/global/global_init.cc +++ b/src/global/global_init.cc @@ -14,6 +14,12 @@ #include #include +#include "acconfig.h" +#ifdef HAVE_BREAKPAD +#include +#include +#include +#endif #include "common/async/context_pool.h" #include "common/ceph_argparse.h" #include "common/code_environment.h" @@ -26,10 +32,6 @@ #include "extblkdev/ExtBlkDevPlugin.h" #include "global/global_context.h" #include "global/global_init.h" -#ifdef HAVE_BREAKPAD -#include -#include -#endif #include "global/pidfile.h" #include "global/signal_handler.h" #include "include/compat.h" diff --git a/src/rgw/CMakeLists.txt b/src/rgw/CMakeLists.txt index 41026555d5b0d..f235f18793698 100644 --- a/src/rgw/CMakeLists.txt +++ b/src/rgw/CMakeLists.txt @@ -365,11 +365,6 @@ if(WITH_JAEGER) target_link_libraries(rgw_common PUBLIC jaeger_base) endif() -if(WITH_BREAKPAD) - add_dependencies(rgw_common breakpad_project) - target_link_libraries(rgw_common PUBLIC breakpad) -endif() - if(WITH_RADOSGW_DBSTORE) target_link_libraries(rgw_common PRIVATE global dbstore) endif() diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 9167ab2a30e5c..17206fefadce8 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -1032,7 +1032,7 @@ if(WITH_BREAKPAD) add_executable(unittest_ceph_breakpad ceph_breakpad.cc) add_ceph_unittest(unittest_ceph_breakpad) -target_link_libraries(unittest_ceph_breakpad ceph-common global) +target_link_libraries(unittest_ceph_breakpad ceph-common global Breakpad::breakpad) endif() if(NOT WIN32) -- 2.39.5