Enhance CMake build system to more closely mirror Abseil's#295
Draft
gtkramer wants to merge 12 commits intogoogle:masterfrom
Draft
Enhance CMake build system to more closely mirror Abseil's#295gtkramer wants to merge 12 commits intogoogle:masterfrom
gtkramer wants to merge 12 commits intogoogle:masterfrom
Conversation
Move TCMalloc CMake helper files from tcmalloc/ to a dedicated CMake/ top-level directory, mirroring the organizational pattern used by abseil-cpp (CMake/AbseilHelpers.cmake, CMake/AbseilDll.cmake, etc.). - tcmalloc/tcmalloc_helpers.cmake -> CMake/TcmallocHelpers.cmake - tcmalloc/tcmalloc_variants.cmake -> CMake/TcmallocVariants.cmake - Add CMAKE_MODULE_PATH setup so includes work by module name - Update top-level CMakeLists.txt to use include(TcmallocHelpers) and include(TcmallocVariants) instead of direct file paths
Add standard CMake infrastructure modules that are prerequisites for a proper install/export system, mirroring abseil-cpp's approach: - include(GNUInstallDirs) for portable install path variables - include(CMakePackageConfigHelpers) for config file generation - Set CMAKE_INSTALL_RPATH, CMAKE_INSTALL_RPATH_USE_LINK_PATH, and CMAKE_BUILD_RPATH_USE_ORIGIN for proper shared library resolution - Define TCMALLOC_SOVERSION (analogous to ABSL_SOVERSION)
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Author
|
I'm talking with my team at work to see how I need to sign the Google's CLA to continue. |
Add TCMALLOC_BUILD_TESTING (default OFF) and TCMALLOC_BUILD_TEST_HELPERS options, mirroring abseil-cpp's ABSL_BUILD_TESTING and ABSL_BUILD_TEST_HELPERS. Gate all test/benchmark/fuzz target creation: - tcmalloc_cc_test() early-returns unless BUILD_TESTING AND TCMALLOC_BUILD_TESTING are both ON - tcmalloc_cc_binary() gets the same guard - tcmalloc_cc_test_variants() and tcmalloc_cc_binary_variants() get the same guard - Replace bare enable_testing() with include(CTest) Users who only want the tcmalloc library can now build without compiling any test, benchmark, or fuzz targets.
Move FetchContent declarations for googletest, protobuf, benchmark, and fuzztest behind a guard that checks BUILD_TESTING AND TCMALLOC_BUILD_TESTING (or TCMALLOC_BUILD_TEST_HELPERS). Only abseil-cpp remains unconditional as a core dependency. This means a library-only build (the default) no longer needs to download or have available any test frameworks, significantly reducing configure time and enabling simpler offline builds for users who only need the allocator.
Add a layered dependency resolution system inspired by abseil-cpp's ABSL_USE_EXTERNAL_GOOGLETEST / ABSL_FIND_GOOGLETEST / ABSL_LOCAL_* pattern. Each dependency now supports three resolution strategies: 1. TCMALLOC_USE_EXTERNAL_<DEP> (ON/OFF): Assume targets already provided by parent project, or use find_package() to locate them. 2. TCMALLOC_LOCAL_<DEP>_DIR (path): Point to a local source checkout which is added via add_subdirectory(). 3. Default: Fall back to FetchContent from GitHub (existing behavior). Dependencies covered: - abseil-cpp: TCMALLOC_USE_EXTERNAL_ABSEIL, TCMALLOC_LOCAL_ABSEIL_DIR - GoogleTest: TCMALLOC_USE_EXTERNAL_GOOGLETEST, TCMALLOC_LOCAL_GOOGLETEST_DIR - Protobuf: TCMALLOC_USE_EXTERNAL_PROTOBUF, TCMALLOC_LOCAL_PROTOBUF_DIR - Benchmark: TCMALLOC_USE_EXTERNAL_BENCHMARK, TCMALLOC_LOCAL_BENCHMARK_DIR - FuzzTest: TCMALLOC_USE_EXTERNAL_FUZZTEST, TCMALLOC_LOCAL_FUZZTEST_DIR This enables fully offline/hermetic builds by pointing all dependencies to pre-fetched local directories or system-installed packages.
gtkramer
force-pushed
the
cmake-enhancements
branch
from
21839a3 to
f5275b5
Compare
gtkramer
changed the title
gtkramer
changed the title
Collaborator
|
I expect CMake rules to be a little clunky at first, so I definitely appreciate the improvements. I think the main consideration is interaction with the rest of the CMakeLists. I have a small, rough tool for mapping the Bazel rules (and variants.bzl) to the CMake files, because it seemed easy to miss a library dependency otherwise doing the manual translation. I haven't put it in the TCMalloc repo yet (it uses https://github.com/bazelbuild/buildtools, i.e., Go), so factoring things out into the *.cmake helpers is really useful for isolating the fast(er) changing stuff (individual libraries) from the slow(er) changing stuff (helpers). |
Replace raw ${CMAKE_SOURCE_DIR} include directory references with
proper CMake generator expressions using BUILD_INTERFACE and
INSTALL_INTERFACE, mirroring abseil-cpp's ABSL_COMMON_INCLUDE_DIRS
pattern:
- Define TCMALLOC_COMMON_INCLUDE_DIRS at module scope
- In tcmalloc_cc_library(): use $<BUILD_INTERFACE:...> for build tree
and $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}> for installs
- In tcmalloc_cc_test(): use $<BUILD_INTERFACE:...> (tests are never
installed so no INSTALL_INTERFACE needed)
This fixes correctness when tcmalloc is used as a subdirectory of
another project (where CMAKE_SOURCE_DIR points to the parent) and
enables proper include paths in installed config files.
Rewrite tcmalloc_cc_library() and tcmalloc_cc_test() to match the feature set of abseil-cpp's absl_cc_library() and absl_cc_test(): tcmalloc_cc_library() now supports: - PUBLIC flag: marks target as public API (affects IDE folder placement) - TESTONLY flag: only builds when testing is enabled, mirroring abseil-cpp's TESTONLY semantics - DISABLE_INSTALL flag: skips installation rules for specific targets - DEFINES parameter: public compile definitions - Header-only auto-detection: automatically strips .h/.inc files from SRCS to determine if library should be INTERFACE - IDE folder organization: places targets in TCMalloc/, TCMalloc/test/, or TCMalloc/internal/ folders (configurable via TCMALLOC_IDE_FOLDER) - Per-target RPATH: sets INSTALL_RPATH per platform (Darwin/Unix) - SOVERSION and OUTPUT_NAME: set when TCMALLOC_ENABLE_INSTALL is ON - Install rules via install(TARGETS ... EXPORT) when enabled - TCMALLOC_DEFAULT_LINKOPTS applied to all targets tcmalloc_cc_test() now supports: - DEFINES parameter - IDE folder placement in TCMalloc/test/ - Cleaner variable naming (TCMALLOC_CC_TEST_ prefix) These changes maintain full backward compatibility with existing CMakeLists.txt files that use the NAME/ALIAS/SRCS/HDRS/COPTS/DEPS parameters.
Add a complete install and export system, mirroring abseil-cpp's ABSL_ENABLE_INSTALL mechanism. Key changes: - Add option(TCMALLOC_ENABLE_INSTALL): defaults to OFF when used as a subproject, ON when standalone (exactly matching Abseil's behavior) - Remove the hard-coded CMAKE_SKIP_INSTALL_RULES=ON - Create CMake/tcmallocConfig.cmake.in template (analogous to CMake/abslConfig.cmake.in) that finds Threads and absl dependencies then includes the exported targets file - Add install(EXPORT tcmallocTargets) with tcmalloc:: namespace - Add configure_package_config_file() to generate tcmallocConfig.cmake - Add install(DIRECTORY tcmalloc ...) for public headers, excluding test-only headers (testing/, mock_*) After this change, downstream projects can use: find_package(tcmalloc REQUIRED) target_link_libraries(myapp PRIVATE tcmalloc::tcmalloc) The install rules in tcmalloc_cc_library() (added in the previous commit) are automatically activated when TCMALLOC_ENABLE_INSTALL is ON.
Add TESTONLY to 16 tcmalloc_cc_library() calls that correspond to
testonly targets in the Bazel BUILD files but were being built
unconditionally in CMake. This ensures that when TCMALLOC_BUILD_TESTING
is OFF, no test-only source code is compiled.
Libraries marked TESTONLY:
tcmalloc/CMakeLists.txt (7):
- tcmalloc_tcmalloc_internal_methods_only
- tcmalloc_mock_central_freelist
- tcmalloc_mock_static_forwarder
- tcmalloc_mock_virtual_allocator
- tcmalloc_page_allocator_test_util
- tcmalloc_mock_transfer_cache
- tcmalloc_mock_huge_page_static_forwarder
tcmalloc/internal/CMakeLists.txt (3):
- tcmalloc_internal_affinity
- tcmalloc_internal_mock_metadata_allocator
- tcmalloc_internal_mock_span
tcmalloc/testing/CMakeLists.txt (6):
- tcmalloc_testing_malloc_hook_recorder
- tcmalloc_testing_testutil
- tcmalloc_testing_thread_manager
- tcmalloc_testing_thread_ctor_test_lib
- tcmalloc_testing_benchmark_main
- tcmalloc_testing_test_allocator_harness
Also fixes the top-level CMakeLists.txt:
- Move ABSL_BUILD_TEST_HELPERS, ABSL_USE_EXTERNAL_GOOGLETEST, and
ABSL_FIND_GOOGLETEST behind the testing guard so abseil does not
attempt to fetch googletest when tests are disabled.
- Move protobuf out of the test-only dependency block since
profile_builder and profile_marshaler are core (non-testonly)
libraries that require it.
gtkramer
force-pushed
the
cmake-enhancements
branch
from
79840f0 to
4a97117
Compare
Remove OUTPUT_NAME that double-prefixed library filenames (e.g. libtcmalloc_tcmalloc_tcmalloc.so) since target names already carry the tcmalloc_ prefix. Install all targets when TCMALLOC_ENABLE_INSTALL is ON, mirroring abseil-cpp's absl_cc_library() which installs every library unconditionally. The previous DISABLE_INSTALL and TESTONLY guards are dropped from the install condition to match.
Follow abseil-cpp's AbseilHelpers.cmake naming convention exactly: - When TCMALLOC_ENABLE_INSTALL=ON: CMake target _NAME uses the bare Bazel name (e.g. 'experiment', 'malloc_extension', 'tcmalloc'). OUTPUT_NAME restores the 'tcmalloc_' prefix so installed libraries are named libtcmalloc_<name>.so. - When TCMALLOC_ENABLE_INSTALL=OFF (subdirectory usage): CMake target _NAME is 'tcmalloc_<bazel_name>' directly; no OUTPUT_NAME override is needed. Both paths produce identically named .so files, and the ALIAS (tcmalloc::<bazel_name>) works in either mode. This makes the CMake NAME fields 1:1 with Bazel target names, simplifying future syncs between the two build systems. Changes: - TcmallocHelpers.cmake: conditional _NAME + OUTPUT_NAME logic (mirrors absl_cc_library), tests always get tcmalloc_ prefix - Strip tcmalloc_ prefix from all 220 NAME fields across tcmalloc/, tcmalloc/internal/, tcmalloc/testing/ CMakeLists.txt - Fix 8 hardcoded bare target DEPS to use tcmalloc:: aliases - Fix proto target_include_directories to use conditional name
tcmalloc does not have releases, so versioning the shared libraries with SOVERSION is unnecessary. Remove the TCMALLOC_SOVERSION variable and the SOVERSION property so installed libraries are plain .so files without .so.0 symlink chains.
Author
|
I imagine such a translation tool will ease the maintenance of the CMake build system once the slower moving helper .cmake functions are ironed out where the project wants them. How does the tool that's translating Bazel to CMake handle library names? For example, in tcmalloc/CMakeLists.txt, the first library is named "tcmalloc_experiment". But in Bazel, it's named "experiment". I noticed that for the libraries in the testing and internal folders, those folder names get prepended to the library name. Since Abseil has a 1:1 translation from Bazel to CMake from what I could tell, my more current commits try to nudge things in that direction. Does that make it easier or harder for the tool? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you @ckennelly for introducing a CMake build system! It should go a long way towards improving accessibility to other projects and distribution methods.
I wanted to pick up from where you started and get some high-impact enhancements together that more closely align the CMake build system to what Abseil has and offers. Please let me know if any of this should go in a different direction.