Skip to content
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

cmake: Re-write the capability database to avoid file appends #10481

Merged
merged 1 commit into from
Oct 19, 2018
Merged

cmake: Re-write the capability database to avoid file appends #10481

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 45 additions & 18 deletions cmake/extensions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,37 +628,53 @@ endfunction()
# caching comes in addition to the caching that CMake does in the
# build folder's CMakeCache.txt)
function(zephyr_check_compiler_flag lang option check)
# Locate the cache
# Locate the cache directory
set_ifndef(
ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE
${USER_CACHE_DIR}/ToolchainCapabilityDatabase.cmake
ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE_DIR
${USER_CACHE_DIR}/ToolchainCapabilityDatabase
)
if(DEFINED ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE)
assert(0
"The deprecated ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE is now a directory"
"and is named ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE_DIR"
)
# Remove this deprecation warning in version 1.14.
endif()

# Read the cache
include(${ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE} OPTIONAL)
# The toolchain capability database/cache is maintained as a
# directory of files. The filenames in the directory are keys, and
# the file contents are the values in this key-value store.

# We need to create a unique key wrt. testing the toolchain
# capability. This key must be a valid C identifier that includes
# everything that can affect the toolchain test.
# capability. This key must include everything that can affect the
# toolchain test.
#
# Also, to fit the key into a filename we calculate the MD5 sum of
# the key.

# The 'cacheformat' must be bumped if a bug in the caching mechanism
# is detected and all old keys must be invalidated.
set(cacheformat 2)
set(cacheformat 3)

set(key_string "")
set(key_string "${key_string}ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE_")
set(key_string "${key_string}cacheformat_")
set(key_string "${key_string}${cacheformat}_")
set(key_string "${key_string}${TOOLCHAIN_SIGNATURE}_")
set(key_string "${key_string}${lang}_")
set(key_string "${key_string}${option}_")
set(key_string "${key_string}${CMAKE_REQUIRED_FLAGS}_")

string(MAKE_C_IDENTIFIER ${key_string} key)
string(MD5 key ${key_string})

# Check the cache
if(DEFINED ${key})
set(${check} ${${key}} PARENT_SCOPE)
set(key_path ${ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE_DIR}/${key})
if(EXISTS ${key_path})
file(READ
${key_path} # File to be read
key_value # Output variable
LIMIT 1 # Read at most 1 byte ('0' or '1')
)

set(${check} ${key_value} PARENT_SCOPE)
return()
endif()

Expand All @@ -668,11 +684,22 @@ function(zephyr_check_compiler_flag lang option check)
set(${check} ${inner_check} PARENT_SCOPE)

# Populate the cache
file(
APPEND
${ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE}
"set(${key} ${inner_check})\n"
)
if(NOT (EXISTS ${key_path}))
file(
WRITE
${key_path}
${inner_check}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't that just reduce the window of the race? I mean it's much better but you could still have two cmake instances trying to create the same key at the same time, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe this is thread-safe.

AFAIK two threads writing the same byte to the same position of the same file is thread-safe.

But I could be wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK so there is a hopefully harmless race but a race nevertheless. PR #15128 just submitted removes the "hopefully".

)

# Populate a metadata file (only intended for trouble shooting)
# with information about the hash, the toolchain capability
# result, and the toolchain test.
file(
APPEND
${ZEPHYR_TOOLCHAIN_CAPABILITY_CACHE_DIR}/log.txt
"${inner_check} ${key} ${key_string}\n"
)
endif()
endfunction()

########################################################
Expand Down