cmake: cleanup board and shield cmake code

Cleanup board and shield cmake code so that code duplication is reduced.

Before this commit, the code for verifying if BOARD or SHIELD user input
had changed was almost identical.

The common code has been made into a Zephyr extension function so that
all part of Zephyr build system can have a uniform way of checking if
sticky arguments has changed, and print user warning in a uniform way.

This also fixes an undiscovered flaw, where changing a shield is not
possible if a shield was defined at first CMake invocation, but in case
no shield had been specified at all, then build system would wrongly
allow specifying and changing shields, for example doing:
cmake -DBOARD=nrf52840dk_nrf52840 -DSHIELD=sparkfun_sara_r4 <sample>
cmake -DSHIELD=wnc_m14a2a

results correctly in:
> CMake Warning at zephyr/cmake/app/boilerplate.cmake:287 (message):
>   The build directory must be cleaned pristinely when changing shields

and correctly keep existing SHIELD settings.

but doing:
cmake -DBOARD=nrf52840dk_nrf52840 <sample>
cmake -DSHIELD=sparkfun_sara_r4
cmake -DSHIELD=wnc_m14a2a

would not result in any warning, because `CACHED_SHIELD` was not
correctly checked in this case.

Changing shield is not a supported feature as it have unknown behavior.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
This commit is contained in:
Torsten Rasmussen 2020-11-10 13:42:32 +01:00 committed by Anas Nashif
parent da1b3c39c5
commit 7247412a9e
2 changed files with 113 additions and 120 deletions

View file

