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 <[email protected]>

Author: tchaikov

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=<INSTALL_DIR>
+            "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 "<INSTALL_DIR>/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

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_PROPERTY:${FMT_LIB},INTERFACE_COMPILE_DEFINITIONS>)
-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

src/common/ceph_context.cc

@@ -22,6 +22,10 @@
 
 #include <boost/algorithm/string.hpp>
 
+#ifdef HAVE_BREAKPAD
+#include <breakpad/client/linux/handler/exception_handler.h>
+#endif
+
 #include "include/common_fwd.h"
 #include "include/mempool.h"
 #include "include/stringify.h"

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;

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)

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

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
   $<TARGET_OBJECTS:libglobal_objs>)

src/global/global_init.cc

@@ -14,6 +14,12 @@
 
 #include <filesystem>
 #include <memory>
+#include "acconfig.h"
+#ifdef HAVE_BREAKPAD
+#include <breakpad/client/linux/handler/exception_handler.h>
+#include <breakpad/client/linux/handler/minidump_descriptor.h>
+#include <breakpad/google_breakpad/common/minidump_format.h>
+#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 <client/linux/handler/minidump_descriptor.h>
-#include <google_breakpad/common/minidump_format.h>
-#endif
 #include "global/pidfile.h"
 #include "global/signal_handler.h"
 #include "include/compat.h"

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()

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)