From afb9f25c5f7c700a25ea102599620301d46d1f10 Mon Sep 17 00:00:00 2001 From: Marti Bolivar Date: Wed, 30 Jan 2019 13:53:40 -0700 Subject: [PATCH 1/3] scripts: west_commands: refactor build.py This is a prep work patch for adding another command. Refactor build.py to use a new Forceable superclass and find_build_dir() helper routine. Fix a help string while we are here. Signed-off-by: Marti Bolivar --- scripts/west_commands/build.py | 131 ++++++++++----------- scripts/west_commands/zephyr_ext_common.py | 60 ++++++++++ 2 files changed, 122 insertions(+), 69 deletions(-) create mode 100644 scripts/west_commands/zephyr_ext_common.py diff --git a/scripts/west_commands/build.py b/scripts/west_commands/build.py index 2e7b426c9e488..78d7668477352 100644 --- a/scripts/west_commands/build.py +++ b/scripts/west_commands/build.py @@ -7,9 +7,10 @@ from west import log from west import cmake -from west.build import DEFAULT_BUILD_DIR, DEFAULT_CMAKE_GENERATOR, \ - is_zephyr_build -from west.commands import WestCommand +from west.build import DEFAULT_CMAKE_GENERATOR, is_zephyr_build + +from zephyr_ext_common import find_build_dir, Forceable, BUILD_DIR_DESCRIPTION + BUILD_DESCRIPTION = '''\ Convenience wrapper for building Zephyr applications. @@ -36,11 +37,14 @@ to run by setting --cmake. To pass additional options to CMake, give them as extra arguments -after a '--' For example, "west build -- -DOVERLAY_CONFIG=some.conf" sets -an overlay config file. (Doing this forces a CMake run.)''' +after a '--'. For example, this sets an overlay config file: + +west build [...] -- -DOVERLAY_CONFIG=some.conf +(Doing this forces a CMake run.)''' -class Build(WestCommand): + +class Build(Forceable): def __init__(self): super(Build, self).__init__( @@ -89,18 +93,14 @@ def do_add_parser(self, parser_adder): cache. Otherwise, the current directory is assumed.''') parser.add_argument('-d', '--build-dir', - help='''Explicitly sets the build directory. - If not given and the current directory is a Zephyr - build directory, it will be used; otherwise, "{}" - is assumed. The directory will be created if - it doesn't exist.'''.format(DEFAULT_BUILD_DIR)) + help=BUILD_DIR_DESCRIPTION + + "The directory is created if it doesn't exist.") parser.add_argument('-t', '--target', help='''Override the build system target (e.g. 'clean', 'pristine', etc.)''') parser.add_argument('-c', '--cmake', action='store_true', help='Force CMake to run') - parser.add_argument('-f', '--force', action='store_true', - help='Ignore any errors and try to build anyway') + self.add_force_arg(parser) parser.add_argument('cmake_opts', nargs='*', metavar='cmake_opt', help='Extra option to pass to CMake; implies -c') @@ -144,12 +144,12 @@ def do_run(self, args, ignored): def _sanity_precheck(self): app = self.args.source_dir if app: - if not os.path.isdir(app): - self._check_force('source directory {} does not exist'. - format(app)) - elif 'CMakeLists.txt' not in os.listdir(app): - self._check_force("{} doesn't contain a CMakeLists.txt". - format(app)) + self.check_force( + os.path.isdir(app), + 'source directory {} does not exist'.format(app)) + self.check_force( + 'CMakeLists.txt' in os.listdir(app), + "{} doesn't contain a CMakeLists.txt".format(app)) def _update_cache(self): try: @@ -159,16 +159,9 @@ def _update_cache(self): def _setup_build_dir(self): # Initialize build_dir and created_build_dir attributes. + # If we created the build directory, we must run CMake. log.dbg('setting up build directory', level=log.VERBOSE_EXTREME) - if self.args.build_dir: - build_dir = self.args.build_dir - else: - cwd = os.getcwd() - if is_zephyr_build(cwd): - build_dir = cwd - else: - build_dir = DEFAULT_BUILD_DIR - build_dir = os.path.abspath(build_dir) + build_dir = find_build_dir(self.args.build_dir) if os.path.exists(build_dir): if not os.path.isdir(build_dir): @@ -211,20 +204,21 @@ def _sanity_check(self): format(self.source_dir, self.build_dir)) srcrel = os.path.relpath(self.source_dir) - if is_zephyr_build(self.source_dir): - self._check_force('it looks like {srcrel} is a build directory: ' - 'did you mean -build-dir {srcrel} instead?'. - format(srcrel=srcrel)) - elif 'CMakeLists.txt' not in os.listdir(self.source_dir): - self._check_force('source directory "{srcrel}" does not contain ' - 'a CMakeLists.txt; is that really what you ' - 'want to build? (Use -s SOURCE_DIR to specify ' - 'the application source directory)'. - format(srcrel=srcrel)) - - if not is_zephyr_build(self.build_dir) and not self.args.board: - self._check_force('this looks like a new or clean build, ' - 'please provide --board') + self.check_force( + not is_zephyr_build(self.source_dir), + 'it looks like {srcrel} is a build directory: ' + 'did you mean --build-dir {srcrel} instead?'. + format(srcrel=srcrel)) + self.check_force( + 'CMakeLists.txt' in os.listdir(self.source_dir), + 'source directory "{srcrel}" does not contain ' + 'a CMakeLists.txt; is this really what you ' + 'want to build? (Use -s SOURCE_DIR to specify ' + 'the application source directory)'. + format(srcrel=srcrel)) + self.check_force( + is_zephyr_build(self.build_dir) or self.args.board, + 'this looks like a new or clean build, please provide --board') if not self.cmake_cache: return # That's all we can check without a cache. @@ -235,37 +229,36 @@ def _sanity_check(self): source_abs = (os.path.abspath(self.args.source_dir) if self.args.source_dir else None) cached_abs = os.path.abspath(cached_app) if cached_app else None - if cached_abs and source_abs and source_abs != cached_abs: - self._check_force('build directory "{}" is for application "{}", ' - 'but source directory "{}" was specified; ' - 'please clean it or use --build-dir to set ' - 'another build directory'. - format(self.build_dir, cached_abs, - source_abs)) + + # If the build directory specifies a source app, make sure it's + # consistent with --source-dir. + apps_mismatched = (source_abs and cached_abs and + source_abs != cached_abs) + self.check_force( + not apps_mismatched, + 'Build directory "{}" is for application "{}", but source ' + 'directory "{}" was specified; please clean it or use --build-dir ' + 'to set another build directory'. + format(self.build_dir, cached_abs, source_abs)) + if apps_mismatched: self.run_cmake = True # If they insist, we need to re-run cmake. + # If CACHED_BOARD is not defined, we need --board from the + # command line. cached_board = self.cmake_cache.get('CACHED_BOARD') log.dbg('CACHED_BOARD:', cached_board, level=log.VERBOSE_EXTREME) - if not cached_board and not self.args.board: - if self.created_build_dir: - self._check_force( - 'Building for the first time: you must provide --board') - else: - self._check_force( - 'Board is missing or unknown, please provide --board') - if self.args.board and cached_board and \ - self.args.board != cached_board: - self._check_force('Build directory {} targets board {}, ' - 'but board {} was specified. (Clean that ' - 'directory or use --build-dir to specify ' - 'a different one.)'. - format(self.build_dir, cached_board, - self.args.board)) - - def _check_force(self, msg): - if not self.args.force: - log.err(msg) - log.die('refusing to proceed without --force due to above error') + self.check_force(cached_board or self.args.board, + 'Cached board not defined, please provide --board') + + # Check consistency between cached board and --board. + boards_mismatched = (self.args.board and cached_board and + self.args.board != cached_board) + self.check_force( + not boards_mismatched, + 'Build directory {} targets board {}, but board {} was specified. ' + '(Clean the directory or use --build-dir to specify a different ' + 'one.)'. + format(self.build_dir, cached_board, self.args.board)) def _run_cmake(self, cmake_opts): if not self.run_cmake: diff --git a/scripts/west_commands/zephyr_ext_common.py b/scripts/west_commands/zephyr_ext_common.py new file mode 100644 index 0000000000000..bf9d7b7bdb742 --- /dev/null +++ b/scripts/west_commands/zephyr_ext_common.py @@ -0,0 +1,60 @@ +# Copyright (c) 2018 Foundries.io +# +# SPDX-License-Identifier: Apache-2.0 + +'''Helpers shared by multiple west extension command modules. + +Note that common helpers used by the flash and debug extension +commands are in run_common -- that's for common code used by +commands which specifically execute runners.''' + +import os + +from west import log +from west.build import DEFAULT_BUILD_DIR, is_zephyr_build +from west.commands import WestCommand + +BUILD_DIR_DESCRIPTION = '''\ +Explicitly sets the build directory. If not given and the current +directory is a Zephyr build directory, it will be used; otherwise, +"{}" is assumed.'''.format(DEFAULT_BUILD_DIR) + + +def find_build_dir(dir): + '''Heuristic for finding a build directory. + + If the given argument is truthy, it is returned. Otherwise, if + the current working directory is a build directory, it is + returned. Otherwise, west.build.DEFAULT_BUILD_DIR is returned.''' + if dir: + build_dir = dir + else: + cwd = os.getcwd() + if is_zephyr_build(cwd): + build_dir = cwd + else: + build_dir = DEFAULT_BUILD_DIR + return os.path.abspath(build_dir) + + +class Forceable(WestCommand): + '''WestCommand subclass for commands with a --force option.''' + + def add_force_arg(self, parser): + '''Add a -f / --force option to the parser.''' + parser.add_argument('-f', '--force', action='store_true', + help='Ignore any errors and try to proceed') + + def check_force(self, cond, msg): + '''Abort if the command needs to be forced and hasn't been. + + The "forced" predicate must be in self.args.forced. + + If cond and self.args.force are both False, scream and die + with message msg. Otherwise, return. That is, "cond" is a + condition which means everything is OK; if it's False, only + self.args.force being True can allow execution to proceed. + ''' + if not (cond or self.args.force): + log.err(msg) + log.die('refusing to proceed without --force due to above error') From d0a63d27f85c641288d63fa182164235331a96c1 Mon Sep 17 00:00:00 2001 From: Marti Bolivar Date: Wed, 30 Jan 2019 20:30:12 -0700 Subject: [PATCH 2/3] scripts: west_commands: refactor run_common.py Pull out some more common functionality we will need elsewhere into a separate file. Signed-off-by: Marti Bolivar --- scripts/west_commands/run_common.py | 22 ++-------------------- scripts/west_commands/zephyr_ext_common.py | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/scripts/west_commands/run_common.py b/scripts/west_commands/run_common.py index 5167c028ce8c6..a237876f85316 100644 --- a/scripts/west_commands/run_common.py +++ b/scripts/west_commands/run_common.py @@ -17,7 +17,8 @@ from west.commands import CommandContextError from runners import get_runner_cls, ZephyrBinaryRunner -from runners.core import RunnerConfig + +from zephyr_ext_common import cached_runner_config # Context-sensitive help indentation. # Don't change this, or output from argparse won't match up. @@ -113,25 +114,6 @@ def desc_common(command_name): '''.format(**{'command': command_name})) -def cached_runner_config(build_dir, cache): - '''Parse the RunnerConfig from a build directory and CMake Cache.''' - board_dir = cache['ZEPHYR_RUNNER_CONFIG_BOARD_DIR'] - elf_file = cache.get('ZEPHYR_RUNNER_CONFIG_ELF_FILE', - cache['ZEPHYR_RUNNER_CONFIG_KERNEL_ELF']) - hex_file = cache.get('ZEPHYR_RUNNER_CONFIG_HEX_FILE', - cache['ZEPHYR_RUNNER_CONFIG_KERNEL_HEX']) - bin_file = cache.get('ZEPHYR_RUNNER_CONFIG_BIN_FILE', - cache['ZEPHYR_RUNNER_CONFIG_KERNEL_BIN']) - gdb = cache.get('ZEPHYR_RUNNER_CONFIG_GDB') - openocd = cache.get('ZEPHYR_RUNNER_CONFIG_OPENOCD') - openocd_search = cache.get('ZEPHYR_RUNNER_CONFIG_OPENOCD_SEARCH') - - return RunnerConfig(build_dir, board_dir, - elf_file, hex_file, bin_file, - gdb=gdb, openocd=openocd, - openocd_search=openocd_search) - - def _override_config_from_namespace(cfg, namespace): '''Override a RunnerConfig's contents with command-line values.''' for var in cfg.__slots__: diff --git a/scripts/west_commands/zephyr_ext_common.py b/scripts/west_commands/zephyr_ext_common.py index bf9d7b7bdb742..944c13e3e0332 100644 --- a/scripts/west_commands/zephyr_ext_common.py +++ b/scripts/west_commands/zephyr_ext_common.py @@ -14,6 +14,8 @@ from west.build import DEFAULT_BUILD_DIR, is_zephyr_build from west.commands import WestCommand +from runners.core import RunnerConfig + BUILD_DIR_DESCRIPTION = '''\ Explicitly sets the build directory. If not given and the current directory is a Zephyr build directory, it will be used; otherwise, @@ -58,3 +60,22 @@ def check_force(self, cond, msg): if not (cond or self.args.force): log.err(msg) log.die('refusing to proceed without --force due to above error') + + +def cached_runner_config(build_dir, cache): + '''Parse the RunnerConfig from a build directory and CMake Cache.''' + board_dir = cache['ZEPHYR_RUNNER_CONFIG_BOARD_DIR'] + elf_file = cache.get('ZEPHYR_RUNNER_CONFIG_ELF_FILE', + cache['ZEPHYR_RUNNER_CONFIG_KERNEL_ELF']) + hex_file = cache.get('ZEPHYR_RUNNER_CONFIG_HEX_FILE', + cache['ZEPHYR_RUNNER_CONFIG_KERNEL_HEX']) + bin_file = cache.get('ZEPHYR_RUNNER_CONFIG_BIN_FILE', + cache['ZEPHYR_RUNNER_CONFIG_KERNEL_BIN']) + gdb = cache.get('ZEPHYR_RUNNER_CONFIG_GDB') + openocd = cache.get('ZEPHYR_RUNNER_CONFIG_OPENOCD') + openocd_search = cache.get('ZEPHYR_RUNNER_CONFIG_OPENOCD_SEARCH') + + return RunnerConfig(build_dir, board_dir, + elf_file, hex_file, bin_file, + gdb=gdb, openocd=openocd, + openocd_search=openocd_search) From b02f5d0713e3358715f5fddd506c86fea31b4e08 Mon Sep 17 00:00:00 2001 From: Marti Bolivar Date: Wed, 30 Jan 2019 20:30:42 -0700 Subject: [PATCH 3/3] scripts: west_commands: add sign command This command is useful for signing binaries for loading by a bootloader. At present, only MCUboot's "imgtool" is supported, but it would be straightforward to add support for additional tools. Using this command instead of "plain" imgtool avoids looking up any numbers for the flash write block size, text section offset, or slot size to get a signed binary. All users need to specify is the location of the signing key. This greatly improves usability for those unfamiliar with MCUboot, or even experienced users who have to deal with multiple flash partition layouts, boards, etc. The command works by inspecting state in the Zephyr build system, some of which is also provided by the runner package. Signed-off-by: Marti Bolivar --- scripts/west-commands.yml | 5 + scripts/west_commands/sign.py | 192 ++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 scripts/west_commands/sign.py diff --git a/scripts/west-commands.yml b/scripts/west-commands.yml index bb65defe34d72..e86fe7f99944a 100644 --- a/scripts/west-commands.yml +++ b/scripts/west-commands.yml @@ -5,6 +5,11 @@ west-commands: - name: build class: Build help: compile a Zephyr application + - file: scripts/west_commands/sign.py + commands: + - name: sign + class: Sign + help: sign a Zephyr binary for bootloader chain-loading - file: scripts/west_commands/flash.py commands: - name: flash diff --git a/scripts/west_commands/sign.py b/scripts/west_commands/sign.py new file mode 100644 index 0000000000000..7e3ef56c00a4a --- /dev/null +++ b/scripts/west_commands/sign.py @@ -0,0 +1,192 @@ +# Copyright (c) 2018 Foundries.io +# +# SPDX-License-Identifier: Apache-2.0 + +import abc +import argparse +import os +import subprocess + +from west import cmake +from west import log +from west.build import is_zephyr_build +from west.util import quote_sh_list + +from runners.core import BuildConfiguration + +from zephyr_ext_common import find_build_dir, Forceable, \ + BUILD_DIR_DESCRIPTION, cached_runner_config + +SIGN_DESCRIPTION = '''\ +This command automates some of the drudgery of creating signed Zephyr +binaries for chain-loading by a bootloader. + +In the simplest usage, run this from your build directory: + + west sign -t your_tool -- ARGS_FOR_YOUR_TOOL + +Assuming your binary was properly built for processing and handling by +tool "your_tool", this creates zephyr.signed.bin and zephyr.signed.hex +files (if supported by "your_tool") which are ready for use by your +bootloader. The "ARGS_FOR_YOUR_TOOL" value can be any additional +arguments you want to pass to the tool, such as the location of a +signing key, a version identifier, etc. + +See tool-specific help below for details.''' + +SIGN_EPILOG = '''\ +imgtool +------- + +Currently, MCUboot's 'imgtool' tool is supported. To build a signed +binary you can load with MCUboot using imgtool, run this from your +build directory: + + west sign -t imgtool -- --key YOUR_SIGNING_KEY.pem + +The image header size, alignment, and slot sizes are determined from +the build directory using board information and the device tree. A +default version number of 0.0.0+0 is used (which can be overridden by +passing "--version x.y.z+w" after "--key"). As shown above, extra +arguments after a '--' are passed to imgtool directly.''' + + +class ToggleAction(argparse.Action): + + def __call__(self, parser, args, ignored, option): + setattr(args, self.dest, not option.startswith('--no-')) + + +class Sign(Forceable): + def __init__(self): + super(Sign, self).__init__( + 'sign', + # Keep this in sync with the string in west-commands.yml. + 'sign a Zephyr binary for bootloader chain-loading', + SIGN_DESCRIPTION, + accepts_unknown_args=False) + + def do_add_parser(self, parser_adder): + parser = parser_adder.add_parser( + self.name, + epilog=SIGN_EPILOG, + help=self.help, + formatter_class=argparse.RawDescriptionHelpFormatter, + description=self.description) + + parser.add_argument('-d', '--build-dir', help=BUILD_DIR_DESCRIPTION) + self.add_force_arg(parser) + + # general options + group = parser.add_argument_group('tool control options') + group.add_argument('-t', '--tool', choices=['imgtool'], + help='image signing tool name') + group.add_argument('-p', '--tool-path', default='imgtool', + help='''path to the tool itself, if needed''') + group.add_argument('tool_args', nargs='*', metavar='tool_opt', + help='extra option(s) to pass to the signing tool') + + # bin file options + group = parser.add_argument_group('binary (.bin) file options') + group.add_argument('--bin', '--no-bin', dest='gen_bin', nargs=0, + action=ToggleAction, + help='''produce a signed .bin file? + (default: yes, if supported)''') + group.add_argument('-B', '--sbin', metavar='BIN', + default='zephyr.signed.bin', + help='''signed .bin file name + (default: zephyr.signed.bin)''') + + # hex file options + group = parser.add_argument_group('Intel HEX (.hex) file options') + group.add_argument('--hex', '--no-hex', dest='gen_hex', nargs=0, + action=ToggleAction, + help='''produce a signed .hex file? + (default: yes, if supported)''') + group.add_argument('-H', '--shex', metavar='HEX', + default='zephyr.signed.hex', + help='''signed .hex file name + (default: zephyr.signed.hex)''') + + # defaults for hex/bin generation + parser.set_defaults(gen_bin=True, gen_hex=True) + + return parser + + def do_run(self, args, ignored): + if not (args.gen_bin or args.gen_hex): + return + + self.check_force(os.path.isdir(args.build_dir), + 'no such build directory {}'.format(args.build_dir)) + self.check_force(is_zephyr_build(args.build_dir), + "build directory {} doesn't look like a Zephyr build " + 'directory'.format(args.build_dir)) + + if args.tool == 'imgtool': + signer = ImgtoolSigner() + # (Add support for other signers here in elif blocks) + else: + raise RuntimeError("can't happen") + + # Provide the build directory if not given, and defer to the signer. + args.build_dir = find_build_dir(args.build_dir) + signer.sign(args) + + +class Signer(abc.ABC): + '''Common abstract superclass for signers. + + To add support for a new tool, subclass this and add support for + it in the Sign.do_run() method.''' + + @abc.abstractmethod + def sign(self, args): + '''Abstract method to perform a signature; subclasses must implement. + + :param args: parsed arguments from Sign command + ''' + + +class ImgtoolSigner(Signer): + + def sign(self, args): + cache = cmake.CMakeCache.from_build_dir(args.build_dir) + runner_config = cached_runner_config(args.build_dir, cache) + bcfg = BuildConfiguration(args.build_dir) + + # Build a signed .bin + if args.gen_bin and runner_config.bin_file: + sign_bin = self.sign_cmd(args, bcfg, runner_config.bin_file, + args.sbin) + log.dbg(quote_sh_list(sign_bin)) + subprocess.check_call(sign_bin) + + # Build a signed .hex + if args.gen_hex and runner_config.hex_file: + sign_hex = self.sign_cmd(args, bcfg, runner_config.hex_file, + args.shex) + log.dbg(quote_sh_list(sign_hex)) + subprocess.check_call(sign_hex) + + def sign_cmd(self, args, bcfg, infile, outfile): + align = str(bcfg['FLASH_WRITE_BLOCK_SIZE']) + vtoff = str(bcfg['CONFIG_TEXT_SECTION_OFFSET']) + slot_size = str(bcfg['FLASH_AREA_IMAGE_0_SIZE']) + + sign_command = [args.tool_path or 'imgtool', + 'sign', + '--align', align, + '--header-size', vtoff, + '--slot-size', slot_size, + # We provide a default --version in case the + # user is just messing around and doesn't want + # to set one. It will be overridden if there is + # a --version in args.tool_args. + '--version', '0.0.0+0', + infile, + outfile] + + sign_command.extend(args.tool_args) + + return sign_command