Skip to content

Do not elide find_package() if MONGO_USE_CCACHE is set #1031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2022
Merged

Do not elide find_package() if MONGO_USE_CCACHE is set #1031

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
793b874
Do not elide find_package() if MONGO_USE_CCACHE is set
eramongodb Jun 7, 2022
a6bf065
Fix undesirable variable expansion in if condition
eramongodb Jun 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 34 additions & 35 deletions build/cmake/CCache.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,40 @@
ON or OFF.
]]

# Find and enable ccache for compiling if not already found.
if (NOT DEFINED MONGO_USE_CCACHE)
find_program (CCACHE_EXECUTABLE ccache)
if (CCACHE_EXECUTABLE)
message (STATUS "Found ccache: ${CCACHE_EXECUTABLE}")

execute_process (
COMMAND ${CCACHE_EXECUTABLE} --version | perl -ne "print $1 if /^ccache version (.+)$/"
OUTPUT_VARIABLE CCACHE_VERSION
OUTPUT_STRIP_TRAILING_WHITESPACE
)

# Assume `ccache --version` mentions a simple version string, e.g. "1.2.3".
# Permit patch number to be omitted, e.g. "1.2".
set (SIMPLE_SEMVER_REGEX "([0-9]+)\.([0-9]+)(\.([0-9]+))?")
string (REGEX MATCH "${SIMPLE_SEMVER_REGEX}" CCACHE_VERSION ${CCACHE_VERSION})

if (CCACHE_VERSION)
message (STATUS "Detected ccache version: ${CCACHE_VERSION}")
else ()
message (WARNING "Could not obtain ccache version from `ccache --version`. Defaulting to 0.1.0.")
set (CCACHE_VERSION 0.1.0)
endif ()

# Avoid spurious "ccache.conf: No such file or directory" errors due to ccache being invoked in parallel, which was patched in ccache version 3.4.3.
if (${CCACHE_VERSION} VERSION_LESS 3.4.3)
message (STATUS "Detected ccache version ${CCACHE_VERSION} is less than 3.4.3, which may lead to spurious failures when run in parallel. See https://github.com/ccache/ccache/issues/260 for more information.")
message (STATUS "Compiling with CCache disabled. Enable by setting MONGO_USE_CCACHE to ON")
option (MONGO_USE_CCACHE "Use CCache when compiling" OFF)
else ()
message (STATUS "Compiling with CCache enabled. Disable by setting MONGO_USE_CCACHE to OFF")
option (MONGO_USE_CCACHE "Use CCache when compiling" ON)
endif ()
endif (CCACHE_EXECUTABLE)
endif (NOT DEFINED MONGO_USE_CCACHE)
find_program (CCACHE_EXECUTABLE ccache)

# Enable ccache for compiling if not already configured.
if (CCACHE_EXECUTABLE AND NOT DEFINED MONGO_USE_CCACHE)
message (STATUS "Found ccache: ${CCACHE_EXECUTABLE}")

execute_process (
COMMAND ${CCACHE_EXECUTABLE} --version | perl -ne "print $1 if /^ccache version (.+)$/"
OUTPUT_VARIABLE CCACHE_VERSION
OUTPUT_STRIP_TRAILING_WHITESPACE
)

# Assume `ccache --version` mentions a simple version string, e.g. "1.2.3".
# Permit patch number to be omitted, e.g. "1.2".
set (SIMPLE_SEMVER_REGEX "([0-9]+)\.([0-9]+)(\.([0-9]+))?")
string (REGEX MATCH "${SIMPLE_SEMVER_REGEX}" CCACHE_VERSION ${CCACHE_VERSION})

if (CCACHE_VERSION)
message (STATUS "Detected ccache version: ${CCACHE_VERSION}")
else ()
message (WARNING "Could not obtain ccache version from `ccache --version`. Defaulting to 0.1.0.")
set (CCACHE_VERSION 0.1.0)
endif ()

# Avoid spurious "ccache.conf: No such file or directory" errors due to ccache being invoked in parallel, which was patched in ccache version 3.4.3.
if (CCACHE_VERSION VERSION_LESS "3.4.3")
message (STATUS "Detected ccache version ${CCACHE_VERSION} is less than 3.4.3, which may lead to spurious failures when run in parallel. See https://github.com/ccache/ccache/issues/260 for more information.")
message (STATUS "Compiling with CCache disabled. Enable by setting MONGO_USE_CCACHE to ON")
option (MONGO_USE_CCACHE "Use CCache when compiling" OFF)
else ()
message (STATUS "Compiling with CCache enabled. Disable by setting MONGO_USE_CCACHE to OFF")
option (MONGO_USE_CCACHE "Use CCache when compiling" ON)
endif ()
endif (CCACHE_EXECUTABLE AND NOT DEFINED MONGO_USE_CCACHE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you're repeating the condition in the endif() call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to not lose track of the corresponding if () given the size of the block, a la #endif // COND or } // namespace <name>. I can remove if you think it is undesirable/unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only tends to be a code smell as it is a very very old pattern that is disrecommended anymore in favor of just using prose-like comments for the purpose, but I don't feel strongly about it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer endif (<cond>) over endif () # cond since CMake actually detects and enforces <cond> matches, which is a maintainability bonus imo as it prevents (or at least tries to prevent) the possibility of prose-like comments becoming outdated/detached from the matching if ().

CMake Warning (dev) in example/CMakeLists.txt:
  A logical block opening on the line
    CMakeLists.txt:10 (if)
  closes on the line
    CMakeLists.txt:12 (endif)
  with mis-matching arguments.

Admittedly it gets confusing if there are any elseif() / else () blocks involved, so I avoid using it in such cases, but I still prefer using them if it's simple (but large) if() / endif() blocks. I may reconsider this stance after further thought. 🤔


if (MONGO_USE_CCACHE)
set (CMAKE_CXX_COMPILER_LAUNCHER "${CCACHE_EXECUTABLE}")
Expand Down