@ -190,66 +190,9 @@ add_custom_target(
# Dummy add to generate files.
zephyr_linker_sources(SECTIONS)
# The BOARD can be set by 3 sources. Through environment variables,
# through the cmake CLI, and through CMakeLists.txt.
#
# CLI has the highest precedence, then comes environment variables,
# and then finally CMakeLists.txt.
#
# A user can ignore all the precedence rules if he simply always uses
# the same source. E.g. always specifies -DBOARD= on the command line,
# always has an environment variable set, or always has a set(BOARD
# foo) line in his CMakeLists.txt and avoids mixing sources.
#
# The selected BOARD can be accessed through the variable 'BOARD'.
# Check that BOARD has been provided, and that it has not changed.
zephyr_check_cache(BOARD REQUIRED)
# Read out the cached board value if present
get_property(cached_board_value CACHE BOARD PROPERTY VALUE)
# There are actually 4 sources, the three user input sources, and the
# previously used value (CACHED_BOARD). The previously used value has
# precedence, and if we detect that the user is trying to change the
# value we give him a warning about needing to clean the build
# directory to be able to change boards.
set(board_cli_argument ${cached_board_value}) # Either new or old
if(board_cli_argument STREQUAL CACHED_BOARD)
# We already have a CACHED_BOARD so there is no new input on the CLI
unset(board_cli_argument)
endif()
set(board_app_cmake_lists ${BOARD})
if(cached_board_value STREQUAL BOARD)
# The app build scripts did not set a default, The BOARD we are
# reading is the cached value from the CLI
unset(board_app_cmake_lists)
endif()
if(CACHED_BOARD)
# Warn the user if it looks like he is trying to change the board
# without cleaning first
if(board_cli_argument)
if(NOT ((CACHED_BOARD STREQUAL board_cli_argument) OR (BOARD_DEPRECATED STREQUAL board_cli_argument)))
message(WARNING "The build directory must be cleaned pristinely when changing boards")
# TODO: Support changing boards without requiring a clean build
endif()
endif()
set(BOARD ${CACHED_BOARD})
elseif(board_cli_argument)
set(BOARD ${board_cli_argument})
elseif(DEFINED ENV{BOARD})
set(BOARD $ENV{BOARD})
elseif(board_app_cmake_lists)
set(BOARD ${board_app_cmake_lists})
else()
message(FATAL_ERROR "BOARD is not being defined on the CMake command-line in the environment or by the app.")
endif()
assert(BOARD "BOARD not set")
message(STATUS "Board: ${BOARD}")
if(DEFINED ENV{ZEPHYR_BOARD_ALIASES})
@ -267,67 +210,8 @@ if(${BOARD}_DEPRECATED)
message(WARNING "Deprecated BOARD=${BOARD_DEPRECATED} name specified, board automatically changed to: ${BOARD}.")
endif()
# Store the selected board in the cache
set(CACHED_BOARD ${BOARD} CACHE STRING "Selected board")
# The SHIELD can be set by 3 sources. Through environment variables,
# through the cmake CLI, and through CMakeLists.txt.
#
# CLI has the highest precedence, then comes environment variables,
# and then finally CMakeLists.txt.
#
# A user can ignore all the precedence rules if he simply always uses
# the same source. E.g. always specifies -DSHIELD= on the command line,
# always has an environment variable set, or always has a set(SHIELD
# foo) line in his CMakeLists.txt and avoids mixing sources.
#
# The selected SHIELD can be accessed through the variable 'SHIELD'.
# Read out the cached shield value if present
get_property(cached_shield_value CACHE SHIELD PROPERTY VALUE)
# There are actually 4 sources, the three user input sources, and the
# previously used value (CACHED_SHIELD). The previously used value has
# precedence, and if we detect that the user is trying to change the
# value we give him a warning about needing to clean the build
# directory to be able to change shields.
set(shield_cli_argument ${cached_shield_value}) # Either new or old
if(shield_cli_argument STREQUAL CACHED_SHIELD)
# We already have a CACHED_SHIELD so there is no new input on the CLI
unset(shield_cli_argument)
endif()
set(shield_app_cmake_lists ${SHIELD})
if(cached_shield_value STREQUAL SHIELD)
# The app build scripts did not set a default, The SHIELD we are
# reading is the cached value from the CLI
unset(shield_app_cmake_lists)
endif()
if(CACHED_SHIELD)
# Warn the user if it looks like he is trying to change the shield
# without cleaning first
if(shield_cli_argument)
if(NOT (CACHED_SHIELD STREQUAL shield_cli_argument))
message(WARNING "The build directory must be cleaned pristinely when changing shields")
# TODO: Support changing shields without requiring a clean build
endif()
endif()
set(SHIELD ${CACHED_SHIELD})
elseif(shield_cli_argument)
set(SHIELD ${shield_cli_argument})
elseif(DEFINED ENV{SHIELD})
set(SHIELD $ENV{SHIELD})
elseif(shield_app_cmake_lists)
set(SHIELD ${shield_app_cmake_lists})
endif()
# Store the selected shield in the cache
set(CACHED_SHIELD ${SHIELD} CACHE STRING "Selected shield")
# Check that SHIELD has not changed.
zephyr_check_cache(SHIELD)
# 'BOARD_ROOT' is a prioritized list of directories where boards may
# be found. It always includes ${ZEPHYR_BASE} at the lowest priority.

View file

@ -1776,6 +1776,115 @@ Relative paths are only allowed with `-D${ARGV1}=<path>`")
endif()
endfunction()
# Usage:
# zephyr_check_cache(<variable> [REQUIRED])
#
# Check the current CMake cache for <variable> and warn the user if the value
# is being modified.
#
# This can be used to ensure the user does not accidentally try to change
# Zephyr build variables, such as:
# - BOARD
# - SHIELD
#
# variable: Name of <variable> to check and set, for example BOARD.
# REQUIRED: Optional flag. If specified, then an unset <variable> will be
# treated as an error.
#
# Details:
# <variable> can be set by 3 sources.
# - Using CMake argument, -D<variable>
# - Using an environment variable
# - In the project CMakeLists.txt before `find_package(Zephyr)`.
#
# CLI has the highest precedence, then comes environment variables,
# and then finally CMakeLists.txt.
#
# The value defined on the first CMake invocation will be stored in the CMake
# cache as CACHED_<variable>. This allows the Zephyr build system to detect
# when a user reconfigures a sticky variable.
#
# A user can ignore all the precedence rules if the same source is always used
# E.g. always specifies -D<variable>= on the command line,
# always has an environment <variable> set, or always has a set(<variable> foo)
# line in his CMakeLists.txt and avoids mixing sources.
#
# The selected <variable> can be accessed through the variable '<variable>' in
# following Zephyr CMake code.
#
# If the user tries to change <variable> to a new value, then a warning will
# be printed, and the previously cached value (CACHED_<variable>) will be
# used, as it has precedence.
#
# Together with the warning, user is informed that in order to change
# <variable> the build directory must be cleaned.
#
function(zephyr_check_cache variable)
cmake_parse_arguments(CACHE_VAR "REQUIRED" "" "" ${ARGN})
string(TOLOWER ${variable} variable_text)
string(REPLACE "_" " " variable_text ${variable_text})
get_property(cached_value CACHE ${variable} PROPERTY VALUE)
# If the build has already been configured in an earlier CMake invocation,
# then CACHED_${variable} is set. The CACHED_${variable} setting takes
# precedence over any user or CMakeLists.txt input.
# If we detect that user tries to change the setting, then print a warning
# that a pristine build is needed.
# If user uses -D<variable>=<new_value>, then cli_argument will hold the new
# value, otherwise cli_argument will hold the existing (old) value.
set(cli_argument ${cached_value})
if(cli_argument STREQUAL CACHED_${variable})
# The is no changes to the <variable> value.
unset(cli_argument)
endif()
set(app_cmake_lists ${${variable}})
if(cached_value STREQUAL ${variable})
# The app build scripts did not set a default, The BOARD we are
# reading is the cached value from the CLI
unset(app_cmake_lists)
endif()
if(DEFINED CACHED_${variable})
# Warn the user if it looks like he is trying to change the board
# without cleaning first
if(cli_argument)
if(NOT ((CACHED_${variable} STREQUAL cli_argument) OR (${variable}_DEPRECATED STREQUAL cli_argument)))
message(WARNING "The build directory must be cleaned pristinely when "
"changing ${variable_text},\n"
"Current value=\"${CACHED_${variable}}\", "
"Ignored value=\"${cli_argument}\"")
endif()
endif()
if(CACHED_${variable})
set(${variable} ${CACHED_${variable}} PARENT_SCOPE)
# This resets the user provided value with previous (working) value.
set(${variable} ${CACHED_${variable}} CACHE STRING "Selected ${variable_text}" FORCE)
else()
unset(${variable} PARENT_SCOPE)
unset(${variable} CACHE)
endif()
elseif(cli_argument)
set(${variable} ${cli_argument})
elseif(DEFINED ENV{${variable}})
set(${variable} $ENV{${variable}})
elseif(app_cmake_lists)
set(${variable} ${app_cmake_lists})
elseif(${CACHE_VAR_REQUIRED})
message(FATAL_ERROR "${variable} is not being defined on the CMake command-line in the environment or by the app.")
endif()
# Store the specified variable in parent scope and the cache
set(${variable} ${${variable}} PARENT_SCOPE)
set(CACHED_${variable} ${${variable}} CACHE STRING "Selected ${variable_text}")
endfunction(zephyr_check_cache variable)
# Usage:
# zephyr_get_targets(<directory> <types> <targets>)
#