west: sign.py: usability fixes

Fix some usability issues with this command.

- add help making it clear that either imgtool must be installed, or
  the path to imgtool.py must be provided using --tool-path.

- in case people don't read that, print a helpful message if imgtool
  is not installed and --tool-path is not provided.

- in case the build directory is not properly set up for an MCUboot
  chain-loaded image, make the BuildConfiguration inspection more
  robust, printing out errors using check_force() when values are
  missing.

- mark the --tool option required to print usage and avoid a
  RuntimeError if it is not provided.

- make sure we search for the default build directory before checking
  for its existence, in case it was not provided

Signed-off-by: Marti Bolivar <marti@foundries.io>
This commit is contained in:
Marti Bolivar 2019-02-14 14:49:03 -07:00 committed by Carles Cufí
commit 4a0f1f2817

View file

@ -5,6 +5,7 @@
import abc
import argparse
import os
import shutil
import subprocess
from west import cmake
@ -44,11 +45,14 @@ build directory:
west sign -t imgtool -- --key YOUR_SIGNING_KEY.pem
For this to work, either imgtool must be installed (e.g. using pip3),
or you must pass the path to imgtool.py using the -p option.
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.'''
the build directory using .config 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):
@ -79,9 +83,10 @@ class Sign(Forceable):
# 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',
group.add_argument('-t', '--tool', choices=['imgtool'], required=True,
help='''image signing tool name; only imgtool is
currently supported''')
group.add_argument('-p', '--tool-path', default=None,
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')
@ -117,6 +122,9 @@ class Sign(Forceable):
if not (args.gen_bin or args.gen_hex):
return
# Provide the build directory if not given, and defer to the signer.
args.build_dir = find_build_dir(args.build_dir)
self.args = args # for check_force
self.check_force(os.path.isdir(args.build_dir),
@ -131,9 +139,7 @@ class Sign(Forceable):
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)
signer.sign(self)
class Signer(abc.ABC):
@ -143,52 +149,79 @@ class Signer(abc.ABC):
it in the Sign.do_run() method.'''
@abc.abstractmethod
def sign(self, args):
def sign(self, command):
'''Abstract method to perform a signature; subclasses must implement.
:param args: parsed arguments from Sign command
:param command: the Sign instance
'''
class ImgtoolSigner(Signer):
def sign(self, args):
def sign(self, command):
args = command.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,
sign_bin = self.sign_cmd(command, 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,
sign_hex = self.sign_cmd(command, 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['DT_FLASH_WRITE_BLOCK_SIZE'])
vtoff = str(bcfg['CONFIG_TEXT_SECTION_OFFSET'])
slot_size = str(bcfg['DT_FLASH_AREA_IMAGE_0_SIZE'])
def sign_cmd(self, command, bcfg, infile, outfile):
align, vtoff, slot_size = [self.get_cfg_str(command, bcfg, x) for x in
('DT_FLASH_WRITE_BLOCK_SIZE',
'CONFIG_TEXT_SECTION_OFFSET',
'DT_FLASH_AREA_IMAGE_0_SIZE')]
align_arg = ['--align', align] if align else []
header_arg = ['--header-size', vtoff] if vtoff else []
slot_arg = ['--slot-size', slot_size] if slot_size else []
sign_command = [args.tool_path or 'imgtool',
'sign',
'--align', align,
'--header-size', vtoff,
'--slot-size', slot_size,
args = command.args
if args.tool_path:
tool_path = args.tool_path
else:
tool_path = shutil.which('imgtool')
if not tool_path:
log.die('imgtool not found; either install it',
'(e.g. "pip3 install imgtool") or provide --tool-path')
sign_command = ([tool_path,
'sign'] +
align_arg +
header_arg +
slot_arg +
# 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]
['--version', '0.0.0+0',
infile,
outfile])
sign_command.extend(args.tool_args)
return sign_command
def get_cfg_str(self, command, bcfg, item):
try:
return str(bcfg[item])
except KeyError:
command.check_force(
False,
"imgtool parameter unknown, build directory has no {} {}".
format('device tree define' if item.startswith('DT_') else
'Kconfig option',
item))
return None