binman: Convert to use ArgumentParser
This class is the new way to handle arguments in Python. Convert binman
over to use it. At the same time, introduce commands so that we can
separate out the different parts of binman functionality.
Signed-off-by: Simon Glass <sjg@chromium.org>
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 1b9db0c..2edc380 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -176,7 +176,7 @@
./tools/buildman/buildman -P sandbox_spl &&
export PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt";
export PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}";
- ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools -t &&
+ ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test &&
./tools/dtoc/dtoc -t
# Test sandbox with test.py
diff --git a/.travis.yml b/.travis.yml
index b971677..38fc103 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -147,7 +147,7 @@
if [[ -n "${TEST_PY_TOOLS}" ]]; then
PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt"
PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}"
- ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools -t &&
+ ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test &&
./tools/patman/patman --test &&
./tools/buildman/buildman -t &&
PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt"
diff --git a/Makefile b/Makefile
index 73fdf70..704579b 100644
--- a/Makefile
+++ b/Makefile
@@ -1196,9 +1196,9 @@
# ---------------------------------------------------------------------------
# Use 'make BINMAN_DEBUG=1' to enable debugging
quiet_cmd_binman = BINMAN $@
-cmd_binman = $(srctree)/tools/binman/binman -u -d u-boot.dtb -O . -m \
+cmd_binman = $(srctree)/tools/binman/binman build -u -d u-boot.dtb -O . -m \
-I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
- $(if $(BINMAN_DEBUG),-D) $(BINMAN_$(@F)) $<
+ $(if $(BINMAN_DEBUG),-D) $(BINMAN_$(@F))
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
diff --git a/test/run b/test/run
index b97647e..d635622 100755
--- a/test/run
+++ b/test/run
@@ -40,7 +40,7 @@
export DTC=${DTC_DIR}/dtc
TOOLS_DIR=build-sandbox_spl/tools
-run_test "binman" ./tools/binman/binman -t --toolpath ${TOOLS_DIR}
+run_test "binman" ./tools/binman/binman --toolpath ${TOOLS_DIR} test
run_test "patman" ./tools/patman/patman --test
[ "$1" == "quick" ] && skip=--skip-net-tests
@@ -52,7 +52,7 @@
# To enable Python test coverage on Debian-type distributions (e.g. Ubuntu):
# $ sudo apt-get install python-pytest python-coverage
export PATH=$PATH:${TOOLS_DIR}
-run_test "binman code coverage" ./tools/binman/binman -T --toolpath ${TOOLS_DIR}
+run_test "binman code coverage" ./tools/binman/binman test -T
run_test "dtoc code coverage" ./tools/dtoc/dtoc -T
run_test "fdt code coverage" ./tools/dtoc/test_fdt -T
diff --git a/tools/binman/README b/tools/binman/README
index ef62d1f..fe734c1 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -187,7 +187,7 @@
Type:
- binman -b <board_name>
+ binman build -b <board_name>
to build an image for a board. The board name is the same name used when
configuring U-Boot (e.g. for sandbox_defconfig the board name is 'sandbox').
@@ -195,7 +195,7 @@
Or you can specify this explicitly:
- binman -I <build_path>
+ binman build -I <build_path>
where <build_path> is the build directory containing the output of the U-Boot
build.
@@ -483,7 +483,7 @@
For details on the various entry types supported by binman and how to use them,
see README.entries. This is generated from the source code using:
- binman -E >tools/binman/README.entries
+ binman entry-docs >tools/binman/README.entries
Hashing Entries
@@ -715,7 +715,7 @@
-------------
Binman is a critical tool and is designed to be very testable. Entry
-implementations target 100% test coverage. Run 'binman -T' to check this.
+implementations target 100% test coverage. Run 'binman test -T' to check this.
To enable Python test coverage on Debian-type distributions (e.g. Ubuntu):
diff --git a/tools/binman/binman.py b/tools/binman/binman.py
index 613aad5..8bd5868 100755
--- a/tools/binman/binman.py
+++ b/tools/binman/binman.py
@@ -20,14 +20,15 @@
import traceback
import unittest
-# Bring in the patman and dtoc libraries
+# Bring in the patman and dtoc libraries (but don't override the first path
+# in PYTHONPATH)
our_path = os.path.dirname(os.path.realpath(__file__))
for dirname in ['../patman', '../dtoc', '..', '../concurrencytest']:
- sys.path.insert(0, os.path.join(our_path, dirname))
+ sys.path.insert(2, os.path.join(our_path, dirname))
# Bring in the libfdt module
-sys.path.insert(0, 'scripts/dtc/pylibfdt')
-sys.path.insert(0, os.path.join(our_path,
+sys.path.insert(2, 'scripts/dtc/pylibfdt')
+sys.path.insert(2, os.path.join(our_path,
'../../build-sandbox_spl/scripts/dtc/pylibfdt'))
# When running under python-coverage on Ubuntu 16.04, the dist-packages
@@ -59,7 +60,7 @@
on the command line.
processes: Number of processes to use to run tests (None=same as #CPUs)
args: List of positional args provided to binman. This can hold a test
- name to execute (as in 'binman -t testSections', for example)
+ name to execute (as in 'binman test testSections', for example)
toolpath: List of paths to use for tools
"""
import cbfs_util_test
@@ -98,7 +99,7 @@
setup_test_args = getattr(module, 'setup_test_args')
setup_test_args(preserve_indir=test_preserve_dirs,
preserve_outdirs=test_preserve_dirs and test_name is not None,
- toolpath=toolpath)
+ toolpath=toolpath, verbosity=verbosity)
if test_name:
try:
suite.addTests(loader.loadTestsFromName(test_name, module))
@@ -158,37 +159,36 @@
for item in glob_list if '_testing' not in item])
test_util.RunTestCoverage('tools/binman/binman.py', None,
['*test*', '*binman.py', 'tools/patman/*', 'tools/dtoc/*'],
- options.build_dir, all_set)
+ args.build_dir, all_set)
-def RunBinman(options, args):
+def RunBinman(args):
"""Main entry point to binman once arguments are parsed
Args:
- options: Command-line options
- args: Non-option arguments
+ args: Command line arguments Namespace object
"""
ret_code = 0
- if not options.debug:
+ if not args.debug:
sys.tracebacklimit = 0
- if options.test:
- ret_code = RunTests(options.debug, options.verbosity, options.processes,
- options.test_preserve_dirs, args[1:],
- options.toolpath)
-
- elif options.test_coverage:
- RunTestCoverage()
+ if args.cmd == 'test':
+ if args.test_coverage:
+ RunTestCoverage()
+ else:
+ ret_code = RunTests(args.debug, args.verbosity, args.processes,
+ args.test_preserve_dirs, args.tests,
+ args.toolpath)
- elif options.entry_docs:
+ elif args.cmd == 'entry-docs':
control.WriteEntryDocs(GetEntryModules())
else:
try:
- ret_code = control.Binman(options, args)
+ ret_code = control.Binman(args)
except Exception as e:
print('binman: %s' % e)
- if options.debug:
+ if args.debug:
print()
traceback.print_exc()
ret_code = 1
@@ -196,6 +196,7 @@
if __name__ == "__main__":
- (options, args) = cmdline.ParseArgs(sys.argv)
- ret_code = RunBinman(options, args)
+ args = cmdline.ParseArgs(sys.argv[1:])
+
+ ret_code = RunBinman(args)
sys.exit(ret_code)
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index 91e007e..a002105 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -5,7 +5,7 @@
# Command-line parser for binman
#
-from optparse import OptionParser
+from argparse import ArgumentParser
def ParseArgs(argv):
"""Parse the binman command-line arguments
@@ -17,56 +17,64 @@
options provides access to the options (e.g. option.debug)
args is a list of string arguments
"""
- parser = OptionParser()
- parser.add_option('-a', '--entry-arg', type='string', action='append',
+ if '-H' in argv:
+ argv.append('build')
+
+ epilog = '''Binman creates and manipulate images for a board from a set of binaries. Binman is
+controlled by a description in the board device tree.'''
+
+ parser = ArgumentParser(epilog=epilog)
+ parser.add_argument('-B', '--build-dir', type=str, default='b',
+ help='Directory containing the build output')
+ parser.add_argument('-D', '--debug', action='store_true',
+ help='Enabling debugging (provides a full traceback on error)')
+ parser.add_argument('-H', '--full-help', action='store_true',
+ default=False, help='Display the README file')
+ parser.add_argument('--toolpath', type=str, action='append',
+ help='Add a path to the directories containing tools')
+ parser.add_argument('-v', '--verbosity', default=1,
+ type=int, help='Control verbosity: 0=silent, 1=warnings, 2=notices, '
+ '3=info, 4=detail, 5=debug')
+
+ subparsers = parser.add_subparsers(dest='cmd')
+
+ build_parser = subparsers.add_parser('build', help='Build firmware image')
+ build_parser.add_argument('-a', '--entry-arg', type=str, action='append',
help='Set argument value arg=value')
- parser.add_option('-b', '--board', type='string',
+ build_parser.add_argument('-b', '--board', type=str,
help='Board name to build')
- parser.add_option('-B', '--build-dir', type='string', default='b',
- help='Directory containing the build output')
- parser.add_option('-d', '--dt', type='string',
+ build_parser.add_argument('-d', '--dt', type=str,
help='Configuration file (.dtb) to use')
- parser.add_option('-D', '--debug', action='store_true',
- help='Enabling debugging (provides a full traceback on error)')
- parser.add_option('-E', '--entry-docs', action='store_true',
- help='Write out entry documentation (see README.entries)')
- parser.add_option('--fake-dtb', action='store_true',
+ build_parser.add_argument('--fake-dtb', action='store_true',
help='Use fake device tree contents (for testing only)')
- parser.add_option('-i', '--image', type='string', action='append',
+ build_parser.add_argument('-i', '--image', type=str, action='append',
help='Image filename to build (if not specified, build all)')
- parser.add_option('-I', '--indir', action='append',
+ build_parser.add_argument('-I', '--indir', action='append',
help='Add a path to the list of directories to use for input files')
- parser.add_option('-H', '--full-help', action='store_true',
- default=False, help='Display the README file')
- parser.add_option('-m', '--map', action='store_true',
+ build_parser.add_argument('-m', '--map', action='store_true',
default=False, help='Output a map file for each image')
- parser.add_option('-O', '--outdir', type='string',
+ build_parser.add_argument('-O', '--outdir', type=str,
action='store', help='Path to directory to use for intermediate and '
'output files')
- parser.add_option('-p', '--preserve', action='store_true',\
+ build_parser.add_argument('-p', '--preserve', action='store_true',\
help='Preserve temporary output directory even if option -O is not '
'given')
- parser.add_option('-P', '--processes', type=int,
- help='set number of processes to use for running tests')
- parser.add_option('-t', '--test', action='store_true',
- default=False, help='run tests')
- parser.add_option('-T', '--test-coverage', action='store_true',
- default=False, help='run tests and check for 100% coverage')
- parser.add_option('--toolpath', type='string', action='append',
- help='Add a path to the directories containing tools')
- parser.add_option('-u', '--update-fdt', action='store_true',
+ build_parser.add_argument('-u', '--update-fdt', action='store_true',
default=False, help='Update the binman node with offset/size info')
- parser.add_option('-v', '--verbosity', default=1,
- type='int', help='Control verbosity: 0=silent, 1=progress, 3=full, '
- '4=debug')
- parser.add_option('-X', '--test-preserve-dirs', action='store_true',
+
+ entry_parser = subparsers.add_parser('entry-docs',
+ help='Write out entry documentation (see README.entries)')
+
+ test_parser = subparsers.add_parser('test', help='Run tests')
+ test_parser.add_argument('-P', '--processes', type=int,
+ help='set number of processes to use for running tests')
+ test_parser.add_argument('-T', '--test-coverage', action='store_true',
+ default=False, help='run tests and check for 100%% coverage')
+ test_parser.add_argument('-X', '--test-preserve-dirs', action='store_true',
help='Preserve and display test-created input directories; also '
'preserve the output directory if a single test is run (pass test '
'name at the end of the command line')
-
- parser.usage += """
-
-Create images for a board from a set of binaries. It is controlled by a
-description in the board device tree."""
+ test_parser.add_argument('tests', nargs='*',
+ help='Test names to run (omit for all)')
return parser.parse_args(argv)
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 4a94afc..9022cf7 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -67,19 +67,18 @@
from entry import Entry
Entry.WriteDocs(modules, test_missing)
-def Binman(options, args):
+def Binman(args):
"""The main control code for binman
This assumes that help and test options have already been dealt with. It
deals with the core task of building images.
Args:
- options: Command line options object
- args: Command line arguments (list of strings)
+ args: Command line arguments Namespace object
"""
global images
- if options.full_help:
+ if args.full_help:
pager = os.getenv('PAGER')
if not pager:
pager = 'more'
@@ -89,17 +88,17 @@
return 0
# Try to figure out which device tree contains our image description
- if options.dt:
- dtb_fname = options.dt
+ if args.dt:
+ dtb_fname = args.dt
else:
- board = options.board
+ board = args.board
if not board:
raise ValueError('Must provide a board to process (use -b <board>)')
- board_pathname = os.path.join(options.build_dir, board)
+ board_pathname = os.path.join(args.build_dir, board)
dtb_fname = os.path.join(board_pathname, 'u-boot.dtb')
- if not options.indir:
- options.indir = ['.']
- options.indir.append(board_pathname)
+ if not args.indir:
+ args.indir = ['.']
+ args.indir.append(board_pathname)
try:
# Import these here in case libfdt.py is not available, in which case
@@ -107,15 +106,15 @@
import fdt
import fdt_util
- tout.Init(options.verbosity)
- elf.debug = options.debug
- cbfs_util.VERBOSE = options.verbosity > 2
- state.use_fake_dtb = options.fake_dtb
+ tout.Init(args.verbosity)
+ elf.debug = args.debug
+ cbfs_util.VERBOSE = args.verbosity > 2
+ state.use_fake_dtb = args.fake_dtb
try:
- tools.SetInputDirs(options.indir)
- tools.PrepareOutputDir(options.outdir, options.preserve)
- tools.SetToolPaths(options.toolpath)
- state.SetEntryArgs(options.entry_arg)
+ tools.SetInputDirs(args.indir)
+ tools.PrepareOutputDir(args.outdir, args.preserve)
+ tools.SetToolPaths(args.toolpath)
+ state.SetEntryArgs(args.entry_arg)
# Get the device tree ready by compiling it and copying the compiled
# output into a file in our output directly. Then scan it for use
@@ -132,16 +131,16 @@
images = _ReadImageDesc(node)
- if options.image:
+ if args.image:
skip = []
new_images = OrderedDict()
for name, image in images.items():
- if name in options.image:
+ if name in args.image:
new_images[name] = image
else:
skip.append(name)
images = new_images
- if skip and options.verbosity >= 2:
+ if skip and args.verbosity >= 2:
print('Skipping images: %s' % ', '.join(skip))
state.Prepare(images, dtb)
@@ -155,7 +154,7 @@
# entry offsets remain the same.
for image in images.values():
image.ExpandEntries()
- if options.update_fdt:
+ if args.update_fdt:
image.AddMissingProperties()
image.ProcessFdt(dtb)
@@ -176,19 +175,19 @@
image.CheckSize()
image.CheckEntries()
except Exception as e:
- if options.map:
+ if args.map:
fname = image.WriteMap()
print("Wrote map file '%s' to show errors" % fname)
raise
image.SetImagePos()
- if options.update_fdt:
+ if args.update_fdt:
image.SetCalculatedProperties()
for dtb_item in state.GetFdts():
dtb_item.Sync()
image.ProcessEntryContents()
image.WriteSymbols()
image.BuildImage()
- if options.map:
+ if args.map:
image.WriteMap()
# Write the updated FDTs to our output files
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6fdecb2..c675299 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -155,7 +155,7 @@
@classmethod
def setup_test_args(cls, preserve_indir=False, preserve_outdirs=False,
- toolpath=None):
+ toolpath=None, verbosity=None):
"""Accept arguments controlling test execution
Args:
@@ -169,6 +169,7 @@
cls.preserve_indir = preserve_indir
cls.preserve_outdirs = preserve_outdirs
cls.toolpath = toolpath
+ cls.verbosity = verbosity
def _CheckLz4(self):
if not self.have_lz4:
@@ -192,19 +193,6 @@
TestFunctional._MakeInputFile('spl/u-boot-spl.dtb', U_BOOT_SPL_DTB_DATA)
TestFunctional._MakeInputFile('tpl/u-boot-tpl.dtb', U_BOOT_TPL_DTB_DATA)
- def _GetVerbosity(self):
- """Check if verbosity should be enabled
-
- Returns:
- list containing either:
- - Verbosity flag (e.g. '-v2') if it is present on the cmd line
- - nothing if the flag is not present
- """
- for arg in sys.argv[1:]:
- if arg.startswith('-v'):
- return [arg]
- return []
-
def _RunBinman(self, *args, **kwargs):
"""Run binman using the command line
@@ -219,7 +207,7 @@
result.stdout + result.stderr))
return result
- def _DoBinman(self, *args):
+ def _DoBinman(self, *argv):
"""Run binman using directly (in the same process)
Args:
@@ -227,16 +215,14 @@
Returns:
Return value (0 for success)
"""
- args = list(args)
- if '-D' in sys.argv:
- args = args + ['-D']
- (options, args) = cmdline.ParseArgs(args)
- options.pager = 'binman-invalid-pager'
- options.build_dir = self._indir
+ argv = list(argv)
+ args = cmdline.ParseArgs(argv)
+ args.pager = 'binman-invalid-pager'
+ args.build_dir = self._indir
# For testing, you can force an increase in verbosity here
- # options.verbosity = tout.DEBUG
- return control.Binman(options, args)
+ # args.verbosity = tout.DEBUG
+ return control.Binman(args)
def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False,
entry_args=None, images=None, use_real_dtb=False,
@@ -254,28 +240,29 @@
value: value of that arg
images: List of image names to build
"""
- args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)]
+ args = []
if debug:
args.append('-D')
+ if verbosity is not None:
+ args.append('-v%d' % verbosity)
+ elif self.verbosity:
+ args.append('-v%d' % self.verbosity)
+ if self.toolpath:
+ for path in self.toolpath:
+ args += ['--toolpath', path]
+ args += ['build', '-p', '-I', self._indir, '-d', self.TestFile(fname)]
if map:
args.append('-m')
if update_dtb:
args.append('-u')
if not use_real_dtb:
args.append('--fake-dtb')
- if verbosity is not None:
- args.append('-v%d' % verbosity)
- else:
- args += self._GetVerbosity()
if entry_args:
for arg, value in entry_args.items():
args.append('-a%s=%s' % (arg, value))
if images:
for image in images:
args += ['-i', image]
- if self.toolpath:
- for path in self.toolpath:
- args += ['--toolpath', path]
return self._DoBinman(*args)
def _SetupDtb(self, fname, outfile='u-boot.dtb'):
@@ -538,20 +525,20 @@
"""Test that we can run it with a specific board"""
self._SetupDtb('005_simple.dts', 'sandbox/u-boot.dtb')
TestFunctional._MakeInputFile('sandbox/u-boot.bin', U_BOOT_DATA)
- result = self._DoBinman('-b', 'sandbox')
+ result = self._DoBinman('build', '-b', 'sandbox')
self.assertEqual(0, result)
def testNeedBoard(self):
"""Test that we get an error when no board ius supplied"""
with self.assertRaises(ValueError) as e:
- result = self._DoBinman()
+ result = self._DoBinman('build')
self.assertIn("Must provide a board to process (use -b <board>)",
str(e.exception))
def testMissingDt(self):
"""Test that an invalid device-tree file generates an error"""
with self.assertRaises(Exception) as e:
- self._RunBinman('-d', 'missing_file')
+ self._RunBinman('build', '-d', 'missing_file')
# We get one error from libfdt, and a different one from fdtget.
self.AssertInList(["Couldn't open blob from 'missing_file'",
'No such file or directory'], str(e.exception))
@@ -563,26 +550,26 @@
will come from the device-tree compiler (dtc).
"""
with self.assertRaises(Exception) as e:
- self._RunBinman('-d', self.TestFile('001_invalid.dts'))
+ self._RunBinman('build', '-d', self.TestFile('001_invalid.dts'))
self.assertIn("FATAL ERROR: Unable to parse input tree",
str(e.exception))
def testMissingNode(self):
"""Test that a device tree without a 'binman' node generates an error"""
with self.assertRaises(Exception) as e:
- self._DoBinman('-d', self.TestFile('002_missing_node.dts'))
+ self._DoBinman('build', '-d', self.TestFile('002_missing_node.dts'))
self.assertIn("does not have a 'binman' node", str(e.exception))
def testEmpty(self):
"""Test that an empty binman node works OK (i.e. does nothing)"""
- result = self._RunBinman('-d', self.TestFile('003_empty.dts'))
+ result = self._RunBinman('build', '-d', self.TestFile('003_empty.dts'))
self.assertEqual(0, len(result.stderr))
self.assertEqual(0, result.return_code)
def testInvalidEntry(self):
"""Test that an invalid entry is flagged"""
with self.assertRaises(Exception) as e:
- result = self._RunBinman('-d',
+ result = self._RunBinman('build', '-d',
self.TestFile('004_invalid_entry.dts'))
self.assertIn("Unknown entry type 'not-a-valid-type' in node "
"'/binman/not-a-valid-type'", str(e.exception))
@@ -1313,7 +1300,8 @@
def testEntryArgsInvalidFormat(self):
"""Test that an invalid entry-argument format is detected"""
- args = ['-d', self.TestFile('064_entry_args_required.dts'), '-ano-value']
+ args = ['build', '-d', self.TestFile('064_entry_args_required.dts'),
+ '-ano-value']
with self.assertRaises(ValueError) as e:
self._DoBinman(*args)
self.assertIn("Invalid entry arguemnt 'no-value'", str(e.exception))
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index ea36cd1..4009815 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -46,9 +46,10 @@
glob_list = []
glob_list += exclude_list
glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*']
+ test_cmd = 'test' if 'binman.py' in prog else '-t'
cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools %s-coverage run '
- '--omit "%s" %s -P1 -t' % (build_dir, PYTHON, ','.join(glob_list),
- prog))
+ '--omit "%s" %s %s -P1' % (build_dir, PYTHON, ','.join(glob_list),
+ prog, test_cmd))
os.system(cmd)
stdout = command.Output('%s-coverage' % PYTHON, 'report')
lines = stdout.splitlines()