-
Notifications
You must be signed in to change notification settings - Fork 376
Description
The cpuinfo project supports two build systems:
- Bazel
- CMake
The Bazel configuration looks modern and hermetic — it uses MODULE.bazel (bzlmod) and handles dependencies cleanly.
However, the CMake setup is a bit harder to follow and can lead to confusing situations for users building the project standalone.
This project can be built both as part of PyTorch and as a standalone library.
While the PyTorch integration seems to be maintained separately, this suggestion focuses only on the standalone CMake build.
Currently, the project depends on three libraries:
- clog – bundled under
deps/clog, which is fine. - googletest
- googlebenchmark
The CMake configuration for googletest and googlebenchmark mixes multiple dependency modes and can result in inconsistent behavior.
Problem
Here’s a excerpt from
Line 127 in 0d5985d
| SET(CONFU_DEPENDENCIES_SOURCE_DIR ${CMAKE_SOURCE_DIR}/deps |
SET(CONFU_DEPENDENCIES_SOURCE_DIR ${CMAKE_SOURCE_DIR}/deps
CACHE PATH "Confu-style dependencies source directory")
SET(CONFU_DEPENDENCIES_BINARY_DIR ${CMAKE_BINARY_DIR}/deps
CACHE PATH "Confu-style dependencies binary directory")
IF(CPUINFO_SUPPORTED_PLATFORM AND (CPUINFO_BUILD_MOCK_TESTS OR CPUINFO_BUILD_UNIT_TESTS))
IF(USE_SYSTEM_GOOGLETEST)
FIND_PACKAGE(GTest REQUIRED)
ELSEIF(NOT DEFINED GOOGLETEST_SOURCE_DIR)
MESSAGE(STATUS "Downloading Google Test to ${CONFU_DEPENDENCIES_SOURCE_DIR}/googletest (define GOOGLETEST_SOURCE_DIR to avoid it)")
CONFIGURE_FILE(cmake/DownloadGoogleTest.cmake "${CONFU_DEPENDENCIES_BINARY_DIR}/googletest-download/CMakeLists.txt")
EXECUTE_PROCESS(COMMAND "${CMAKE_COMMAND}" -G "${CMAKE_GENERATOR}" .
WORKING_DIRECTORY "${CONFU_DEPENDENCIES_BINARY_DIR}/googletest-download")
EXECUTE_PROCESS(COMMAND "${CMAKE_COMMAND}" --build .
WORKING_DIRECTORY "${CONFU_DEPENDENCIES_BINARY_DIR}/googletest-download")
SET(GOOGLETEST_SOURCE_DIR "${CONFU_DEPENDENCIES_SOURCE_DIR}/googletest" CACHE STRING "Google Test source directory")
ENDIF()
ENDIF()Behavior summary:
- If
USE_SYSTEM_GOOGLETESTis set → uses system-installed GTest which works fine. - If
GOOGLETEST_SOURCE_DIRis defined → uses user-provided source which also works fine. - If neither is set → it attempts to download googletest using
ExternalProject_Add.
The confusing part is that the helper “download” project lives under
${CMAKE_BINARY_DIR}/deps/googletest-download, but the GOOGLETEST_SOURCE_DIR variable is set to
${CMAKE_SOURCE_DIR}/deps/googletest. If the download or extraction fails, that directory never appears,
yet the variable remains cached and points to a non-existent path.
This can result in hard-to-diagnose configuration errors later on.
A similar pattern exists for googlebenchmark.
Example scenario:
On a clean clone with an out-of-source build and no GOOGLETEST_SOURCE_DIR defined, the first download and configure step fails.
This currently happens in the HEAD CI as well, since the bundled googletest version is too old for recent CMake releases.
CMake still caches GOOGLETEST_SOURCE_DIR=${CMAKE_SOURCE_DIR}/deps/googletest, and subsequent configuration fails with:
ADD_SUBDIRECTORY given source
"path/to/cpuinfo/deps/googlebenchmark" which is not an existing
directory.
Suggested Improvements
This logic could be simplified and made more predictable by clearly supporting three modes:
- Use system-installed dependencies
(USE_SYSTEM_GOOGLETEST,USE_SYSTEM_BENCHMARK) - Use user-provided source directories
(GOOGLETEST_SOURCE_DIR,GOOGLEBENCHMARK_SOURCE_DIR) - Automatically fetch dependencies inside the build directory
(e.g.,${CMAKE_BINARY_DIR}/_deps) — without touching the source tree.
For automatic fetching, using FetchContent or ExternalProject_Add with a build-local path would avoid stale cache issues and ensure clean builds.
As an optional enhancement, CI builds could rely on package managers (e.g., vcpkg) to fetch googletest and googlebenchmark automatically.
This would minimize configuration errors, keep builds reproducible, and ensure the project regularly builds against the latest versions of these dependencies — making it easier to catch deprecated API usage early.
(The current HEAD, for example, does not compile cleanly with the latest googlebenchmark.)
Summary
This proposal aims to:
- Make dependency paths consistent and predictable.
- Avoid polluting the source tree with downloaded code.
- Improve failure recovery when dependency downloads / configuration fail.
- Preserve backward compatibility with existing options.
Implementing these changes would make cpuinfo’s standalone CMake build more robust and developer-friendly.