runners: remove dependencies from runners
Today, there is a build target is added for each runner: flash, debug, debugserver, attach. And those runners will have a dependency to Zephyr logical target that is built before invoking `west <runner>`. This design has some flaws, mainly that additional dependencies directly on the target will not be built when running `west <runner>` directly. That generator expressions cannot be used for the DEPENDS argument. Instead, the build target `<runner>` will not have any dependencies, and will raise a build error if a dependency is added to the target. Due to how `add_dependencies()` work, this must be done as a build time check, and not configure time check. `west <runner>` will invoke a build before executing the runner, and this way ensure the build target is up-to-date, which again removes the need for a dedicated `west_<runner>_target`. It also minimizes the risk of developer errors, as developers no longer need to consider the need for adding additional dependencies. If a custom target is part of the default `all` build, then it's ensured to be up-to-date. Fixes: Issue reported on slack. Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
This commit is contained in:
parent
f0dbfec5cd
commit
3dd65a7663
4 changed files with 35 additions and 30 deletions
|
@ -654,8 +654,6 @@ endif()
|
|||
#
|
||||
# Currently used properties:
|
||||
# - COMPILES_OPTIONS: Used by application memory partition feature
|
||||
# - ${TARGET}_DEPENDENCIES: additional dependencies for targets that need them
|
||||
# like flash (FLASH_DEPENDENCIES), debug (DEBUG_DEPENDENCIES), etc.
|
||||
add_custom_target(zephyr_property_target)
|
||||
|
||||
# "app" is a CMake library containing all the application code and is
|
||||
|
|
|
@ -159,37 +159,26 @@ foreach(target flash debug debugserver attach)
|
|||
set(RUNNER_VERBOSE)
|
||||
endif()
|
||||
|
||||
# We pass --skip-rebuild here because the DEPENDS value ensures the
|
||||
# build is already up to date before west is run.
|
||||
if(WEST)
|
||||
set(cmd
|
||||
${CMAKE_COMMAND} -E env
|
||||
${WEST}
|
||||
${RUNNER_VERBOSE}
|
||||
${target}
|
||||
--skip-rebuild
|
||||
DEPENDS ${RUNNERS_DEPS}
|
||||
$<TARGET_PROPERTY:zephyr_property_target,${TARGET_UPPER}_DEPENDENCIES>
|
||||
WORKING_DIRECTORY ${APPLICATION_BINARY_DIR}
|
||||
)
|
||||
|
||||
add_custom_target(${target}
|
||||
# This script will print an error message and fail if <target> has added
|
||||
# dependencies. This is done using dedicated CMake script, as
|
||||
# `cmake -E {true|false}` is not available until CMake 3.16.
|
||||
COMMAND ${CMAKE_COMMAND}
|
||||
-DTARGET=${target}
|
||||
-DDEPENDENCIES="$<TARGET_PROPERTY:${target},MANUALLY_ADDED_DEPENDENCIES>"
|
||||
-P ${CMAKE_CURRENT_LIST_DIR}/check_runner_dependencies.cmake
|
||||
COMMAND
|
||||
${cmd}
|
||||
${CMAKE_COMMAND} -E env
|
||||
${WEST}
|
||||
${RUNNER_VERBOSE}
|
||||
${target}
|
||||
WORKING_DIRECTORY
|
||||
${APPLICATION_BINARY_DIR}
|
||||
COMMENT
|
||||
${comment}
|
||||
${comment}
|
||||
USES_TERMINAL
|
||||
)
|
||||
|
||||
# This is an artificial target to allow west to ensure all dependencies of
|
||||
# target is build before carrying out the actual command.
|
||||
# This allows `west` to run just the dependencies before flashing.
|
||||
add_custom_target(west_${target}_depends
|
||||
COMMAND ${CMAKE_COMMAND} -E echo ""
|
||||
DEPENDS ${RUNNERS_DEPS}
|
||||
$<TARGET_PROPERTY:zephyr_property_target,${TARGET_UPPER}_DEPENDENCIES>
|
||||
USES_TERMINAL
|
||||
)
|
||||
)
|
||||
else()
|
||||
add_custom_target(${target}
|
||||
COMMAND ${CMAKE_COMMAND} -E echo \"West was not found in path. To support
|
||||
|
|
19
cmake/flash/check_runner_dependencies.cmake
Normal file
19
cmake/flash/check_runner_dependencies.cmake
Normal file
|
@ -0,0 +1,19 @@
|
|||
# SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
# The purpose of this script is to ensure that no runners targets contains
|
||||
# additional dependencies.
|
||||
#
|
||||
# If the target contains dependencies, an error will be printed.
|
||||
#
|
||||
# Arguments to the script
|
||||
#
|
||||
# TARGET: The target being checked.
|
||||
# DEPENDENCIES: List containing dependencies on target.
|
||||
|
||||
if(DEPENDENCIES)
|
||||
string(REPLACE ";" " " DEPENDENCIES "${DEPENDENCIES}")
|
||||
message(FATAL_ERROR
|
||||
"`${TARGET}` cannot have dependencies, please remove "
|
||||
"`add_dependencies(${TARGET} ${DEPENDENCIES})` in build system."
|
||||
)
|
||||
endif()
|
|
@ -272,9 +272,8 @@ def load_cmake_cache(build_dir, args):
|
|||
|
||||
def rebuild(command, build_dir, args):
|
||||
_banner(f'west {command.name}: rebuilding')
|
||||
extra_args = ['--target', 'west_' + command.name + '_depends']
|
||||
try:
|
||||
zcmake.run_build(build_dir, extra_args=extra_args)
|
||||
zcmake.run_build(build_dir)
|
||||
except CalledProcessError:
|
||||
if args.build_dir:
|
||||
log.die(f're-build in {args.build_dir} failed')
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue