Common west flash/debug etc. arguments cannot be set in CMake #22563
Assignees
Labels
Comments
mbolivar
pushed a commit
to mbolivar/zephyr
that referenced
this issue
Feb 11, 2020
Refactor the code to support the new runners.yaml file created by the
build system.
Compared to fishing around in the CMake cache, this makes it trivial
to put all the command line arguments to a runner-based command on
equal footing, regardless of if they're defined in the runners package
proper or defined in run_common.py.
This allows board.cmake files to do things like this:
board_set_runner_args(foo
--bin-file=${PROJECT_BINARY_DIR}/my-signed.bin)
While at it, make some other cleanups:
- Stop using the obsolete and deprecated west.cmake module while we're
here in favor of the zcmake.py module which was added to Zephyr a long
time ago. Yikes. I had forgotten this was still here.
- Stop using west.util's wrap function in favor of raw use of
textwrap. The west function splits on hyphens, which is breaking
runner names like "em-starterkit".
- Clean up the --context output a bit
Fixes: zephyrproject-rtos#22563
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Merged
jhedberg
pushed a commit
that referenced
this issue
Feb 20, 2020
Refactor the code to support the new runners.yaml file created by the
build system.
Compared to fishing around in the CMake cache, this makes it trivial
to put all the command line arguments to a runner-based command on
equal footing, regardless of if they're defined in the runners package
proper or defined in run_common.py.
This allows board.cmake files to do things like this:
board_set_runner_args(foo
--bin-file=${PROJECT_BINARY_DIR}/my-signed.bin)
While at it, make some other cleanups:
- Stop using the obsolete and deprecated west.cmake module while we're
here in favor of the zcmake.py module which was added to Zephyr a long
time ago. Yikes. I had forgotten this was still here.
- Stop using west.util's wrap function in favor of raw use of
textwrap. The west function splits on hyphens, which is breaking
runner names like "em-starterkit".
- Clean up the --context output a bit
Fixes: #22563
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
mbolivar-nordic
added a commit
to mbolivar-nordic/zephyr
that referenced
this issue
Sep 3, 2020
This commit message is a bit of a novel mostly: - because the issues involved are longstanding - as evidence this is not a capricious refactoring The runners.core.RunnerConfig Python class holds common configuration values used by multiple runners, such as the location of the build outputs and board directory. The runners code, first written in 2017-ish, replaced various shell scripts that got this information from the environment. Avoiding environment variables was a requirement, however. It's ghastly to set environment variables for a single command invocation on Windows, and the whole thing was part of a larger push to make Zephyr development on Windows better. I had a hammer (the argparse module). Finding a replacement naturally looked like a nail, so the information that ends up in RunnerConfig got shunted from the build system to Python in the form of 'west flash' / 'west debug' command line options like '--board-dir', '--elf-file', etc. I initially stored the options and their values in the CMake cache. This was chosen in hopes the build system maintainer would like the strategy (which worked). I knew the command line arguments approach was a bit hacky (this wasn't a nail), but I also honestly didn't have a better idea at the time. It did indeed cause issues: - users don't know that just because they specify --bin-file on the command line doesn't mean that their runner respects the option, and have gotten confused trying to flash alternate files, usually for chain-loading by MCUboot (for example, see zephyrproject-rtos#15961) - common options weren't possible to pass via board.cmake files (zephyrproject-rtos#22563, fixed partly via introduction of runners.yaml and the west flash/debug commands no longer relying on the cache) - it is confusing that "west flash --help" prints information about openocd related options even when the user's board has no openocd support. The same could be said about gdb in potential future use cases where debugging occurs via some other tool. Over time, they've caused enough users enough problems that improvements are a priority. To work towards this, put these values into runners.yaml using a new 'config: ...' key/value instead of command line options. For example, instead of this in the generated runners.yaml file: args: common: - --hex-file=.../zephyr.hex we now have: config: hex_file: zephyr.hex and similarly for other values. In Python, we still support the command line options, but they are not generated by the build system for any in-tree boards. Further work is needed to deprecate the confusing ones (like --hex-file) and move the runner-specific host tool related options (like --openocd) to the runners that need them. Individual board.cmake files should now influence these values by overriding the relevant target properties of the runners_yaml_props_target. For example, instead of: board_runner_args(foo "--hex-file=bar.hex") Do this: set_target_properties(runners_yaml_props_target PROPERTIES hex_file bar.hex) This change additionally allows us to stitch cmake/mcuboot.cmake and the runners together easily by having mcuboot.cmake override the properties that set the hex or bin file to flash. (The command line arguments are still supported as-is.) Combined with 98e0c95d91ae16f14e4997fb64ccdf0956595712 ("build: auto-generate signed mcuboot binaries"), this will allow users to build and flash images to be chain loaded by mcuboot in a way that avoids calling 'west sign' and passing 'west flash' its output files entirely. While we are here, rename runner_yml_write to runners_yaml_append(). This function doesn't actually write anything, and we're here refactoring this file anyway, so we might as well improve the situation while we're at it. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
MaureenHelm
pushed a commit
that referenced
this issue
Sep 3, 2020
This commit message is a bit of a novel mostly: - because the issues involved are longstanding - as evidence this is not a capricious refactoring The runners.core.RunnerConfig Python class holds common configuration values used by multiple runners, such as the location of the build outputs and board directory. The runners code, first written in 2017-ish, replaced various shell scripts that got this information from the environment. Avoiding environment variables was a requirement, however. It's ghastly to set environment variables for a single command invocation on Windows, and the whole thing was part of a larger push to make Zephyr development on Windows better. I had a hammer (the argparse module). Finding a replacement naturally looked like a nail, so the information that ends up in RunnerConfig got shunted from the build system to Python in the form of 'west flash' / 'west debug' command line options like '--board-dir', '--elf-file', etc. I initially stored the options and their values in the CMake cache. This was chosen in hopes the build system maintainer would like the strategy (which worked). I knew the command line arguments approach was a bit hacky (this wasn't a nail), but I also honestly didn't have a better idea at the time. It did indeed cause issues: - users don't know that just because they specify --bin-file on the command line doesn't mean that their runner respects the option, and have gotten confused trying to flash alternate files, usually for chain-loading by MCUboot (for example, see #15961) - common options weren't possible to pass via board.cmake files (#22563, fixed partly via introduction of runners.yaml and the west flash/debug commands no longer relying on the cache) - it is confusing that "west flash --help" prints information about openocd related options even when the user's board has no openocd support. The same could be said about gdb in potential future use cases where debugging occurs via some other tool. Over time, they've caused enough users enough problems that improvements are a priority. To work towards this, put these values into runners.yaml using a new 'config: ...' key/value instead of command line options. For example, instead of this in the generated runners.yaml file: args: common: - --hex-file=.../zephyr.hex we now have: config: hex_file: zephyr.hex and similarly for other values. In Python, we still support the command line options, but they are not generated by the build system for any in-tree boards. Further work is needed to deprecate the confusing ones (like --hex-file) and move the runner-specific host tool related options (like --openocd) to the runners that need them. Individual board.cmake files should now influence these values by overriding the relevant target properties of the runners_yaml_props_target. For example, instead of: board_runner_args(foo "--hex-file=bar.hex") Do this: set_target_properties(runners_yaml_props_target PROPERTIES hex_file bar.hex) This change additionally allows us to stitch cmake/mcuboot.cmake and the runners together easily by having mcuboot.cmake override the properties that set the hex or bin file to flash. (The command line arguments are still supported as-is.) Combined with 98e0c95d91ae16f14e4997fb64ccdf0956595712 ("build: auto-generate signed mcuboot binaries"), this will allow users to build and flash images to be chain loaded by mcuboot in a way that avoids calling 'west sign' and passing 'west flash' its output files entirely. While we are here, rename runner_yml_write to runners_yaml_append(). This function doesn't actually write anything, and we're here refactoring this file anyway, so we might as well improve the situation while we're at it. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The common runner arguments, like --bin-file, cannot be set via board_runner_args() in cmake.
To Reproduce
Steps to reproduce the behavior:
board_runner_args(nrfjprog "--nrf-family=NRF91" "--bin-file=foo.bin")west build && west flashsomethingFATAL ERROR: Runner nrfjprog received unknown arguments: ['--bin-file=foo.bin']Expected behavior
--bin-file ought to be respected
Impact
Users can't override the files to flash and debug. This prevents them from being able to flash and debug if they postprocess the base files generated by the build system as part of the build.
Additional context
Reported by @henrikbrixandersen .
The text was updated successfully, but these errors were encountered: