From 0b3d24a7790d436bb611e5e10c4d0c06ea04fd3c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:48 -0600 Subject: patman: Use test_util to show test results This handles skipped tests correctly, so use it instead of the existing code. Signed-off-by: Simon Glass --- tools/patman/main.py | 8 ++------ tools/patman/test_util.py | 6 +++--- 2 files changed, 5 insertions(+), 9 deletions(-) (limited to 'tools') diff --git a/tools/patman/main.py b/tools/patman/main.py index 28a9a260879..03668d1bb8c 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -25,6 +25,7 @@ from patman import patchstream from patman import project from patman import settings from patman import terminal +from patman import test_util from patman import test_checkpatch @@ -101,12 +102,7 @@ elif options.test: suite = doctest.DocTestSuite(module) suite.run(result) - # TODO: Surely we can just 'print' result? - print(result) - for test, err in result.errors: - print(err) - for test, err in result.failures: - print(err) + sys.exit(test_util.ReportResult('patman', None, result)) # Called from git with a patch filename as argument # Printout a list of additional CC recipients for this patch diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index aac58fb72f0..0827488f33d 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult): for test, err in result.failures: print(err, result.failures) if result.skipped: - print('%d binman test%s SKIPPED:' % - (len(result.skipped), 's' if len(result.skipped) > 1 else '')) + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname, + 's' if len(result.skipped) > 1 else '')) for skip_info in result.skipped: print('%s: %s' % (skip_info[0], skip_info[1])) if result.errors or result.failures: - print('binman tests FAILED') + print('%s tests FAILED' % toolname) return 1 return 0 -- cgit v1.2.3 From 7d5b04e8a533402799c39468ac880a22f67ffbc9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:49 -0600 Subject: patman: Move main code out to a control module To make testing easier, move the code out from main into a separate 'control' module and split it into four parts: setup, preparing patches, checking patches and emailing patches. Add comments and fix a few code-style issues while we are here. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 6 ++ tools/patman/control.py | 170 +++++++++++++++++++++++++++++++++++++++++++++ tools/patman/gitutil.py | 4 +- tools/patman/main.py | 57 +-------------- tools/patman/series.py | 2 +- 5 files changed, 182 insertions(+), 57 deletions(-) create mode 100644 tools/patman/control.py (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 07c3e2739ab..263bac3fc90 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -41,6 +41,12 @@ def FindCheckPatch(): def CheckPatch(fname, verbose=False, show_types=False): """Run checkpatch.pl on a file. + Args: + fname: Filename to check + verbose: True to print out every line of the checkpatch output as it is + parsed + show_types: Tell checkpatch to show the type (number) of each message + Returns: namedtuple containing: ok: False=failure, True=ok diff --git a/tools/patman/control.py b/tools/patman/control.py new file mode 100644 index 00000000000..a896c924b5f --- /dev/null +++ b/tools/patman/control.py @@ -0,0 +1,170 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2020 Google LLC +# +"""Handles the main control logic of patman + +This module provides various functions called by the main program to implement +the features of patman. +""" + +import os +import sys + +from patman import checkpatch +from patman import gitutil +from patman import patchstream +from patman import terminal + +def setup(): + """Do required setup before doing anything""" + gitutil.Setup() + +def prepare_patches(col, count, start, ignore_binary): + """Figure out what patches to generate, then generate them + + The patch files are written to the current directory, e.g. 0001_xxx.patch + 0002_yyy.patch + + Args: + col (terminal.Color): Colour output object + count (int): Number of patches to produce, or -1 to produce patches for + the current branch back to the upstream commit + start (int): Start partch to use (0=first / top of branch) + ignore_binary (bool): Don't generate patches for binary files + + Returns: + Tuple: + Series object for this series (set of patches) + Filename of the cover letter as a string (None if none) + patch_files: List of patch filenames, each a string, e.g. + ['0001_xxx.patch', '0002_yyy.patch'] + """ + if count == -1: + # Work out how many patches to send if we can + count = (gitutil.CountCommitsToBranch() - start) + + if not count: + sys.exit(col.Color(col.RED, + 'No commits found to process - please use -c flag')) + + # Read the metadata from the commits + to_do = count + series = patchstream.GetMetaData(start, to_do) + cover_fname, patch_files = gitutil.CreatePatches( + start, to_do, ignore_binary, series) + + # Fix up the patch files to our liking, and insert the cover letter + patchstream.FixPatches(series, patch_files) + if cover_fname and series.get('cover'): + patchstream.InsertCoverLetter(cover_fname, series, to_do) + return series, cover_fname, patch_files + +def check_patches(series, patch_files, run_checkpatch, verbose): + """Run some checks on a set of patches + + This santiy-checks the patman tags like Series-version and runs the patches + through checkpatch + + Args: + series (Series): Series object for this series (set of patches) + patch_files (list): List of patch filenames, each a string, e.g. + ['0001_xxx.patch', '0002_yyy.patch'] + run_checkpatch (bool): True to run checkpatch.pl + verbose (bool): True to print out every line of the checkpatch output as + it is parsed + + Returns: + bool: True if the patches had no errors, False if they did + """ + # Do a few checks on the series + series.DoChecks() + + # Check the patches, and run them through 'git am' just to be sure + if run_checkpatch: + ok = checkpatch.CheckPatches(verbose, patch_files) + else: + ok = True + return ok + + +def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, + ignore_bad_tags, add_maintainers, limit, dry_run, in_reply_to, + thread, smtp_server): + """Email patches to the recipients + + This emails out the patches and cover letter using 'git send-email'. Each + patch is copied to recipients identified by the patch tag and output from + the get_maintainer.pl script. The cover letter is copied to all recipients + of any patch. + + To make this work a CC file is created holding the recipients for each patch + and the cover letter. See the main program 'cc_cmd' for this logic. + + Args: + col (terminal.Color): Colour output object + series (Series): Series object for this series (set of patches) + cover_fname (str): Filename of the cover letter as a string (None if + none) + patch_files (list): List of patch filenames, each a string, e.g. + ['0001_xxx.patch', '0002_yyy.patch'] + process_tags (bool): True to process subject tags in each patch, e.g. + for 'dm: spi: Add SPI support' this would be 'dm' and 'spi'. The + tags are looked up in the configured sendemail.aliasesfile and also + in ~/.patman (see README) + its_a_go (bool): True if we are going to actually send the patches, + False if the patches have errors and will not be sent unless + @ignore_errors + ignore_bad_tags (bool): True to just print a warning for unknown tags, + False to halt with an error + add_maintainers (bool): Run the get_maintainer.pl script for each patch + limit (int): Limit on the number of people that can be cc'd on a single + patch or the cover letter (None if no limit) + dry_run (bool): Don't actually email the patches, just print out what + would be sent + in_reply_to (str): If not None we'll pass this to git as --in-reply-to. + Should be a message ID that this is in reply to. + thread (bool): True to add --thread to git send-email (make all patches + reply to cover-letter or first patch in series) + smtp_server (str): SMTP server to use to send patches (None for default) + """ + cc_file = series.MakeCcFile(process_tags, cover_fname, not ignore_bad_tags, + add_maintainers, limit) + + # Email the patches out (giving the user time to check / cancel) + cmd = '' + if its_a_go: + cmd = gitutil.EmailPatches( + series, cover_fname, patch_files, dry_run, not ignore_bad_tags, + cc_file, in_reply_to=in_reply_to, thread=thread, + smtp_server=smtp_server) + else: + print(col.Color(col.RED, "Not sending emails due to errors/warnings")) + + # For a dry run, just show our actions as a sanity check + if dry_run: + series.ShowActions(patch_files, cmd, process_tags) + if not its_a_go: + print(col.Color(col.RED, "Email would not be sent")) + + os.remove(cc_file) + +def send(options): + """Create, check and send patches by email + + Args: + options (optparse.Values): Arguments to patman + """ + setup() + col = terminal.Color() + series, cover_fname, patch_files = prepare_patches( + col, options.count, options.start, options.ignore_binary) + ok = check_patches(series, patch_files, options.check_patch, + options.verbose) + its_a_go = ok or options.ignore_errors + if its_a_go: + email_patches( + col, series, cover_fname, patch_files, options.process_tags, + its_a_go, options.ignore_bad_tags, options.add_maintainers, + options.limit, options.dry_run, options.in_reply_to, options.thread, + options.smtp_server) diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 5189840eaba..29444bf8e92 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -261,8 +261,10 @@ def CreatePatches(start, count, ignore_binary, series): Args: start: Commit to start from: 0=HEAD, 1=next one, etc. count: number of commits to include + ignore_binary: Don't generate patches for binary files + series: Series object for this series (set of patches) Return: - Filename of cover letter + Filename of cover letter (None if none) List of filenames of patch files """ if series.get('version'): diff --git a/tools/patman/main.py b/tools/patman/main.py index 03668d1bb8c..2432d31871c 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -18,10 +18,9 @@ if __name__ == "__main__": sys.path.append(os.path.join(our_path, '..')) # Our modules -from patman import checkpatch from patman import command +from patman import control from patman import gitutil -from patman import patchstream from patman import project from patman import settings from patman import terminal @@ -128,56 +127,4 @@ elif options.full_help: # Process commits, produce patches files, check them, email them else: - gitutil.Setup() - - if options.count == -1: - # Work out how many patches to send if we can - options.count = gitutil.CountCommitsToBranch() - options.start - - col = terminal.Color() - if not options.count: - str = 'No commits found to process - please use -c flag' - sys.exit(col.Color(col.RED, str)) - - # Read the metadata from the commits - if options.count: - series = patchstream.GetMetaData(options.start, options.count) - cover_fname, args = gitutil.CreatePatches(options.start, options.count, - options.ignore_binary, series) - - # Fix up the patch files to our liking, and insert the cover letter - patchstream.FixPatches(series, args) - if cover_fname and series.get('cover'): - patchstream.InsertCoverLetter(cover_fname, series, options.count) - - # Do a few checks on the series - series.DoChecks() - - # Check the patches, and run them through 'git am' just to be sure - if options.check_patch: - ok = checkpatch.CheckPatches(options.verbose, args) - else: - ok = True - - cc_file = series.MakeCcFile(options.process_tags, cover_fname, - not options.ignore_bad_tags, - options.add_maintainers, options.limit) - - # Email the patches out (giving the user time to check / cancel) - cmd = '' - its_a_go = ok or options.ignore_errors - if its_a_go: - cmd = gitutil.EmailPatches(series, cover_fname, args, - options.dry_run, not options.ignore_bad_tags, cc_file, - in_reply_to=options.in_reply_to, thread=options.thread, - smtp_server=options.smtp_server) - else: - print(col.Color(col.RED, "Not sending emails due to errors/warnings")) - - # For a dry run, just show our actions as a sanity check - if options.dry_run: - series.ShowActions(args, cmd, options.process_tags) - if not its_a_go: - print(col.Color(col.RED, "Email would not be sent")) - - os.remove(cc_file) + control.send(options) diff --git a/tools/patman/series.py b/tools/patman/series.py index b7eef37d036..9f885c89873 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -244,7 +244,7 @@ class Series(dict): add_maintainers: Either: True/False to call the get_maintainers to CC maintainers List of maintainers to include (for testing) - limit: Limit the length of the Cc list + limit: Limit the length of the Cc list (None if no limit) Return: Filename of temp file created """ -- cgit v1.2.3 From fd70986a62afc291d3fbc172dc0c7219dc84d4b7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:50 -0600 Subject: patman: Add a test that uses gitpython It is convenient to use gitpython to create a real git repo for testing patman's operation. Add a test for this. So far it just checks that patman produces the right number of patches for a branch. Signed-off-by: Simon Glass --- tools/patman/func_test.py | 151 +++++++++++++++++++++++++++++++++++++++++++++- tools/patman/tools.py | 4 +- 2 files changed, 152 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index dc30078ccee..211952154aa 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -14,15 +14,23 @@ import unittest from io import StringIO +from patman import control from patman import gitutil from patman import patchstream from patman import settings +from patman import terminal from patman import tools +from patman.test_util import capture_sys_output + +try: + import pygit2 + HAVE_PYGIT2= True +except ModuleNotFoundError: + HAVE_PYGIT2 = False @contextlib.contextmanager def capture(): - import sys oldout,olderr = sys.stdout, sys.stderr try: out=[StringIO(), StringIO()] @@ -37,6 +45,8 @@ def capture(): class TestFunctional(unittest.TestCase): def setUp(self): self.tmpdir = tempfile.mkdtemp(prefix='patman.') + self.gitdir = os.path.join(self.tmpdir, 'git') + self.repo = None def tearDown(self): shutil.rmtree(self.tmpdir) @@ -286,3 +296,142 @@ Changes in v2: if expected: expected = expected.splitlines() self.assertEqual(expected, lines[start:(start+len(expected))]) + + def make_commit_with_file(self, subject, body, fname, text): + """Create a file and add it to the git repo with a new commit + + Args: + subject (str): Subject for the commit + body (str): Body text of the commit + fname (str): Filename of file to create + text (str): Text to put into the file + """ + path = os.path.join(self.gitdir, fname) + tools.WriteFile(path, text, binary=False) + index = self.repo.index + index.add(fname) + author = pygit2.Signature('Test user', 'test@email.com') + committer = author + tree = index.write_tree() + message = subject + '\n' + body + self.repo.create_commit('HEAD', author, committer, message, tree, + [self.repo.head.target]) + + def make_git_tree(self): + """Make a simple git tree suitable for testing + + It has three branches: + 'base' has two commits: PCI, main + 'first' has base as upstream and two more commits: I2C, SPI + 'second' has base as upstream and three more: video, serial, bootm + + Returns: + pygit2 repository + """ + repo = pygit2.init_repository(self.gitdir) + self.repo = repo + new_tree = repo.TreeBuilder().write() + + author = pygit2.Signature('Test user', 'test@email.com') + committer = author + commit = repo.create_commit('HEAD', author, committer, + 'Created master', new_tree, []) + + self.make_commit_with_file('Initial commit', ''' +Add a README + +''', 'README', '''This is the README file +describing this project +in very little detail''') + + self.make_commit_with_file('pci: PCI implementation', ''' +Here is a basic PCI implementation + +''', 'pci.c', '''This is a file +it has some contents +and some more things''') + self.make_commit_with_file('main: Main program', ''' +Hello here is the second commit. +''', 'main.c', '''This is the main file +there is very little here +but we can always add more later +if we want to + +Series-to: u-boot +Series-cc: Barry Crump +''') + base_target = repo.revparse_single('HEAD') + self.make_commit_with_file('i2c: I2C things', ''' +This has some stuff to do with I2C +''', 'i2c.c', '''And this is the file contents +with some I2C-related things in it''') + self.make_commit_with_file('spi: SPI fixes', ''' +SPI needs some fixes +and here they are +''', 'spi.c', '''Some fixes for SPI in this +file to make SPI work +better than before''') + first_target = repo.revparse_single('HEAD') + + target = repo.revparse_single('HEAD~2') + repo.reset(target.oid, pygit2.GIT_CHECKOUT_FORCE) + self.make_commit_with_file('video: Some video improvements', ''' +Fix up the video so that +it looks more purple. Purple is +a very nice colour. +''', 'video.c', '''More purple here +Purple and purple +Even more purple +Could not be any more purple''') + self.make_commit_with_file('serial: Add a serial driver', ''' +Here is the serial driver +for my chip. + +Cover-letter: +Series for my board +This series implements support +for my glorious board. +END +''', 'serial.c', '''The code for the +serial driver is here''') + self.make_commit_with_file('bootm: Make it boot', ''' +This makes my board boot +with a fix to the bootm +command +''', 'bootm.c', '''Fix up the bootm +command to make the code as +complicated as possible''') + second_target = repo.revparse_single('HEAD') + + repo.branches.local.create('first', first_target) + repo.config.set_multivar('branch.first.remote', '', '.') + repo.config.set_multivar('branch.first.merge', '', 'refs/heads/base') + + repo.branches.local.create('second', second_target) + repo.config.set_multivar('branch.second.remote', '', '.') + repo.config.set_multivar('branch.second.merge', '', 'refs/heads/base') + + repo.branches.local.create('base', base_target) + return repo + + @unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2') + def testBranch(self): + """Test creating patches from a branch""" + repo = self.make_git_tree() + target = repo.lookup_reference('refs/heads/first') + self.repo.checkout(target, strategy=pygit2.GIT_CHECKOUT_FORCE) + control.setup() + try: + orig_dir = os.getcwd() + os.chdir(self.gitdir) + + # Check that it can detect the current branch + self.assertEqual(2, gitutil.CountCommitsToBranch()) + col = terminal.Color() + with capture_sys_output() as _: + _, cover_fname, patch_files = control.prepare_patches( + col, count=-1, start=0, ignore_binary=False) + self.assertIsNone(cover_fname) + self.assertEqual(2, len(patch_files)) + finally: + os.chdir(orig_dir) diff --git a/tools/patman/tools.py b/tools/patman/tools.py index b50370dfe8d..f402b9aab8b 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -270,7 +270,7 @@ def ReadFile(fname, binary=True): #(fname, len(data), len(data))) return data -def WriteFile(fname, data): +def WriteFile(fname, data, binary=True): """Write data into a file. Args: @@ -279,7 +279,7 @@ def WriteFile(fname, data): """ #self._out.Info("Write file '%s' size %d (%#0x)" % #(fname, len(data), len(data))) - with open(Filename(fname), 'wb') as fd: + with open(Filename(fname), binary and 'wb' or 'w') as fd: fd.write(data) def GetBytes(byte, size): -- cgit v1.2.3 From 262130f57c398d7a548cf55fdc278efe561c7afb Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:51 -0600 Subject: patman: Allow creating patches for another branch Add a -b option to allow patches to be created from a branch other than the current one. Signed-off-by: Simon Glass --- tools/patman/control.py | 13 ++++++++----- tools/patman/func_test.py | 13 +++++++++++-- tools/patman/gitutil.py | 19 ++++++++++++++----- tools/patman/main.py | 2 ++ tools/patman/patchstream.py | 8 +++++--- 5 files changed, 40 insertions(+), 15 deletions(-) (limited to 'tools') diff --git a/tools/patman/control.py b/tools/patman/control.py index a896c924b5f..b48eac41fd7 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -20,7 +20,7 @@ def setup(): """Do required setup before doing anything""" gitutil.Setup() -def prepare_patches(col, count, start, ignore_binary): +def prepare_patches(col, branch, count, start, ignore_binary): """Figure out what patches to generate, then generate them The patch files are written to the current directory, e.g. 0001_xxx.patch @@ -28,6 +28,7 @@ def prepare_patches(col, count, start, ignore_binary): Args: col (terminal.Color): Colour output object + branch (str): Branch to create patches from (None = current) count (int): Number of patches to produce, or -1 to produce patches for the current branch back to the upstream commit start (int): Start partch to use (0=first / top of branch) @@ -42,7 +43,7 @@ def prepare_patches(col, count, start, ignore_binary): """ if count == -1: # Work out how many patches to send if we can - count = (gitutil.CountCommitsToBranch() - start) + count = (gitutil.CountCommitsToBranch(branch) - start) if not count: sys.exit(col.Color(col.RED, @@ -50,9 +51,9 @@ def prepare_patches(col, count, start, ignore_binary): # Read the metadata from the commits to_do = count - series = patchstream.GetMetaData(start, to_do) + series = patchstream.GetMetaData(branch, start, to_do) cover_fname, patch_files = gitutil.CreatePatches( - start, to_do, ignore_binary, series) + branch, start, to_do, ignore_binary, series) # Fix up the patch files to our liking, and insert the cover letter patchstream.FixPatches(series, patch_files) @@ -158,9 +159,11 @@ def send(options): setup() col = terminal.Color() series, cover_fname, patch_files = prepare_patches( - col, options.count, options.start, options.ignore_binary) + col, options.branch, options.count, options.start, + options.ignore_binary) ok = check_patches(series, patch_files, options.check_patch, options.verbose) + its_a_go = ok or options.ignore_errors if its_a_go: email_patches( diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 211952154aa..588be73ef48 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -426,12 +426,21 @@ complicated as possible''') os.chdir(self.gitdir) # Check that it can detect the current branch - self.assertEqual(2, gitutil.CountCommitsToBranch()) + self.assertEqual(2, gitutil.CountCommitsToBranch(None)) col = terminal.Color() with capture_sys_output() as _: _, cover_fname, patch_files = control.prepare_patches( - col, count=-1, start=0, ignore_binary=False) + col, branch=None, count=-1, start=0, ignore_binary=False) self.assertIsNone(cover_fname) self.assertEqual(2, len(patch_files)) + + # Check that it can detect a different branch + self.assertEqual(3, gitutil.CountCommitsToBranch('second')) + with capture_sys_output() as _: + _, cover_fname, patch_files = control.prepare_patches( + col, branch='second', count=-1, start=0, + ignore_binary=False) + self.assertIsNotNone(cover_fname) + self.assertEqual(3, len(patch_files)) finally: os.chdir(orig_dir) diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 29444bf8e92..b683481a574 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -49,17 +49,24 @@ def LogCmd(commit_range, git_dir=None, oneline=False, reverse=False, cmd.append('--') return cmd -def CountCommitsToBranch(): +def CountCommitsToBranch(branch): """Returns number of commits between HEAD and the tracking branch. This looks back to the tracking branch and works out the number of commits since then. + Args: + branch: Branch to count from (None for current branch) + Return: Number of patches that exist on top of the branch """ - pipe = [LogCmd('@{upstream}..', oneline=True), - ['wc', '-l']] + if branch: + us, msg = GetUpstream('.git', branch) + rev_range = '%s..%s' % (us, branch) + else: + rev_range = '@{upstream}..' + pipe = [LogCmd(rev_range, oneline=True), ['wc', '-l']] stdout = command.RunPipe(pipe, capture=True, oneline=True).stdout patch_count = int(stdout) return patch_count @@ -252,13 +259,14 @@ def Fetch(git_dir=None, work_tree=None): if result.return_code != 0: raise OSError('git fetch: %s' % result.stderr) -def CreatePatches(start, count, ignore_binary, series): +def CreatePatches(branch, start, count, ignore_binary, series): """Create a series of patches from the top of the current branch. The patch files are written to the current directory using git format-patch. Args: + branch: Branch to create patches from (None for current branch) start: Commit to start from: 0=HEAD, 1=next one, etc. count: number of commits to include ignore_binary: Don't generate patches for binary files @@ -277,7 +285,8 @@ def CreatePatches(start, count, ignore_binary, series): prefix = series.GetPatchPrefix() if prefix: cmd += ['--subject-prefix=%s' % prefix] - cmd += ['HEAD~%d..HEAD~%d' % (start + count, start)] + brname = branch or 'HEAD' + cmd += ['%s~%d..%s~%d' % (brname, start + count, brname, start)] stdout = command.RunList(cmd) files = stdout.splitlines() diff --git a/tools/patman/main.py b/tools/patman/main.py index 2432d31871c..066754196e5 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -31,6 +31,8 @@ from patman import test_checkpatch parser = OptionParser() parser.add_option('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') +parser.add_option('-b', '--branch', type='str', + help="Branch to process (by default, the current branch)") parser.add_option('-c', '--count', dest='count', type='int', default=-1, help='Automatically create patches from top n commits') parser.add_option('-i', '--ignore-errors', action='store_true', diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 4fe465e9ab8..2ea8ebcc3fd 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -512,17 +512,19 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None, ps.Finalize() return series -def GetMetaData(start, count): +def GetMetaData(branch, start, count): """Reads out patch series metadata from the commits This does a 'git log' on the relevant commits and pulls out the tags we are interested in. Args: - start: Commit to start from: 0=HEAD, 1=next one, etc. + branch: Branch to use (None for current branch) + start: Commit to start from: 0=branch HEAD, 1=next one, etc. count: Number of commits to list """ - return GetMetaDataForList('HEAD~%d' % start, None, count) + return GetMetaDataForList('%s~%d' % (branch if branch else 'HEAD', start), + None, count) def GetMetaDataForTest(text): """Process metadata from a file containing a git log. Used for tests -- cgit v1.2.3 From 137947e05b25a9e511bbc30bd795c9d95deec0cb Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:52 -0600 Subject: patman: Allow skipping patches at the end The -s option allows skipping patches at the top of the branch. Sometimes there are commits at the bottom that need to be skipped. At present it is necessary to count the number of commits and then use -c to tell patman how many to process. Add a -e option to easily skip a number of commits at the bottom of the branch. Signed-off-by: Simon Glass --- tools/patman/control.py | 8 +++++--- tools/patman/func_test.py | 13 +++++++++++-- tools/patman/main.py | 2 ++ 3 files changed, 18 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/patman/control.py b/tools/patman/control.py index b48eac41fd7..b481ff6b270 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -20,7 +20,7 @@ def setup(): """Do required setup before doing anything""" gitutil.Setup() -def prepare_patches(col, branch, count, start, ignore_binary): +def prepare_patches(col, branch, count, start, end, ignore_binary): """Figure out what patches to generate, then generate them The patch files are written to the current directory, e.g. 0001_xxx.patch @@ -32,6 +32,8 @@ def prepare_patches(col, branch, count, start, ignore_binary): count (int): Number of patches to produce, or -1 to produce patches for the current branch back to the upstream commit start (int): Start partch to use (0=first / top of branch) + end (int): End patch to use (0=last one in series, 1=one before that, + etc.) ignore_binary (bool): Don't generate patches for binary files Returns: @@ -50,7 +52,7 @@ def prepare_patches(col, branch, count, start, ignore_binary): 'No commits found to process - please use -c flag')) # Read the metadata from the commits - to_do = count + to_do = count - end series = patchstream.GetMetaData(branch, start, to_do) cover_fname, patch_files = gitutil.CreatePatches( branch, start, to_do, ignore_binary, series) @@ -159,7 +161,7 @@ def send(options): setup() col = terminal.Color() series, cover_fname, patch_files = prepare_patches( - col, options.branch, options.count, options.start, + col, options.branch, options.count, options.start, options.end, options.ignore_binary) ok = check_patches(series, patch_files, options.check_patch, options.verbose) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 588be73ef48..810af9c6042 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -430,7 +430,8 @@ complicated as possible''') col = terminal.Color() with capture_sys_output() as _: _, cover_fname, patch_files = control.prepare_patches( - col, branch=None, count=-1, start=0, ignore_binary=False) + col, branch=None, count=-1, start=0, end=0, + ignore_binary=False) self.assertIsNone(cover_fname) self.assertEqual(2, len(patch_files)) @@ -438,9 +439,17 @@ complicated as possible''') self.assertEqual(3, gitutil.CountCommitsToBranch('second')) with capture_sys_output() as _: _, cover_fname, patch_files = control.prepare_patches( - col, branch='second', count=-1, start=0, + col, branch='second', count=-1, start=0, end=0, ignore_binary=False) self.assertIsNotNone(cover_fname) self.assertEqual(3, len(patch_files)) + + # Check that it can skip patches at the end + with capture_sys_output() as _: + _, cover_fname, patch_files = control.prepare_patches( + col, branch='second', count=-1, start=0, end=1, + ignore_binary=False) + self.assertIsNotNone(cover_fname) + self.assertEqual(2, len(patch_files)) finally: os.chdir(orig_dir) diff --git a/tools/patman/main.py b/tools/patman/main.py index 066754196e5..4d7a3044eae 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -35,6 +35,8 @@ parser.add_option('-b', '--branch', type='str', help="Branch to process (by default, the current branch)") parser.add_option('-c', '--count', dest='count', type='int', default=-1, help='Automatically create patches from top n commits') +parser.add_option('-e', '--end', type='int', default=0, + help='Commits to skip at end of patch list') parser.add_option('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') -- cgit v1.2.3 From fda1e372d35dc956317923f5057c3ac83be7f571 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:53 -0600 Subject: patman: Convert to ArgumentParser Convert from OptionParser to ArgumentParser to match binman. With this we can easily add sub-commands. Signed-off-by: Simon Glass --- tools/patman/control.py | 22 +++++------ tools/patman/main.py | 97 ++++++++++++++++++++++++------------------------ tools/patman/settings.py | 10 +++-- 3 files changed, 65 insertions(+), 64 deletions(-) (limited to 'tools') diff --git a/tools/patman/control.py b/tools/patman/control.py index b481ff6b270..e67867b845c 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -152,24 +152,24 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, os.remove(cc_file) -def send(options): +def send(args): """Create, check and send patches by email Args: - options (optparse.Values): Arguments to patman + args (argparse.Namespace): Arguments to patman """ setup() col = terminal.Color() series, cover_fname, patch_files = prepare_patches( - col, options.branch, options.count, options.start, options.end, - options.ignore_binary) - ok = check_patches(series, patch_files, options.check_patch, - options.verbose) + col, args.branch, args.count, args.start, args.end, + args.ignore_binary) + ok = check_patches(series, patch_files, args.check_patch, + args.verbose) - its_a_go = ok or options.ignore_errors + its_a_go = ok or args.ignore_errors if its_a_go: email_patches( - col, series, cover_fname, patch_files, options.process_tags, - its_a_go, options.ignore_bad_tags, options.add_maintainers, - options.limit, options.dry_run, options.in_reply_to, options.thread, - options.smtp_server) + col, series, cover_fname, patch_files, args.process_tags, + its_a_go, args.ignore_bad_tags, args.add_maintainers, + args.limit, args.dry_run, args.in_reply_to, args.thread, + args.smtp_server) diff --git a/tools/patman/main.py b/tools/patman/main.py index 4d7a3044eae..34cad9a562c 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -6,7 +6,7 @@ """See README for more information""" -from optparse import OptionParser +from argparse import ArgumentParser import os import re import sys @@ -27,71 +27,70 @@ from patman import terminal from patman import test_util from patman import test_checkpatch +epilog = '''Create patches from commits in a branch, check them and email them +as specified by tags you place in the commits. Use -n to do a dry run first.''' -parser = OptionParser() -parser.add_option('-H', '--full-help', action='store_true', dest='full_help', +parser = ArgumentParser(epilog=epilog) +parser.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') -parser.add_option('-b', '--branch', type='str', +parser.add_argument('-b', '--branch', type=str, help="Branch to process (by default, the current branch)") -parser.add_option('-c', '--count', dest='count', type='int', +parser.add_argument('-c', '--count', dest='count', type=int, default=-1, help='Automatically create patches from top n commits') -parser.add_option('-e', '--end', type='int', default=0, +parser.add_argument('-e', '--end', type=int, default=0, help='Commits to skip at end of patch list') -parser.add_option('-i', '--ignore-errors', action='store_true', +parser.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') -parser.add_option('-l', '--limit-cc', dest='limit', type='int', - default=None, help='Limit the cc list to LIMIT entries [default: %default]') -parser.add_option('-m', '--no-maintainers', action='store_false', +parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None, + help='Limit the cc list to LIMIT entries [default: %(default)]') +parser.add_argument('-m', '--no-maintainers', action='store_false', dest='add_maintainers', default=True, help="Don't cc the file maintainers automatically") -parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run', +parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run', default=False, help="Do a dry run (create but don't email patches)") -parser.add_option('-p', '--project', default=project.DetectProject(), - help="Project name; affects default option values and " - "aliases [default: %default]") -parser.add_option('-r', '--in-reply-to', type='string', action='store', +parser.add_argument('-p', '--project', default=project.DetectProject(), + help="Project name; affects default option values and " + "aliases [default: %(default)]") +parser.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") -parser.add_option('-s', '--start', dest='start', type='int', +parser.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') -parser.add_option('-t', '--ignore-bad-tags', action='store_true', - default=False, help='Ignore bad tags / aliases') -parser.add_option('-v', '--verbose', action='store_true', dest='verbose', +parser.add_argument('-t', '--ignore-bad-tags', action='store_true', + default=False, help='Ignore bad tags / aliases') +parser.add_argument('-v', '--verbose', action='store_true', dest='verbose', default=False, help='Verbose output of errors and warnings') -parser.add_option('-T', '--thread', action='store_true', dest='thread', - default=False, help='Create patches as a single thread') -parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store', +parser.add_argument('-T', '--thread', action='store_true', dest='thread', + default=False, help='Create patches as a single thread') +parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', default=None, help='Output cc list for patch file (used by git)') -parser.add_option('--no-binary', action='store_true', dest='ignore_binary', - default=False, - help="Do not output contents of changes in binary files") -parser.add_option('--no-check', action='store_false', dest='check_patch', - default=True, - help="Don't check for patch compliance") -parser.add_option('--no-tags', action='store_false', dest='process_tags', - default=True, help="Don't process subject tags as aliases") -parser.add_option('--smtp-server', type='str', - help="Specify the SMTP server to 'git send-email'") -parser.add_option('--test', action='store_true', dest='test', - default=False, help='run tests') - -parser.usage += """ - -Create patches from commits in a branch, check them and email them as -specified by tags you place in the commits. Use -n to do a dry run first.""" - +parser.add_argument('--no-binary', action='store_true', dest='ignore_binary', + default=False, + help="Do not output contents of changes in binary files") +parser.add_argument('--no-check', action='store_false', dest='check_patch', + default=True, + help="Don't check for patch compliance") +parser.add_argument('--no-tags', action='store_false', dest='process_tags', + default=True, help="Don't process subject tags as aliases") +parser.add_argument('--smtp-server', type=str, + help="Specify the SMTP server to 'git send-email'") +parser.add_argument('--test', action='store_true', dest='test', + default=False, help='run tests') + +parser.add_argument('patchfiles', nargs='*') # Parse options twice: first to get the project and second to handle # defaults properly (which depends on project). -(options, args) = parser.parse_args() -settings.Setup(gitutil, parser, options.project, '') -(options, args) = parser.parse_args() +argv = sys.argv[1:] +args = parser.parse_args(argv) +settings.Setup(gitutil, parser, args.project, '') +args = parser.parse_args(argv) if __name__ != "__main__": pass # Run our meagre tests -elif options.test: +elif args.test: import doctest from patman import func_test @@ -109,19 +108,19 @@ elif options.test: # Called from git with a patch filename as argument # Printout a list of additional CC recipients for this patch -elif options.cc_cmd: - fd = open(options.cc_cmd, 'r') +elif args.cc_cmd: + fd = open(args.cc_cmd, 'r') re_line = re.compile('(\S*) (.*)') for line in fd.readlines(): match = re_line.match(line) - if match and match.group(1) == args[0]: + if match and match.group(1) == args.patchfiles[0]: for cc in match.group(2).split('\0'): cc = cc.strip() if cc: print(cc) fd.close() -elif options.full_help: +elif args.full_help: pager = os.getenv('PAGER') if not pager: pager = 'more' @@ -131,4 +130,4 @@ elif options.full_help: # Process commits, produce patches files, check them, email them else: - control.send(options) + control.send(args) diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 635561ac056..732bd401067 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -233,17 +233,19 @@ def _UpdateDefaults(parser, config): config: An instance of _ProjectConfigParser that we will query for settings. """ - defaults = parser.get_default_values() + defaults = parser.parse_known_args()[0] + defaults = vars(defaults) for name, val in config.items('settings'): - if hasattr(defaults, name): - default_val = getattr(defaults, name) + if name in defaults: + default_val = defaults[name] if isinstance(default_val, bool): val = config.getboolean('settings', name) elif isinstance(default_val, int): val = config.getint('settings', name) - parser.set_default(name, val) + defaults[name] = val else: print("WARNING: Unknown setting %s" % name) + parser.set_defaults(**defaults) def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists. -- cgit v1.2.3 From c4e79029e2aba29b769c9ddc6967a09cd7f5d61d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:54 -0600 Subject: patman: Allow different commands At present patman only does one thing so does not have any comments. We want to add a few more command, so create a sub-parser for the default command ('send'). Signed-off-by: Simon Glass --- tools/patman/main.py | 77 ++++++++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 36 deletions(-) (limited to 'tools') diff --git a/tools/patman/main.py b/tools/patman/main.py index 34cad9a562c..fee9bc848bd 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -31,60 +31,65 @@ epilog = '''Create patches from commits in a branch, check them and email them as specified by tags you place in the commits. Use -n to do a dry run first.''' parser = ArgumentParser(epilog=epilog) -parser.add_argument('-H', '--full-help', action='store_true', dest='full_help', +subparsers = parser.add_subparsers(dest='cmd') +send = subparsers.add_parser('send') +send.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') -parser.add_argument('-b', '--branch', type=str, +send.add_argument('-b', '--branch', type=str, help="Branch to process (by default, the current branch)") -parser.add_argument('-c', '--count', dest='count', type=int, +send.add_argument('-c', '--count', dest='count', type=int, default=-1, help='Automatically create patches from top n commits') -parser.add_argument('-e', '--end', type=int, default=0, +send.add_argument('-e', '--end', type=int, default=0, help='Commits to skip at end of patch list') -parser.add_argument('-i', '--ignore-errors', action='store_true', +send.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') -parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None, - help='Limit the cc list to LIMIT entries [default: %(default)]') -parser.add_argument('-m', '--no-maintainers', action='store_false', +send.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None, + help='Limit the cc list to LIMIT entries [default: %(default)s]') +send.add_argument('-m', '--no-maintainers', action='store_false', dest='add_maintainers', default=True, help="Don't cc the file maintainers automatically") -parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run', +send.add_argument('-n', '--dry-run', action='store_true', dest='dry_run', default=False, help="Do a dry run (create but don't email patches)") -parser.add_argument('-p', '--project', default=project.DetectProject(), - help="Project name; affects default option values and " - "aliases [default: %(default)]") -parser.add_argument('-r', '--in-reply-to', type=str, action='store', +send.add_argument('-p', '--project', default=project.DetectProject(), + help="Project name; affects default option values and " + "aliases [default: %(default)s]") +send.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") -parser.add_argument('-s', '--start', dest='start', type=int, +send.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') -parser.add_argument('-t', '--ignore-bad-tags', action='store_true', - default=False, help='Ignore bad tags / aliases') -parser.add_argument('-v', '--verbose', action='store_true', dest='verbose', +send.add_argument('-t', '--ignore-bad-tags', action='store_true', + default=False, help='Ignore bad tags / aliases') +send.add_argument('-v', '--verbose', action='store_true', dest='verbose', default=False, help='Verbose output of errors and warnings') -parser.add_argument('-T', '--thread', action='store_true', dest='thread', - default=False, help='Create patches as a single thread') -parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', +send.add_argument('-T', '--thread', action='store_true', dest='thread', + default=False, help='Create patches as a single thread') +send.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', default=None, help='Output cc list for patch file (used by git)') -parser.add_argument('--no-binary', action='store_true', dest='ignore_binary', - default=False, - help="Do not output contents of changes in binary files") -parser.add_argument('--no-check', action='store_false', dest='check_patch', - default=True, - help="Don't check for patch compliance") -parser.add_argument('--no-tags', action='store_false', dest='process_tags', - default=True, help="Don't process subject tags as aliases") -parser.add_argument('--smtp-server', type=str, - help="Specify the SMTP server to 'git send-email'") -parser.add_argument('--test', action='store_true', dest='test', - default=False, help='run tests') - -parser.add_argument('patchfiles', nargs='*') +send.add_argument('--no-binary', action='store_true', dest='ignore_binary', + default=False, + help="Do not output contents of changes in binary files") +send.add_argument('--no-check', action='store_false', dest='check_patch', + default=True, + help="Don't check for patch compliance") +send.add_argument('--no-tags', action='store_false', dest='process_tags', + default=True, help="Don't process subject tags as aliases") +send.add_argument('--smtp-server', type=str, + help="Specify the SMTP server to 'git send-email'") +send.add_argument('--test', action='store_true', dest='test', + default=False, help='run tests') + +send.add_argument('patchfiles', nargs='*') # Parse options twice: first to get the project and second to handle # defaults properly (which depends on project). argv = sys.argv[1:] +if len(argv) < 1 or argv[0].startswith('-'): + argv = ['send'] + argv args = parser.parse_args(argv) -settings.Setup(gitutil, parser, args.project, '') -args = parser.parse_args(argv) +if hasattr(args, 'project'): + settings.Setup(gitutil, send, args.project, '') + args = parser.parse_args(argv) if __name__ != "__main__": pass -- cgit v1.2.3 From 6bb74de7ed070a5ea44cc111939ed5bb07df5ef5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:55 -0600 Subject: patman: Add a 'test' subcommand At present we use --test to indicate that tests should be run. It is better to use a subcommand for list, like binman. Change it and adjust the existing code to fit under a 'send' subcommand, the default. Give this subcommand the same default arguments as the others. Signed-off-by: Simon Glass --- tools/patman/main.py | 75 +++++++++++++++++++++++++---------------------- tools/patman/test_util.py | 2 +- 2 files changed, 41 insertions(+), 36 deletions(-) (limited to 'tools') diff --git a/tools/patman/main.py b/tools/patman/main.py index fee9bc848bd..77f187e7693 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -27,6 +27,16 @@ from patman import terminal from patman import test_util from patman import test_checkpatch +def AddCommonArgs(parser): + parser.add_argument('-b', '--branch', type=str, + help="Branch to process (by default, the current branch)") + parser.add_argument('-c', '--count', dest='count', type=int, + default=-1, help='Automatically create patches from top n commits') + parser.add_argument('-e', '--end', type=int, default=0, + help='Commits to skip at end of patch list') + parser.add_argument('-s', '--start', dest='start', type=int, + default=0, help='Commit to start creating patches from (0 = HEAD)') + epilog = '''Create patches from commits in a branch, check them and email them as specified by tags you place in the commits. Use -n to do a dry run first.''' @@ -35,12 +45,6 @@ subparsers = parser.add_subparsers(dest='cmd') send = subparsers.add_parser('send') send.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') -send.add_argument('-b', '--branch', type=str, - help="Branch to process (by default, the current branch)") -send.add_argument('-c', '--count', dest='count', type=int, - default=-1, help='Automatically create patches from top n commits') -send.add_argument('-e', '--end', type=int, default=0, - help='Commits to skip at end of patch list') send.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') @@ -56,8 +60,6 @@ send.add_argument('-p', '--project', default=project.DetectProject(), "aliases [default: %(default)s]") send.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") -send.add_argument('-s', '--start', dest='start', type=int, - default=0, help='Commit to start creating patches from (0 = HEAD)') send.add_argument('-t', '--ignore-bad-tags', action='store_true', default=False, help='Ignore bad tags / aliases') send.add_argument('-v', '--verbose', action='store_true', dest='verbose', @@ -76,11 +78,13 @@ send.add_argument('--no-tags', action='store_false', dest='process_tags', default=True, help="Don't process subject tags as aliases") send.add_argument('--smtp-server', type=str, help="Specify the SMTP server to 'git send-email'") -send.add_argument('--test', action='store_true', dest='test', - default=False, help='run tests') +AddCommonArgs(send) send.add_argument('patchfiles', nargs='*') +test_parser = subparsers.add_parser('test', help='Run tests') +AddCommonArgs(test_parser) + # Parse options twice: first to get the project and second to handle # defaults properly (which depends on project). argv = sys.argv[1:] @@ -95,7 +99,7 @@ if __name__ != "__main__": pass # Run our meagre tests -elif args.test: +if args.cmd == 'test': import doctest from patman import func_test @@ -111,28 +115,29 @@ elif args.test: sys.exit(test_util.ReportResult('patman', None, result)) -# Called from git with a patch filename as argument -# Printout a list of additional CC recipients for this patch -elif args.cc_cmd: - fd = open(args.cc_cmd, 'r') - re_line = re.compile('(\S*) (.*)') - for line in fd.readlines(): - match = re_line.match(line) - if match and match.group(1) == args.patchfiles[0]: - for cc in match.group(2).split('\0'): - cc = cc.strip() - if cc: - print(cc) - fd.close() - -elif args.full_help: - pager = os.getenv('PAGER') - if not pager: - pager = 'more' - fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), - 'README') - command.Run(pager, fname) - # Process commits, produce patches files, check them, email them -else: - control.send(args) +elif args.cmd == 'send': + # Called from git with a patch filename as argument + # Printout a list of additional CC recipients for this patch + if args.cc_cmd: + fd = open(args.cc_cmd, 'r') + re_line = re.compile('(\S*) (.*)') + for line in fd.readlines(): + match = re_line.match(line) + if match and match.group(1) == args.patchfiles[0]: + for cc in match.group(2).split('\0'): + cc = cc.strip() + if cc: + print(cc) + fd.close() + + elif args.full_help: + pager = os.getenv('PAGER') + if not pager: + pager = 'more' + fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), + 'README') + command.Run(pager, fname) + + else: + control.send(args) diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index 0827488f33d..a87d3cc8f37 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -47,7 +47,7 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): glob_list = [] glob_list += exclude_list glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*'] - test_cmd = 'test' if 'binman' in prog else '-t' + test_cmd = 'test' if 'binman' in prog or 'patman' in prog else '-t' prefix = '' if build_dir: prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir -- cgit v1.2.3 From 3c541c08366533f1abbf7e36dd25033064d686c8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:56 -0600 Subject: patman: Allow disabling 'bright' mode with Print output At present all text is marked bright, which makes it stand out on the terminal. Add a way to disable that, as is done with the Color class. Signed-off-by: Simon Glass --- tools/patman/terminal.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py index c709438bdc4..60dbce3ce1f 100644 --- a/tools/patman/terminal.py +++ b/tools/patman/terminal.py @@ -122,7 +122,7 @@ def TrimAsciiLen(text, size): return out -def Print(text='', newline=True, colour=None, limit_to_line=False): +def Print(text='', newline=True, colour=None, limit_to_line=False, bright=True): """Handle a line of output to the terminal. In test mode this is recorded in a list. Otherwise it is output to the @@ -140,7 +140,7 @@ def Print(text='', newline=True, colour=None, limit_to_line=False): else: if colour: col = Color() - text = col.Color(colour, text) + text = col.Color(colour, text, bright=bright) if newline: print(text) last_print_len = None -- cgit v1.2.3 From 7207e2b984c1bdd58658a9be384fc627770e86c2 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:57 -0600 Subject: patman: Support collecting response tags in Patchstream Collect response tags such as 'Reviewed-by' while parsing the stream. This allows us to see what tags are present. Add a new 'Fixes' tag also, since this is now quite common. Signed-off-by: Simon Glass --- tools/patman/commit.py | 14 ++++++++++++++ tools/patman/patchstream.py | 21 ++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/patman/commit.py b/tools/patman/commit.py index 48d0529c536..8d583c4ed39 100644 --- a/tools/patman/commit.py +++ b/tools/patman/commit.py @@ -2,6 +2,7 @@ # Copyright (c) 2011 The Chromium OS Authors. # +import collections import re # Separates a tag: at the beginning of the subject from the rest of it @@ -23,6 +24,9 @@ class Commit: notes: List of lines in the commit (not series) notes change_id: the Change-Id: tag that was stripped from this commit and can be used to generate the Message-Id. + rtags: Response tags (e.g. Reviewed-by) collected by the commit, dict: + key: rtag type (e.g. 'Reviewed-by') + value: Set of people who gave that rtag, each a name/email string """ def __init__(self, hash): self.hash = hash @@ -33,6 +37,7 @@ class Commit: self.signoff_set = set() self.notes = [] self.change_id = None + self.rtags = collections.defaultdict(set) def AddChange(self, version, info): """Add a new change line to the change list for a version. @@ -88,3 +93,12 @@ class Commit: return False self.signoff_set.add(signoff) return True + + def AddRtag(self, rtag_type, who): + """Add a response tag to a commit + + Args: + key: rtag type (e.g. 'Reviewed-by') + who: Person who gave that rtag, e.g. 'Fred Bloggs ' + """ + self.rtags[rtag_type].add(who) diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 2ea8ebcc3fd..0c68c861569 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -37,7 +37,7 @@ re_change_id = re.compile('^Change-Id: *(.*)') re_commit_tag = re.compile('^Commit-([a-z-]*): *(.*)') # Commit tags that we want to collect and keep -re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc): (.*)') +re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc|Fixes): (.*)') # The start of a new commit in the git log re_commit = re.compile('^commit ([0-9a-f]*)$') @@ -112,6 +112,15 @@ class PatchStream: self.in_section = 'commit-' + name self.skip_blank = False + def AddCommitRtag(self, rtag_type, who): + """Add a response tag to the current commit + + Args: + key: rtag type (e.g. 'Reviewed-by') + who: Person who gave that rtag, e.g. 'Fred Bloggs ' + """ + self.commit.AddRtag(rtag_type, who) + def CloseCommit(self): """Save the current commit into our commit list, and reset our state""" if self.commit and self.is_log: @@ -346,12 +355,14 @@ class PatchStream: # Detect tags in the commit message elif tag_match: + rtag_type, who = tag_match.groups() + self.AddCommitRtag(rtag_type, who) # Remove Tested-by self, since few will take much notice - if (tag_match.group(1) == 'Tested-by' and - tag_match.group(2).find(os.getenv('USER') + '@') != -1): + if (rtag_type == 'Tested-by' and + who.find(os.getenv('USER') + '@') != -1): self.warn.append("Ignoring %s" % line) - elif tag_match.group(1) == 'Patch-cc': - self.commit.AddCc(tag_match.group(2).split(',')) + elif rtag_type == 'Patch-cc': + self.commit.AddCc(who.split(',')) else: out = [line] -- cgit v1.2.3 From c9360f1626cfe1d6af8d06e5e8fd6cec14c5a839 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 5 Jul 2020 21:41:59 -0600 Subject: patman: Add a -D option to enable debugging Most users don't want to see traceback errors. Add an option to enable them for debugging. Disable them by default. Signed-off-by: Simon Glass --- tools/patman/main.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tools') diff --git a/tools/patman/main.py b/tools/patman/main.py index 77f187e7693..b96000807eb 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -10,6 +10,7 @@ from argparse import ArgumentParser import os import re import sys +import traceback import unittest if __name__ == "__main__": @@ -34,6 +35,8 @@ def AddCommonArgs(parser): default=-1, help='Automatically create patches from top n commits') parser.add_argument('-e', '--end', type=int, default=0, help='Commits to skip at end of patch list') + parser.add_argument('-D', '--debug', action='store_true', + help='Enabling debugging (provides a full traceback on error)') parser.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') @@ -98,6 +101,9 @@ if hasattr(args, 'project'): if __name__ != "__main__": pass +if not args.debug: + sys.tracebacklimit = 0 + # Run our meagre tests if args.cmd == 'test': import doctest -- cgit v1.2.3 From 6c8e0bfe6e379408bcc80224f74ba47e5f82505e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:26 -0600 Subject: binman: Output errors to stderr At present binman outputs errors to stdout which means that fails are effectively silent when printed by buildman, for example. Fix this by outputing errors to stderr. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/binman/main.py b/tools/binman/main.py index efa7fa8386f..0ab2bb62065 100755 --- a/tools/binman/main.py +++ b/tools/binman/main.py @@ -123,7 +123,7 @@ def RunBinman(args): try: ret_code = control.Binman(args) except Exception as e: - print('binman: %s' % e) + print('binman: %s' % e, file=sys.stderr) if args.debug: print() traceback.print_exc() -- cgit v1.2.3 From 5de9b9c03c65e834361bf50e525fc5af889e0ff4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:27 -0600 Subject: binman: cbfs: Fix IFWI typo This comment references the wrong thing. Fix it. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/etype/cbfs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py index e9aed8310c7..744a32fa0c2 100644 --- a/tools/binman/etype/cbfs.py +++ b/tools/binman/etype/cbfs.py @@ -204,7 +204,7 @@ class Entry_cbfs(Entry): return True def _ReadSubnodes(self): - """Read the subnodes to find out what should go in this IFWI""" + """Read the subnodes to find out what should go in this CBFS""" for node in self._node.subnodes: entry = Entry.Create(self, node) entry.ReadNode() -- cgit v1.2.3 From fdb3040e964608c4d6caac9e8b18cb4b0a2a45b5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:28 -0600 Subject: binman: Correct the search patch for pylibfdt Now that binman uses tools/ as its base directory for importing modules, the path to the pylibfdt build by U-Boot is incorrect. Fix it with a new path. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/main.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tools') diff --git a/tools/binman/main.py b/tools/binman/main.py index 0ab2bb62065..6f5a9d1ca22 100755 --- a/tools/binman/main.py +++ b/tools/binman/main.py @@ -26,6 +26,7 @@ from patman import test_util # Bring in the libfdt module sys.path.insert(2, 'scripts/dtc/pylibfdt') +sys.path.insert(2, os.path.join(our_path, '../../scripts/dtc/pylibfdt')) sys.path.insert(2, os.path.join(our_path, '../../build-sandbox_spl/scripts/dtc/pylibfdt')) -- cgit v1.2.3 From 32eb66d2d4f3f761d98946414bcf7ca08600d422 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:29 -0600 Subject: binman: Specify the toolpath when running test coverage At present binman's test coverage runs without a toolpath set. This means that the system tools will be used. That may not be correct if they are out of date or missing and this can result in a reduction in test coverage below 100%. Provide the toolpath to binman in this case. Signed-off-by: Simon Glass --- tools/binman/main.py | 10 +++++++--- tools/patman/test_util.py | 9 ++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) (limited to 'tools') diff --git a/tools/binman/main.py b/tools/binman/main.py index 6f5a9d1ca22..a5793d5d231 100755 --- a/tools/binman/main.py +++ b/tools/binman/main.py @@ -89,14 +89,18 @@ def GetEntryModules(include_testing=True): for item in glob_list if include_testing or '_testing' not in item]) -def RunTestCoverage(): +def RunTestCoverage(toolpath): """Run the tests and check that we get 100% coverage""" glob_list = GetEntryModules(False) all_set = set([os.path.splitext(os.path.basename(item))[0] for item in glob_list if '_testing' not in item]) + extra_args = '' + if toolpath: + for path in toolpath: + extra_args += ' --toolpath %s' % path test_util.RunTestCoverage('tools/binman/binman', None, ['*test*', '*main.py', 'tools/patman/*', 'tools/dtoc/*'], - args.build_dir, all_set) + args.build_dir, all_set, extra_args or None) def RunBinman(args): """Main entry point to binman once arguments are parsed @@ -111,7 +115,7 @@ def RunBinman(args): if args.cmd == 'test': if args.test_coverage: - RunTestCoverage() + RunTestCoverage(args.toolpath) else: ret_code = RunTests(args.debug, args.verbosity, args.processes, args.test_preserve_dirs, args.tests, diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index a87d3cc8f37..20dc1e49244 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -21,7 +21,8 @@ except: use_concurrent = False -def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): +def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None, + extra_args=None): """Run tests and check that we get 100% coverage Args: @@ -34,6 +35,8 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): calculation build_dir: Build directory, used to locate libfdt.py required: List of modules which must be in the coverage report + extra_args (str): Extra arguments to pass to the tool before the -t/test + arg Raises: ValueError if the code coverage is not 100% @@ -52,8 +55,8 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): if build_dir: prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir cmd = ('%spython3-coverage run ' - '--omit "%s" %s %s -P1' % (prefix, ','.join(glob_list), - prog, test_cmd)) + '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list), + prog, extra_args or '', test_cmd)) os.system(cmd) stdout = command.Output('python3-coverage', 'report') lines = stdout.splitlines() -- cgit v1.2.3 From b5287c41266fcbd5d31271e1faef3159debf02e1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:30 -0600 Subject: binman: Set a default toolpath When binman is run from 'make check' it is given a toolpath so that the latest tools (e.g. mkimage) are used. When run manually with no toolpath, it relies on the system mkimage. But this may be missing or old. Make some effort to find the built-from-soruce version by looking in the current directory and in the builds created by 'make check'. Signed-off-by: Simon Glass --- tools/binman/main.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'tools') diff --git a/tools/binman/main.py b/tools/binman/main.py index a5793d5d231..e543a7d06a7 100755 --- a/tools/binman/main.py +++ b/tools/binman/main.py @@ -113,6 +113,11 @@ def RunBinman(args): if not args.debug: sys.tracebacklimit = 0 + # Provide a default toolpath in the hope of finding a mkimage built from + # current source + if not args.toolpath: + args.toolpath = ['./tools', 'build-sandbox/tools'] + if args.cmd == 'test': if args.test_coverage: RunTestCoverage(args.toolpath) -- cgit v1.2.3 From 0dc706fe544dc1121c15008ec50809cd6d72ff53 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:31 -0600 Subject: binman: Add support for calling mkimage As a first step to integrating mkimage into binman, add a new entry type that feeds data into mkimage for processing and incorporates that output into the image. Signed-off-by: Simon Glass --- tools/binman/README.entries | 23 +++++++++++++++ tools/binman/etype/_testing.py | 5 ++++ tools/binman/etype/mkimage.py | 62 +++++++++++++++++++++++++++++++++++++++ tools/binman/ftest.py | 7 +++++ tools/binman/test/156_mkimage.dts | 23 +++++++++++++++ 5 files changed, 120 insertions(+) create mode 100644 tools/binman/etype/mkimage.py create mode 100644 tools/binman/test/156_mkimage.dts (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 6a816bba6bc..4f2c48fdc26 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -587,6 +587,29 @@ See README.x86 for information about Intel binary blobs. +Entry: mkimage: Entry containing a binary produced by mkimage +------------------------------------------------------------- + +Properties / Entry arguments: + - datafile: Filename for -d argument + - args: Other arguments to pass + +The data passed to mkimage is collected from subnodes of the mkimage node, +e.g.: + + mkimage { + args = "-n test -T imximage"; + + u-boot-spl { + }; + }; + +This calls mkimage to create an imximage with u-boot-spl.bin as the input +file. The output from mkimage then becomes part of the image produced by +binman. + + + Entry: powerpc-mpc85xx-bootpg-resetvec: PowerPC mpc85xx bootpg + resetvec code for U-Boot ----------------------------------------------------------------------------------------- diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index ed718eed145..ea60561adba 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -57,6 +57,8 @@ class Entry__testing(Entry): 'return-contents-once') self.bad_update_contents_twice = fdt_util.GetBool(self._node, 'bad-update-contents-twice') + self.return_contents_later = fdt_util.GetBool(self._node, + 'return-contents-later') # Set to True when the entry is ready to process the FDT. self.process_fdt_ready = False @@ -83,6 +85,9 @@ class Entry__testing(Entry): def ObtainContents(self): if self.return_unknown_contents or not self.return_contents: return False + if self.return_contents_later: + self.return_contents_later = False + return False self.data = self.contents self.contents_size = len(self.data) if self.return_contents_once: diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py new file mode 100644 index 00000000000..1aa563963a6 --- /dev/null +++ b/tools/binman/etype/mkimage.py @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# Entry-type module for producing an image using mkimage +# + +from collections import OrderedDict + +from binman.entry import Entry +from dtoc import fdt_util +from patman import tools + +class Entry_mkimage(Entry): + """Entry containing a binary produced by mkimage + + Properties / Entry arguments: + - datafile: Filename for -d argument + - args: Other arguments to pass + + The data passed to mkimage is collected from subnodes of the mkimage node, + e.g.: + + mkimage { + args = "-n test -T imximage"; + + u-boot-spl { + }; + }; + + This calls mkimage to create an imximage with u-boot-spl.bin as the input + file. The output from mkimage then becomes part of the image produced by + binman. + """ + def __init__(self, section, etype, node): + Entry.__init__(self, section, etype, node) + self._args = fdt_util.GetString(self._node, 'args').split(' ') + self._mkimage_entries = OrderedDict() + self._ReadSubnodes() + + def ObtainContents(self): + data = b'' + for entry in self._mkimage_entries.values(): + # First get the input data and put it in a file. If not available, + # try later. + if not entry.ObtainContents(): + return False + data += entry.GetData() + uniq = self.GetUniqueName() + input_fname = tools.GetOutputFilename('mkimage.%s' % uniq) + tools.WriteFile(input_fname, data) + output_fname = tools.GetOutputFilename('mkimage-out.%s' % uniq) + tools.Run('mkimage', '-d', input_fname, *self._args, output_fname) + self.SetContents(tools.ReadFile(output_fname)) + return True + + def _ReadSubnodes(self): + """Read the subnodes to find out what should go in this image""" + for node in self._node.subnodes: + entry = Entry.Create(self, node) + entry.ReadNode() + self._mkimage_entries[entry.name] = entry diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5e24920088c..39e67b9042b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3357,6 +3357,13 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('154_intel_fsp_t.dts') self.assertEqual(FSP_T_DATA, data[:len(FSP_T_DATA)]) + def testMkimage(self): + """Test using mkimage to build an image""" + data = self._DoReadFile('156_mkimage.dts') + + # Just check that the data appears in the file somewhere + self.assertIn(U_BOOT_SPL_DATA, data) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/156_mkimage.dts b/tools/binman/test/156_mkimage.dts new file mode 100644 index 00000000000..933b13143a8 --- /dev/null +++ b/tools/binman/test/156_mkimage.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x80>; + + mkimage { + args = "-n test -T script"; + + u-boot-spl { + }; + + _testing { + return-contents-later; + }; + }; + }; +}; -- cgit v1.2.3 From c6162cf87c8896094213023266737667813a86d8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:32 -0600 Subject: binman: Fix a few typos in the entry docs Some typos have been fixed in the generated entry docs but the code was not updated. Fix this. Signed-off-by: Simon Glass --- tools/binman/etype/intel_me.py | 2 +- tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/binman/etype/intel_me.py b/tools/binman/etype/intel_me.py index 41c9c6b9203..2707ca6912b 100644 --- a/tools/binman/etype/intel_me.py +++ b/tools/binman/etype/intel_me.py @@ -16,7 +16,7 @@ class Entry_intel_me(Entry_blob): This file contains code used by the SoC that is required to make it work. The Management Engine is like a background task that runs things that are - not clearly documented, but may include keyboard, deplay and network + not clearly documented, but may include keyboard, display and network access. For platform that use ME it is not possible to disable it. U-Boot does not directly execute code in the ME binary. diff --git a/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py b/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py index cefd425a5dc..28005c60b3a 100644 --- a/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py +++ b/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py @@ -13,7 +13,7 @@ class Entry_powerpc_mpc85xx_bootpg_resetvec(Entry_blob): Properties / Entry arguments: - filename: Filename of u-boot-br.bin (default 'u-boot-br.bin') - This enrty is valid for PowerPC mpc85xx cpus. This entry holds + This entry is valid for PowerPC mpc85xx cpus. This entry holds 'bootpg + resetvec' code for PowerPC mpc85xx CPUs which needs to be placed at offset 'RESET_VECTOR_ADDRESS - 0xffc'. """ -- cgit v1.2.3 From 34861d506cafc8ff758dbba9ffa7340a6e3ec4f1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:35 -0600 Subject: binman: Use super() instead of specifying parent type It is easier and less error-prone to use super() when the parent type is needed. Update binman to remove the type names. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/etype/_testing.py | 4 ++-- tools/binman/etype/blob.py | 2 +- tools/binman/etype/blob_dtb.py | 6 +++--- tools/binman/etype/blob_named_by_arg.py | 2 +- tools/binman/etype/cbfs.py | 14 +++++++------- tools/binman/etype/cros_ec_rw.py | 3 +-- tools/binman/etype/fdtmap.py | 2 +- tools/binman/etype/files.py | 2 +- tools/binman/etype/fill.py | 4 ++-- tools/binman/etype/fmap.py | 2 +- tools/binman/etype/gbb.py | 2 +- tools/binman/etype/image_header.py | 4 ++-- tools/binman/etype/intel_cmc.py | 2 +- tools/binman/etype/intel_descriptor.py | 4 ++-- tools/binman/etype/intel_fit.py | 4 ++-- tools/binman/etype/intel_fit_ptr.py | 4 ++-- tools/binman/etype/intel_fsp.py | 2 +- tools/binman/etype/intel_fsp_m.py | 2 +- tools/binman/etype/intel_fsp_s.py | 2 +- tools/binman/etype/intel_fsp_t.py | 2 +- tools/binman/etype/intel_ifwi.py | 4 ++-- tools/binman/etype/intel_me.py | 2 +- tools/binman/etype/intel_mrc.py | 2 +- tools/binman/etype/intel_refcode.py | 2 +- tools/binman/etype/intel_vbt.py | 2 +- tools/binman/etype/intel_vga.py | 2 +- tools/binman/etype/mkimage.py | 2 +- tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py | 2 +- tools/binman/etype/section.py | 16 ++++++++-------- tools/binman/etype/text.py | 2 +- tools/binman/etype/u_boot.py | 2 +- tools/binman/etype/u_boot_dtb.py | 2 +- tools/binman/etype/u_boot_dtb_with_ucode.py | 4 ++-- tools/binman/etype/u_boot_elf.py | 4 ++-- tools/binman/etype/u_boot_img.py | 2 +- tools/binman/etype/u_boot_nodtb.py | 2 +- tools/binman/etype/u_boot_spl.py | 2 +- tools/binman/etype/u_boot_spl_bss_pad.py | 2 +- tools/binman/etype/u_boot_spl_dtb.py | 2 +- tools/binman/etype/u_boot_spl_elf.py | 2 +- tools/binman/etype/u_boot_spl_nodtb.py | 2 +- tools/binman/etype/u_boot_spl_with_ucode_ptr.py | 2 +- tools/binman/etype/u_boot_tpl.py | 2 +- tools/binman/etype/u_boot_tpl_dtb.py | 2 +- tools/binman/etype/u_boot_tpl_dtb_with_ucode.py | 2 +- tools/binman/etype/u_boot_tpl_elf.py | 2 +- tools/binman/etype/u_boot_tpl_with_ucode_ptr.py | 2 +- tools/binman/etype/u_boot_ucode.py | 2 +- tools/binman/etype/u_boot_with_ucode_ptr.py | 2 +- tools/binman/etype/vblock.py | 2 +- tools/binman/etype/x86_reset16.py | 2 +- tools/binman/etype/x86_reset16_spl.py | 2 +- tools/binman/etype/x86_reset16_tpl.py | 2 +- tools/binman/etype/x86_start16.py | 2 +- tools/binman/etype/x86_start16_spl.py | 2 +- tools/binman/etype/x86_start16_tpl.py | 2 +- tools/binman/image.py | 12 ++++++------ 57 files changed, 86 insertions(+), 87 deletions(-) (limited to 'tools') diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index ea60561adba..0800c25899a 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -41,10 +41,10 @@ class Entry__testing(Entry): data type (generating an error) """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) def ReadNode(self): - Entry.ReadNode(self) + super().ReadNode() self.return_invalid_entry = fdt_util.GetBool(self._node, 'return-invalid-entry') self.return_unknown_contents = fdt_util.GetBool(self._node, diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index ede7a7a68cf..e507203709f 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -31,7 +31,7 @@ class Entry_blob(Entry): data. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._filename = fdt_util.GetString(self._node, 'filename', self.etype) self.compress = fdt_util.GetString(self._node, 'compress', 'none') diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 6c069437633..724647a7bbb 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -20,13 +20,13 @@ class Entry_blob_dtb(Entry_blob): global state from binman import state - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def ObtainContents(self): """Get the device-tree from the list held by the 'state' module""" self._filename = self.GetDefaultFilename() self._pathname, _ = state.GetFdtContents(self.GetFdtEtype()) - return Entry_blob.ReadBlobContents(self) + return super().ReadBlobContents() def ProcessContents(self): """Re-read the DTB contents so that we get any calculated properties""" @@ -57,7 +57,7 @@ class Entry_blob_dtb(Entry_blob): return {self.GetFdtEtype(): [self, fname]} def WriteData(self, data, decomp=True): - ok = Entry_blob.WriteData(self, data, decomp) + ok = super().WriteData(data, decomp) # Update the state module, since it has the authoritative record of the # device trees used. If we don't do this, then state.GetFdtContents() diff --git a/tools/binman/etype/blob_named_by_arg.py b/tools/binman/etype/blob_named_by_arg.py index 3b4593f071a..e95dabe4d07 100644 --- a/tools/binman/etype/blob_named_by_arg.py +++ b/tools/binman/etype/blob_named_by_arg.py @@ -29,6 +29,6 @@ class Entry_blob_named_by_arg(Entry_blob): See cros_ec_rw for an example of this. """ def __init__(self, section, etype, node, blob_fname): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._filename, = self.GetEntryArgsOrProps( [EntryArg('%s-path' % blob_fname, str)]) diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py index 744a32fa0c2..650ab2c292f 100644 --- a/tools/binman/etype/cbfs.py +++ b/tools/binman/etype/cbfs.py @@ -167,7 +167,7 @@ class Entry_cbfs(Entry): global state from binman import state - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._cbfs_arg = fdt_util.GetString(node, 'cbfs-arch', 'x86') self._cbfs_entries = OrderedDict() self._ReadSubnodes() @@ -226,7 +226,7 @@ class Entry_cbfs(Entry): Args: image_pos: Position of this entry in the image """ - Entry.SetImagePos(self, image_pos) + super().SetImagePos(image_pos) # Now update the entries with info from the CBFS entries for entry in self._cbfs_entries.values(): @@ -238,7 +238,7 @@ class Entry_cbfs(Entry): entry.uncomp_size = cfile.memlen def AddMissingProperties(self): - Entry.AddMissingProperties(self) + super().AddMissingProperties() for entry in self._cbfs_entries.values(): entry.AddMissingProperties() if entry._cbfs_compress: @@ -250,7 +250,7 @@ class Entry_cbfs(Entry): def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman""" - Entry.SetCalculatedProperties(self) + super().SetCalculatedProperties() for entry in self._cbfs_entries.values(): state.SetInt(entry._node, 'offset', entry.offset) state.SetInt(entry._node, 'size', entry.size) @@ -260,7 +260,7 @@ class Entry_cbfs(Entry): def ListEntries(self, entries, indent): """Override this method to list all files in the section""" - Entry.ListEntries(self, entries, indent) + super().ListEntries(entries, indent) for entry in self._cbfs_entries.values(): entry.ListEntries(entries, indent + 1) @@ -268,12 +268,12 @@ class Entry_cbfs(Entry): return self._cbfs_entries def ReadData(self, decomp=True): - data = Entry.ReadData(self, True) + data = super().ReadData(True) return data def ReadChildData(self, child, decomp=True): if not self.reader: - data = Entry.ReadData(self, True) + data = super().ReadData(True) self.reader = cbfs_util.CbfsReader(data) reader = self.reader cfile = reader.files.get(child.name) diff --git a/tools/binman/etype/cros_ec_rw.py b/tools/binman/etype/cros_ec_rw.py index 0dbe14b342a..7ad62d02655 100644 --- a/tools/binman/etype/cros_ec_rw.py +++ b/tools/binman/etype/cros_ec_rw.py @@ -18,5 +18,4 @@ class Entry_cros_ec_rw(Entry_blob_named_by_arg): updating the EC on startup via software sync. """ def __init__(self, section, etype, node): - Entry_blob_named_by_arg.__init__(self, section, etype, node, - 'cros-ec-rw') + super().__init__(section, etype, node, 'cros-ec-rw') diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py index aa8807990b7..6ca88a100e5 100644 --- a/tools/binman/etype/fdtmap.py +++ b/tools/binman/etype/fdtmap.py @@ -85,7 +85,7 @@ class Entry_fdtmap(Entry): from binman import state from dtoc.fdt import Fdt - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) def _GetFdtmap(self): """Build an FDT map from the entries in the current image diff --git a/tools/binman/etype/files.py b/tools/binman/etype/files.py index 10ab585f0ed..9adb3afeb14 100644 --- a/tools/binman/etype/files.py +++ b/tools/binman/etype/files.py @@ -32,7 +32,7 @@ class Entry_files(Entry_section): global state from binman import state - Entry_section.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._pattern = fdt_util.GetString(self._node, 'pattern') if not self._pattern: self.Raise("Missing 'pattern' property") diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py index 860410ed6e5..efb2d13e910 100644 --- a/tools/binman/etype/fill.py +++ b/tools/binman/etype/fill.py @@ -22,10 +22,10 @@ class Entry_fill(Entry): byte value of a region. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) def ReadNode(self): - Entry.ReadNode(self) + super().ReadNode() if self.size is None: self.Raise("'fill' entry must have a size property") self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0) diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py index a43fac38de2..3e9b815d119 100644 --- a/tools/binman/etype/fmap.py +++ b/tools/binman/etype/fmap.py @@ -32,7 +32,7 @@ class Entry_fmap(Entry): the sub-entries are ignored. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) def _GetFmap(self): """Build an FMAP from the entries in the current image diff --git a/tools/binman/etype/gbb.py b/tools/binman/etype/gbb.py index dd105997179..41554eba8f6 100644 --- a/tools/binman/etype/gbb.py +++ b/tools/binman/etype/gbb.py @@ -54,7 +54,7 @@ class Entry_gbb(Entry): README.chromium for how to obtain the required keys and tools. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.hardware_id, self.keydir, self.bmpblk = self.GetEntryArgsOrProps( [EntryArg('hardware-id', str), EntryArg('keydir', str), diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py index 176bdeb29b3..24011884958 100644 --- a/tools/binman/etype/image_header.py +++ b/tools/binman/etype/image_header.py @@ -57,7 +57,7 @@ class Entry_image_header(Entry): first/last in the entry list. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.location = fdt_util.GetString(self._node, 'location') def _GetHeader(self): @@ -101,7 +101,7 @@ class Entry_image_header(Entry): else: offset = image_size - IMAGE_HEADER_LEN offset += self.section.GetStartOffset() - return Entry.Pack(self, offset) + return super().Pack(offset) def ProcessContents(self): """Write an updated version of the FDT map to this entry diff --git a/tools/binman/etype/intel_cmc.py b/tools/binman/etype/intel_cmc.py index 5e6edbe4dfd..9ab471e7b6f 100644 --- a/tools/binman/etype/intel_cmc.py +++ b/tools/binman/etype/intel_cmc.py @@ -20,4 +20,4 @@ class Entry_intel_cmc(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_descriptor.py b/tools/binman/etype/intel_descriptor.py index d4d7a26901d..6afc42ece54 100644 --- a/tools/binman/etype/intel_descriptor.py +++ b/tools/binman/etype/intel_descriptor.py @@ -45,14 +45,14 @@ class Entry_intel_descriptor(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._regions = [] def Pack(self, offset): """Put this entry at the start of the image""" if self.offset is None: offset = self.section.GetStartOffset() - return Entry_blob.Pack(self, offset) + return super().Pack(offset) def GetOffsets(self): offset = self.data.find(FD_SIGNATURE) diff --git a/tools/binman/etype/intel_fit.py b/tools/binman/etype/intel_fit.py index ea482a61254..ad6c1caa856 100644 --- a/tools/binman/etype/intel_fit.py +++ b/tools/binman/etype/intel_fit.py @@ -19,11 +19,11 @@ class Entry_intel_fit(Entry_blob): At present binman only supports a basic FIT with no microcode. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def ReadNode(self): """Force 16-byte alignment as required by FIT pointer""" - Entry_blob.ReadNode(self) + super().ReadNode() self.align = 16 def ObtainContents(self): diff --git a/tools/binman/etype/intel_fit_ptr.py b/tools/binman/etype/intel_fit_ptr.py index df118a68f2d..a06d12e740b 100644 --- a/tools/binman/etype/intel_fit_ptr.py +++ b/tools/binman/etype/intel_fit_ptr.py @@ -16,7 +16,7 @@ class Entry_intel_fit_ptr(Entry_blob): 0xffffffc0 in the image. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) if self.HasSibling('intel-fit') is False: self.Raise("'intel-fit-ptr' section must have an 'intel-fit' sibling") @@ -38,4 +38,4 @@ class Entry_intel_fit_ptr(Entry_blob): def Pack(self, offset): """Special pack method to set the offset to the right place""" - return Entry_blob.Pack(self, 0xffffffc0) + return super().Pack(0xffffffc0) diff --git a/tools/binman/etype/intel_fsp.py b/tools/binman/etype/intel_fsp.py index 7db3d96b432..a1c89adcea6 100644 --- a/tools/binman/etype/intel_fsp.py +++ b/tools/binman/etype/intel_fsp.py @@ -24,4 +24,4 @@ class Entry_intel_fsp(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_fsp_m.py b/tools/binman/etype/intel_fsp_m.py index 51b4e7e1ac3..4c225b24d36 100644 --- a/tools/binman/etype/intel_fsp_m.py +++ b/tools/binman/etype/intel_fsp_m.py @@ -24,4 +24,4 @@ class Entry_intel_fsp_m(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_fsp_s.py b/tools/binman/etype/intel_fsp_s.py index b3683e476a0..9e1107182a3 100644 --- a/tools/binman/etype/intel_fsp_s.py +++ b/tools/binman/etype/intel_fsp_s.py @@ -24,4 +24,4 @@ class Entry_intel_fsp_s(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_fsp_t.py b/tools/binman/etype/intel_fsp_t.py index 0f196f0f1c1..5dca145a3f4 100644 --- a/tools/binman/etype/intel_fsp_t.py +++ b/tools/binman/etype/intel_fsp_t.py @@ -23,4 +23,4 @@ class Entry_intel_fsp_t(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index 6a96f6be552..ba63f6574f9 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -45,13 +45,13 @@ class Entry_intel_ifwi(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._convert_fit = fdt_util.GetBool(self._node, 'convert-fit') self._ifwi_entries = OrderedDict() def ReadNode(self): self._ReadSubnodes() - Entry_blob.ReadNode(self) + super().ReadNode() def _BuildIfwi(self): """Build the contents of the IFWI and write it to the 'data' property""" diff --git a/tools/binman/etype/intel_me.py b/tools/binman/etype/intel_me.py index 2707ca6912b..6b3803819ec 100644 --- a/tools/binman/etype/intel_me.py +++ b/tools/binman/etype/intel_me.py @@ -27,4 +27,4 @@ class Entry_intel_me(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_mrc.py b/tools/binman/etype/intel_mrc.py index 854a4dda615..74781848e27 100644 --- a/tools/binman/etype/intel_mrc.py +++ b/tools/binman/etype/intel_mrc.py @@ -21,7 +21,7 @@ class Entry_intel_mrc(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'mrc.bin' diff --git a/tools/binman/etype/intel_refcode.py b/tools/binman/etype/intel_refcode.py index a1059f787e6..5754fec4f8f 100644 --- a/tools/binman/etype/intel_refcode.py +++ b/tools/binman/etype/intel_refcode.py @@ -21,7 +21,7 @@ class Entry_intel_refcode(Entry_blob): See README.x86 for information about x86 binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'refcode.bin' diff --git a/tools/binman/etype/intel_vbt.py b/tools/binman/etype/intel_vbt.py index 4d465ad0173..f6d7b466ea2 100644 --- a/tools/binman/etype/intel_vbt.py +++ b/tools/binman/etype/intel_vbt.py @@ -19,4 +19,4 @@ class Entry_intel_vbt(Entry_blob): See README.x86 for information about Intel binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/intel_vga.py b/tools/binman/etype/intel_vga.py index 04cd72f3dc1..6b87c01b4ce 100644 --- a/tools/binman/etype/intel_vga.py +++ b/tools/binman/etype/intel_vga.py @@ -22,4 +22,4 @@ class Entry_intel_vga(Entry_blob): See README.x86 for information about Intel binary blobs. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 1aa563963a6..8fddc881187 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -33,7 +33,7 @@ class Entry_mkimage(Entry): binman. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._args = fdt_util.GetString(self._node, 'args').split(' ') self._mkimage_entries = OrderedDict() self._ReadSubnodes() diff --git a/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py b/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py index 28005c60b3a..b0fa75fbf88 100644 --- a/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py +++ b/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py @@ -19,7 +19,7 @@ class Entry_powerpc_mpc85xx_bootpg_resetvec(Entry_blob): """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot-br.bin' diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 91b8e0c1100..f108121c3a2 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -43,7 +43,7 @@ class Entry_section(Entry): """ def __init__(self, section, etype, node, test=False): if not test: - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._entries = OrderedDict() self._pad_byte = 0 self._sort = False @@ -52,7 +52,7 @@ class Entry_section(Entry): def ReadNode(self): """Read properties from the image node""" - Entry.ReadNode(self) + super().ReadNode() self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0) self._sort = fdt_util.GetBool(self._node, 'sort-by-offset') self._end_4gb = fdt_util.GetBool(self._node, 'end-at-4gb') @@ -126,13 +126,13 @@ class Entry_section(Entry): a section containing a list of files. Process these entries so that this information is added to the device tree. """ - Entry.ExpandEntries(self) + super().ExpandEntries() for entry in self._entries.values(): entry.ExpandEntries() def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" - Entry.AddMissingProperties(self) + super().AddMissingProperties() for entry in self._entries.values(): entry.AddMissingProperties() @@ -168,14 +168,14 @@ class Entry_section(Entry): def ResetForPack(self): """Reset offset/size fields so that packing can be done again""" - Entry.ResetForPack(self) + super().ResetForPack() for entry in self._entries.values(): entry.ResetForPack() def Pack(self, offset): """Pack all entries into the section""" self._PackEntries() - return Entry.Pack(self, offset) + return super().Pack(offset) def _PackEntries(self): """Pack all entries into the image""" @@ -232,12 +232,12 @@ class Entry_section(Entry): entry.WriteSymbols(self) def SetCalculatedProperties(self): - Entry.SetCalculatedProperties(self) + super().SetCalculatedProperties() for entry in self._entries.values(): entry.SetCalculatedProperties() def SetImagePos(self, image_pos): - Entry.SetImagePos(self, image_pos) + super().SetImagePos(image_pos) for entry in self._entries.values(): entry.SetImagePos(image_pos + self.offset) diff --git a/tools/binman/etype/text.py b/tools/binman/etype/text.py index 3577135adbe..a69c2a4ec48 100644 --- a/tools/binman/etype/text.py +++ b/tools/binman/etype/text.py @@ -57,7 +57,7 @@ class Entry_text(Entry): by setting the size of the entry to something larger than the text. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) value = fdt_util.GetString(self._node, 'text') if value: value = tools.ToBytes(value) diff --git a/tools/binman/etype/u_boot.py b/tools/binman/etype/u_boot.py index ab1019b00c7..4767197e13a 100644 --- a/tools/binman/etype/u_boot.py +++ b/tools/binman/etype/u_boot.py @@ -26,7 +26,7 @@ class Entry_u_boot(Entry_blob): in the binman README for more information. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot.bin' diff --git a/tools/binman/etype/u_boot_dtb.py b/tools/binman/etype/u_boot_dtb.py index e98350088f5..65e71291d27 100644 --- a/tools/binman/etype/u_boot_dtb.py +++ b/tools/binman/etype/u_boot_dtb.py @@ -22,7 +22,7 @@ class Entry_u_boot_dtb(Entry_blob_dtb): binman to know which entries contain a device tree. """ def __init__(self, section, etype, node): - Entry_blob_dtb.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot.dtb' diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index aec145533eb..66a9db55cad 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -28,7 +28,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): global state from binman import state - Entry_blob_dtb.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.ucode_data = b'' self.collate = False self.ucode_offset = None @@ -78,7 +78,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): def ObtainContents(self): # Call the base class just in case it does something important. - Entry_blob_dtb.ObtainContents(self) + super().ObtainContents() if self.ucode and not self.collate: for node in self.ucode.subnodes: data_prop = node.props.get('data') diff --git a/tools/binman/etype/u_boot_elf.py b/tools/binman/etype/u_boot_elf.py index 5f906e520cf..6614a75fafa 100644 --- a/tools/binman/etype/u_boot_elf.py +++ b/tools/binman/etype/u_boot_elf.py @@ -21,7 +21,7 @@ class Entry_u_boot_elf(Entry_blob): relocated to any address for execution. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self._strip = fdt_util.GetBool(self._node, 'strip') def ReadBlobContents(self): @@ -31,7 +31,7 @@ class Entry_u_boot_elf(Entry_blob): tools.WriteFile(out_fname, tools.ReadFile(self._pathname)) tools.Run('strip', out_fname) self._pathname = out_fname - Entry_blob.ReadBlobContents(self) + super().ReadBlobContents() return True def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_img.py b/tools/binman/etype/u_boot_img.py index 50cc71d3ce2..8a739d8edb6 100644 --- a/tools/binman/etype/u_boot_img.py +++ b/tools/binman/etype/u_boot_img.py @@ -21,7 +21,7 @@ class Entry_u_boot_img(Entry_blob): applications. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot.img' diff --git a/tools/binman/etype/u_boot_nodtb.py b/tools/binman/etype/u_boot_nodtb.py index e8c0e1a1d6c..e84df490f65 100644 --- a/tools/binman/etype/u_boot_nodtb.py +++ b/tools/binman/etype/u_boot_nodtb.py @@ -21,7 +21,7 @@ class Entry_u_boot_nodtb(Entry_blob): U-Boot and the device tree). """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot-nodtb.bin' diff --git a/tools/binman/etype/u_boot_spl.py b/tools/binman/etype/u_boot_spl.py index a6fddbe8f14..d66e46140be 100644 --- a/tools/binman/etype/u_boot_spl.py +++ b/tools/binman/etype/u_boot_spl.py @@ -32,7 +32,7 @@ class Entry_u_boot_spl(Entry_blob): binman uses that to look up symbols to write into the SPL binary. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.elf_fname = 'spl/u-boot-spl' def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index a6a177a1287..596b2bed97e 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -31,7 +31,7 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): binman uses that to look up the BSS address. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def ObtainContents(self): fname = tools.GetInputFilename('spl/u-boot-spl') diff --git a/tools/binman/etype/u_boot_spl_dtb.py b/tools/binman/etype/u_boot_spl_dtb.py index a0761eeacd6..eefc4a44aab 100644 --- a/tools/binman/etype/u_boot_spl_dtb.py +++ b/tools/binman/etype/u_boot_spl_dtb.py @@ -19,7 +19,7 @@ class Entry_u_boot_spl_dtb(Entry_blob_dtb): to activate. """ def __init__(self, section, etype, node): - Entry_blob_dtb.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'spl/u-boot-spl.dtb' diff --git a/tools/binman/etype/u_boot_spl_elf.py b/tools/binman/etype/u_boot_spl_elf.py index f99f74abab2..7f1236bcbb3 100644 --- a/tools/binman/etype/u_boot_spl_elf.py +++ b/tools/binman/etype/u_boot_spl_elf.py @@ -18,7 +18,7 @@ class Entry_u_boot_spl_elf(Entry_blob): be relocated to any address for execution. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'spl/u-boot-spl' diff --git a/tools/binman/etype/u_boot_spl_nodtb.py b/tools/binman/etype/u_boot_spl_nodtb.py index 072b915ff3a..6f4529396d8 100644 --- a/tools/binman/etype/u_boot_spl_nodtb.py +++ b/tools/binman/etype/u_boot_spl_nodtb.py @@ -22,7 +22,7 @@ class Entry_u_boot_spl_nodtb(Entry_blob): both SPL and the device tree). """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'spl/u-boot-spl-nodtb.bin' diff --git a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py index b1543a5ef32..72739a5eb67 100644 --- a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py @@ -18,7 +18,7 @@ class Entry_u_boot_spl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): process. """ def __init__(self, section, etype, node): - Entry_u_boot_with_ucode_ptr.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.elf_fname = 'spl/u-boot-spl' def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_tpl.py b/tools/binman/etype/u_boot_tpl.py index 6562457c9a6..02287ab3275 100644 --- a/tools/binman/etype/u_boot_tpl.py +++ b/tools/binman/etype/u_boot_tpl.py @@ -32,7 +32,7 @@ class Entry_u_boot_tpl(Entry_blob): binman uses that to look up symbols to write into the TPL binary. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.elf_fname = 'tpl/u-boot-tpl' def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_tpl_dtb.py b/tools/binman/etype/u_boot_tpl_dtb.py index 890155f271f..2ff1d7ced1a 100644 --- a/tools/binman/etype/u_boot_tpl_dtb.py +++ b/tools/binman/etype/u_boot_tpl_dtb.py @@ -19,7 +19,7 @@ class Entry_u_boot_tpl_dtb(Entry_blob_dtb): to activate. """ def __init__(self, section, etype, node): - Entry_blob_dtb.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'tpl/u-boot-tpl.dtb' diff --git a/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py index ca1bf85ace7..066f18dfef2 100644 --- a/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py @@ -16,7 +16,7 @@ class Entry_u_boot_tpl_dtb_with_ucode(Entry_u_boot_dtb_with_ucode): process. """ def __init__(self, section, etype, node): - Entry_u_boot_dtb_with_ucode.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'tpl/u-boot-tpl.dtb' diff --git a/tools/binman/etype/u_boot_tpl_elf.py b/tools/binman/etype/u_boot_tpl_elf.py index 7fa8e963640..3f24d3aa7bc 100644 --- a/tools/binman/etype/u_boot_tpl_elf.py +++ b/tools/binman/etype/u_boot_tpl_elf.py @@ -18,7 +18,7 @@ class Entry_u_boot_tpl_elf(Entry_blob): be relocated to any address for execution. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'tpl/u-boot-tpl' diff --git a/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py index 7f7fab71051..c7f3f9dedb5 100644 --- a/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py @@ -20,7 +20,7 @@ class Entry_u_boot_tpl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): process. """ def __init__(self, section, etype, node): - Entry_u_boot_with_ucode_ptr.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.elf_fname = 'tpl/u-boot-tpl' def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index d9e1a605efa..44622936182 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -58,7 +58,7 @@ class Entry_u_boot_ucode(Entry_blob): contents of this entry. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def ObtainContents(self): # If the section does not need microcode, there is nothing to do diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 06047b654df..92d2fc68538 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -29,7 +29,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): complicated. Otherwise it is the same as the u_boot entry. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.elf_fname = 'u-boot' self.target_offset = None diff --git a/tools/binman/etype/vblock.py b/tools/binman/etype/vblock.py index 5753de7ec7a..f734fbaec49 100644 --- a/tools/binman/etype/vblock.py +++ b/tools/binman/etype/vblock.py @@ -36,7 +36,7 @@ class Entry_vblock(Entry): and kernel are genuine. """ def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) + super().__init__(section, etype, node) self.content = fdt_util.GetPhandleList(self._node, 'content') if not self.content: self.Raise("Vblock must have a 'content' property") diff --git a/tools/binman/etype/x86_reset16.py b/tools/binman/etype/x86_reset16.py index ad864e54429..5d49f16e21c 100644 --- a/tools/binman/etype/x86_reset16.py +++ b/tools/binman/etype/x86_reset16.py @@ -23,7 +23,7 @@ class Entry_x86_reset16(Entry_blob): For 64-bit U-Boot, the 'x86_reset16_spl' entry type is used instead. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot-x86-reset16.bin' diff --git a/tools/binman/etype/x86_reset16_spl.py b/tools/binman/etype/x86_reset16_spl.py index 9a663f0ae23..775b90699ba 100644 --- a/tools/binman/etype/x86_reset16_spl.py +++ b/tools/binman/etype/x86_reset16_spl.py @@ -23,7 +23,7 @@ class Entry_x86_reset16_spl(Entry_blob): For 32-bit U-Boot, the 'x86_reset_spl' entry type is used instead. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'spl/u-boot-x86-reset16-spl.bin' diff --git a/tools/binman/etype/x86_reset16_tpl.py b/tools/binman/etype/x86_reset16_tpl.py index 864508f3672..52d3f4869ae 100644 --- a/tools/binman/etype/x86_reset16_tpl.py +++ b/tools/binman/etype/x86_reset16_tpl.py @@ -23,7 +23,7 @@ class Entry_x86_reset16_tpl(Entry_blob): For 32-bit U-Boot, the 'x86_reset_tpl' entry type is used instead. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'tpl/u-boot-x86-reset16-tpl.bin' diff --git a/tools/binman/etype/x86_start16.py b/tools/binman/etype/x86_start16.py index d8345f67221..18fdd95d370 100644 --- a/tools/binman/etype/x86_start16.py +++ b/tools/binman/etype/x86_start16.py @@ -25,7 +25,7 @@ class Entry_x86_start16(Entry_blob): For 64-bit U-Boot, the 'x86_start16_spl' entry type is used instead. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'u-boot-x86-start16.bin' diff --git a/tools/binman/etype/x86_start16_spl.py b/tools/binman/etype/x86_start16_spl.py index ad520d3c6d9..ac8e90f2e0c 100644 --- a/tools/binman/etype/x86_start16_spl.py +++ b/tools/binman/etype/x86_start16_spl.py @@ -25,7 +25,7 @@ class Entry_x86_start16_spl(Entry_blob): For 32-bit U-Boot, the 'x86-start16' entry type is used instead. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'spl/u-boot-x86-start16-spl.bin' diff --git a/tools/binman/etype/x86_start16_tpl.py b/tools/binman/etype/x86_start16_tpl.py index ccc8727d1d4..72d4608bb73 100644 --- a/tools/binman/etype/x86_start16_tpl.py +++ b/tools/binman/etype/x86_start16_tpl.py @@ -26,7 +26,7 @@ class Entry_x86_start16_tpl(Entry_blob): may be used instead. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + super().__init__(section, etype, node) def GetDefaultFilename(self): return 'tpl/u-boot-x86-start16-tpl.bin' diff --git a/tools/binman/image.py b/tools/binman/image.py index 523b274c319..a8772c3763b 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -45,7 +45,7 @@ class Image(section.Entry_section): we create a section manually. """ def __init__(self, name, node, copy_to_orig=True, test=False): - section.Entry_section.__init__(self, None, 'section', node, test=test) + super().__init__(None, 'section', node, test=test) self.copy_to_orig = copy_to_orig self.name = 'main-section' self.image_name = name @@ -57,7 +57,7 @@ class Image(section.Entry_section): self.ReadNode() def ReadNode(self): - section.Entry_section.ReadNode(self) + super().ReadNode() filename = fdt_util.GetString(self._node, 'filename') if filename: self._filename = filename @@ -116,11 +116,11 @@ class Image(section.Entry_section): def PackEntries(self): """Pack all entries into the image""" - section.Entry_section.Pack(self, 0) + super().Pack(0) def SetImagePos(self): # This first section in the image so it starts at 0 - section.Entry_section.SetImagePos(self, 0) + super().SetImagePos(0) def ProcessEntryContents(self): """Call the ProcessContents() method for each entry @@ -139,7 +139,7 @@ class Image(section.Entry_section): def WriteSymbols(self): """Write symbol values into binary files for access at run time""" - section.Entry_section.WriteSymbols(self, self) + super().WriteSymbols(self) def BuildImage(self): """Write the image to a file""" @@ -161,7 +161,7 @@ class Image(section.Entry_section): with open(fname, 'w') as fd: print('%8s %8s %8s %s' % ('ImagePos', 'Offset', 'Size', 'Name'), file=fd) - section.Entry_section.WriteMap(self, fd, 0) + super().WriteMap(fd, 0) return fname def BuildEntryList(self): -- cgit v1.2.3 From ce867ad7c8eaae2b14699aee7235695d605c1dda Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:36 -0600 Subject: binman: Add an etype for external binary blobs It is useful to be able to distinguish between ordinary blobs such as u-boot.bin and external blobs that cannot be build by the U-Boot build system. If the external blobs are not available for some reason, then we know that a value image cannot be built. Introduce a new 'blob-ext' entry type for that. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/README.entries | 10 ++++++++++ tools/binman/etype/blob_ext.py | 31 ++++++++++++++++++++++++++++++ tools/binman/ftest.py | 12 ++++++++++++ tools/binman/test/157_blob_ext.dts | 14 ++++++++++++++ tools/binman/test/158_blob_ext_missing.dts | 16 +++++++++++++++ 5 files changed, 83 insertions(+) create mode 100644 tools/binman/etype/blob_ext.py create mode 100644 tools/binman/test/157_blob_ext.dts create mode 100644 tools/binman/test/158_blob_ext_missing.dts (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 4f2c48fdc26..46f6ab18998 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -42,6 +42,16 @@ obtained from the list of available device-tree files, managed by the +Entry: blob-ext: Entry containing an externally built binary blob +----------------------------------------------------------------- + +Note: This should not be used by itself. It is normally used as a parent +class by other entry types. + +See 'blob' for Properties / Entry arguments. + + + Entry: blob-named-by-arg: A blob entry which gets its filename property from its subclass ----------------------------------------------------------------------------------------- diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py new file mode 100644 index 00000000000..cc8d91bb599 --- /dev/null +++ b/tools/binman/etype/blob_ext.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# Entry-type module for external blobs, not built by U-Boot +# + +import os + +from binman.etype.blob import Entry_blob +from dtoc import fdt_util +from patman import tools +from patman import tout + +class Entry_blob_ext(Entry_blob): + """Entry containing an externally built binary blob + + Note: This should not be used by itself. It is normally used as a parent + class by other entry types. + + See 'blob' for Properties / Entry arguments. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + self.external = True + + def ObtainContents(self): + self._filename = self.GetDefaultFilename() + self._pathname = tools.GetInputFilename(self._filename) + self.ReadBlobContents() + return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 39e67b9042b..f8d51916723 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3364,6 +3364,18 @@ class TestFunctional(unittest.TestCase): # Just check that the data appears in the file somewhere self.assertIn(U_BOOT_SPL_DATA, data) + def testExtblob(self): + """Test an image with an external blob""" + data = self._DoReadFile('157_blob_ext.dts') + self.assertEqual(REFCODE_DATA, data) + + def testExtblobMissing(self): + """Test an image with a missing external blob""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('158_blob_ext_missing.dts') + self.assertIn("Filename 'missing-file' not found in input path", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/157_blob_ext.dts b/tools/binman/test/157_blob_ext.dts new file mode 100644 index 00000000000..8afdd5339e5 --- /dev/null +++ b/tools/binman/test/157_blob_ext.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + blob-ext { + filename = "refcode.bin"; + }; + }; +}; diff --git a/tools/binman/test/158_blob_ext_missing.dts b/tools/binman/test/158_blob_ext_missing.dts new file mode 100644 index 00000000000..d315e5592e1 --- /dev/null +++ b/tools/binman/test/158_blob_ext_missing.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x80>; + + blob-ext { + filename = "missing-file"; + }; + }; +}; -- cgit v1.2.3 From 04e6a6b9ecdc75023edeebe73286f311c40b346b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:37 -0600 Subject: binman: Convert existing binary blobs to blob_ext Many of the existing blobs rely on external binaries which may not be available. Move them over to use blob_ext to indicate this. Unfortunately cros-ec-rw cannot use this class because it inherits another. So set the 'external' value for that class. While we are here, drop the import of Entry since it is not used (and pylint3 complains). Signed-off-by: Simon Glass --- tools/binman/etype/cros_ec_rw.py | 1 + tools/binman/etype/intel_cmc.py | 5 ++--- tools/binman/etype/intel_descriptor.py | 4 ++-- tools/binman/etype/intel_fit.py | 4 ++-- tools/binman/etype/intel_fit_ptr.py | 4 ++-- tools/binman/etype/intel_fsp.py | 5 ++--- tools/binman/etype/intel_fsp_m.py | 5 ++--- tools/binman/etype/intel_fsp_s.py | 5 ++--- tools/binman/etype/intel_fsp_t.py | 5 ++--- tools/binman/etype/intel_ifwi.py | 4 ++-- tools/binman/etype/intel_me.py | 5 ++--- tools/binman/etype/intel_mrc.py | 5 ++--- tools/binman/etype/intel_refcode.py | 5 ++--- tools/binman/etype/intel_vbt.py | 5 ++--- tools/binman/etype/intel_vga.py | 5 ++--- tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py | 1 - 16 files changed, 29 insertions(+), 39 deletions(-) (limited to 'tools') diff --git a/tools/binman/etype/cros_ec_rw.py b/tools/binman/etype/cros_ec_rw.py index 7ad62d02655..741372e1af4 100644 --- a/tools/binman/etype/cros_ec_rw.py +++ b/tools/binman/etype/cros_ec_rw.py @@ -19,3 +19,4 @@ class Entry_cros_ec_rw(Entry_blob_named_by_arg): """ def __init__(self, section, etype, node): super().__init__(section, etype, node, 'cros-ec-rw') + self.external = True diff --git a/tools/binman/etype/intel_cmc.py b/tools/binman/etype/intel_cmc.py index 9ab471e7b6f..644fa421d3f 100644 --- a/tools/binman/etype/intel_cmc.py +++ b/tools/binman/etype/intel_cmc.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Chip Microcode binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_cmc(Entry_blob): +class Entry_intel_cmc(Entry_blob_ext): """Entry containing an Intel Chipset Micro Code (CMC) file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_descriptor.py b/tools/binman/etype/intel_descriptor.py index 6afc42ece54..5b18893ccdc 100644 --- a/tools/binman/etype/intel_descriptor.py +++ b/tools/binman/etype/intel_descriptor.py @@ -8,7 +8,7 @@ import struct from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext FD_SIGNATURE = struct.pack('> 4) | 0xfff self.size = self.limit - self.base + 1 -class Entry_intel_descriptor(Entry_blob): +class Entry_intel_descriptor(Entry_blob_ext): """Intel flash descriptor block (4KB) Properties / Entry arguments: diff --git a/tools/binman/etype/intel_fit.py b/tools/binman/etype/intel_fit.py index ad6c1caa856..f1a10c55a67 100644 --- a/tools/binman/etype/intel_fit.py +++ b/tools/binman/etype/intel_fit.py @@ -7,9 +7,9 @@ import struct -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_fit(Entry_blob): +class Entry_intel_fit(Entry_blob_ext): """Intel Firmware Image Table (FIT) This entry contains a dummy FIT as required by recent Intel CPUs. The FIT diff --git a/tools/binman/etype/intel_fit_ptr.py b/tools/binman/etype/intel_fit_ptr.py index a06d12e740b..01f082281c5 100644 --- a/tools/binman/etype/intel_fit_ptr.py +++ b/tools/binman/etype/intel_fit_ptr.py @@ -7,9 +7,9 @@ import struct -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_fit_ptr(Entry_blob): +class Entry_intel_fit_ptr(Entry_blob_ext): """Intel Firmware Image Table (FIT) pointer This entry contains a pointer to the FIT. It is required to be at address diff --git a/tools/binman/etype/intel_fsp.py b/tools/binman/etype/intel_fsp.py index a1c89adcea6..2ac012bce19 100644 --- a/tools/binman/etype/intel_fsp.py +++ b/tools/binman/etype/intel_fsp.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Firmware Support Package binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_fsp(Entry_blob): +class Entry_intel_fsp(Entry_blob_ext): """Entry containing an Intel Firmware Support Package (FSP) file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_fsp_m.py b/tools/binman/etype/intel_fsp_m.py index 4c225b24d36..434b0f1856e 100644 --- a/tools/binman/etype/intel_fsp_m.py +++ b/tools/binman/etype/intel_fsp_m.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Firmware Support Package binary blob (M section) # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_fsp_m(Entry_blob): +class Entry_intel_fsp_m(Entry_blob_ext): """Entry containing Intel Firmware Support Package (FSP) memory init Properties / Entry arguments: diff --git a/tools/binman/etype/intel_fsp_s.py b/tools/binman/etype/intel_fsp_s.py index 9e1107182a3..564e1228bba 100644 --- a/tools/binman/etype/intel_fsp_s.py +++ b/tools/binman/etype/intel_fsp_s.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Firmware Support Package binary blob (S section) # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_fsp_s(Entry_blob): +class Entry_intel_fsp_s(Entry_blob_ext): """Entry containing Intel Firmware Support Package (FSP) silicon init Properties / Entry arguments: diff --git a/tools/binman/etype/intel_fsp_t.py b/tools/binman/etype/intel_fsp_t.py index 5dca145a3f4..df0c5fbee0f 100644 --- a/tools/binman/etype/intel_fsp_t.py +++ b/tools/binman/etype/intel_fsp_t.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Firmware Support Package binary blob (T section) # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_fsp_t(Entry_blob): +class Entry_intel_fsp_t(Entry_blob_ext): """Entry containing Intel Firmware Support Package (FSP) temp ram init Properties / Entry arguments: diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index ba63f6574f9..b0c2b1aaa33 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -8,11 +8,11 @@ from collections import OrderedDict from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext from dtoc import fdt_util from patman import tools -class Entry_intel_ifwi(Entry_blob): +class Entry_intel_ifwi(Entry_blob_ext): """Entry containing an Intel Integrated Firmware Image (IFWI) file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_me.py b/tools/binman/etype/intel_me.py index 6b3803819ec..a6fe5427f31 100644 --- a/tools/binman/etype/intel_me.py +++ b/tools/binman/etype/intel_me.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Management Engine binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_me(Entry_blob): +class Entry_intel_me(Entry_blob_ext): """Entry containing an Intel Management Engine (ME) file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_mrc.py b/tools/binman/etype/intel_mrc.py index 74781848e27..ccbb046519d 100644 --- a/tools/binman/etype/intel_mrc.py +++ b/tools/binman/etype/intel_mrc.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Memory Reference Code binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_mrc(Entry_blob): +class Entry_intel_mrc(Entry_blob_ext): """Entry containing an Intel Memory Reference Code (MRC) file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_refcode.py b/tools/binman/etype/intel_refcode.py index 5754fec4f8f..5ead08b2be4 100644 --- a/tools/binman/etype/intel_refcode.py +++ b/tools/binman/etype/intel_refcode.py @@ -5,10 +5,9 @@ # Entry-type module for Intel Memory Reference Code binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_refcode(Entry_blob): +class Entry_intel_refcode(Entry_blob_ext): """Entry containing an Intel Reference Code file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_vbt.py b/tools/binman/etype/intel_vbt.py index f6d7b466ea2..2a98c123688 100644 --- a/tools/binman/etype/intel_vbt.py +++ b/tools/binman/etype/intel_vbt.py @@ -4,10 +4,9 @@ # Entry-type module for Intel Video BIOS Table binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_vbt(Entry_blob): +class Entry_intel_vbt(Entry_blob_ext): """Entry containing an Intel Video BIOS Table (VBT) file Properties / Entry arguments: diff --git a/tools/binman/etype/intel_vga.py b/tools/binman/etype/intel_vga.py index 6b87c01b4ce..a103f1ce0ee 100644 --- a/tools/binman/etype/intel_vga.py +++ b/tools/binman/etype/intel_vga.py @@ -5,10 +5,9 @@ # Entry-type module for x86 VGA ROM binary blob # -from binman.entry import Entry -from binman.etype.blob import Entry_blob +from binman.etype.blob_ext import Entry_blob_ext -class Entry_intel_vga(Entry_blob): +class Entry_intel_vga(Entry_blob_ext): """Entry containing an Intel Video Graphics Adaptor (VGA) file Properties / Entry arguments: diff --git a/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py b/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py index b0fa75fbf88..3a92fa399fb 100644 --- a/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py +++ b/tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py @@ -4,7 +4,6 @@ # Entry-type module for the PowerPC mpc85xx bootpg and resetvec code for U-Boot # -from binman.entry import Entry from binman.etype.blob import Entry_blob class Entry_powerpc_mpc85xx_bootpg_resetvec(Entry_blob): -- cgit v1.2.3 From 4f9f1056ecf2d9e54803b89fd0784240a8d89fd8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:38 -0600 Subject: binman: Allow external binaries to be missing Sometimes it is useful to build an image even though external binaries are not present. This allows the build system to continue to function without these files, albeit not producing valid images. U-Boot does with with ATF (ARM Trusted Firmware) today. Add a new flag to binman to request this behaviour. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/README.entries | 3 +++ tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 7 +++++-- tools/binman/entry.py | 9 +++++++++ tools/binman/etype/blob_ext.py | 13 ++++++++++--- tools/binman/etype/section.py | 24 ++++++++++++++++++++++++ tools/binman/ftest.py | 8 +++++++- tools/patman/tools.py | 8 ++++++-- 8 files changed, 66 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 46f6ab18998..f45f51428ad 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -48,6 +48,9 @@ Entry: blob-ext: Entry containing an externally built binary blob Note: This should not be used by itself. It is normally used as a parent class by other entry types. +If the file providing this blob is missing, binman can optionally ignore it +and produce a broken image with a warning. + See 'blob' for Properties / Entry arguments. diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 1e385935797..bb4d9d1288b 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -53,6 +53,8 @@ controlled by a description in the board device tree.''' help='Add a path to the list of directories to use for input files') build_parser.add_argument('-m', '--map', action='store_true', default=False, help='Output a map file for each image') + build_parser.add_argument('-M', '--allow-missing', action='store_true', + default=False, help='Allow external blobs to be missing') build_parser.add_argument('-O', '--outdir', type=str, action='store', help='Path to directory to use for intermediate and ' 'output files') diff --git a/tools/binman/control.py b/tools/binman/control.py index dc1dd2a7dcf..8c6eae83f15 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -387,7 +387,7 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt): def ProcessImage(image, update_fdt, write_map, get_contents=True, - allow_resize=True): + allow_resize=True, allow_missing=False): """Perform all steps for this image, including checking and # writing it. This means that errors found with a later image will be reported after @@ -402,8 +402,10 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, the contents is already present allow_resize: True to allow entries to change size (this does a re-pack of the entries), False to raise an exception + allow_missing: Allow blob_ext objects to be missing """ if get_contents: + image.SetAllowMissing(allow_missing) image.GetEntryContents() image.GetEntryOffsets() @@ -523,7 +525,8 @@ def Binman(args): images = PrepareImagesAndDtbs(dtb_fname, args.image, args.update_fdt) for image in images.values(): - ProcessImage(image, args.update_fdt, args.map) + ProcessImage(image, args.update_fdt, args.map, + allow_missing=args.allow_missing) # Write the updated FDTs to our output files for dtb_item in state.GetAllFdts(): diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 90ffd276177..9388586e7cb 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -794,3 +794,12 @@ features to produce new behaviours. elif self == entries[-1]: return 'end' return 'middle' + + def SetAllowMissing(self, allow_missing): + """Set whether a section allows missing external blobs + + Args: + allow_missing: True if allowed, False if not allowed + """ + # This is meaningless for anything other than sections + pass diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index cc8d91bb599..51779c88c90 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -18,6 +18,9 @@ class Entry_blob_ext(Entry_blob): Note: This should not be used by itself. It is normally used as a parent class by other entry types. + If the file providing this blob is missing, binman can optionally ignore it + and produce a broken image with a warning. + See 'blob' for Properties / Entry arguments. """ def __init__(self, section, etype, node): @@ -26,6 +29,10 @@ class Entry_blob_ext(Entry_blob): def ObtainContents(self): self._filename = self.GetDefaultFilename() - self._pathname = tools.GetInputFilename(self._filename) - self.ReadBlobContents() - return True + self._pathname = tools.GetInputFilename(self._filename, + self.section.GetAllowMissing()) + # Allow the file to be missing + if not self._pathname: + self.SetContents(b'') + return True + return super().ObtainContents() diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index f108121c3a2..9b718f1fa77 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -34,6 +34,11 @@ class Entry_section(Entry): name-prefix: Adds a prefix to the name of every entry in the section when writing out the map + Properties: + _allow_missing: True if this section permits external blobs to be + missing their contents. The second will produce an image but of + course it will not work. + Since a section is also an entry, it inherits all the properies of entries too. @@ -49,6 +54,7 @@ class Entry_section(Entry): self._sort = False self._skip_at_start = None self._end_4gb = False + self._allow_missing = False def ReadNode(self): """Read properties from the image node""" @@ -535,3 +541,21 @@ class Entry_section(Entry): def WriteChildData(self, child): return True + + def SetAllowMissing(self, allow_missing): + """Set whether a section allows missing external blobs + + Args: + allow_missing: True if allowed, False if not allowed + """ + self._allow_missing = allow_missing + for entry in self._entries.values(): + entry.SetAllowMissing(allow_missing) + + def GetAllowMissing(self): + """Get whether a section allows missing external blobs + + Returns: + True if allowed, False if not allowed + """ + return self._allow_missing diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f8d51916723..7c8b3eb3a09 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -285,7 +285,7 @@ class TestFunctional(unittest.TestCase): def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, entry_args=None, images=None, use_real_dtb=False, - verbosity=None): + verbosity=None, allow_missing=False): """Run binman with a given test file Args: @@ -319,6 +319,8 @@ class TestFunctional(unittest.TestCase): if entry_args: for arg, value in entry_args.items(): args.append('-a%s=%s' % (arg, value)) + if allow_missing: + args.append('-M') if images: for image in images: args += ['-i', image] @@ -3376,6 +3378,10 @@ class TestFunctional(unittest.TestCase): self.assertIn("Filename 'missing-file' not found in input path", str(e.exception)) + def testExtblobMissingOk(self): + """Test an image with an missing external blob that is allowed""" + self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True) + if __name__ == "__main__": unittest.main() diff --git a/tools/patman/tools.py b/tools/patman/tools.py index f402b9aab8b..d41115a22c4 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -114,14 +114,16 @@ def SetInputDirs(dirname): indir = dirname tout.Debug("Using input directories %s" % indir) -def GetInputFilename(fname): +def GetInputFilename(fname, allow_missing=False): """Return a filename for use as input. Args: fname: Filename to use for new file + allow_missing: True if the filename can be missing Returns: - The full path of the filename, within the input directory + The full path of the filename, within the input directory, or + None on error """ if not indir or fname[:1] == '/': return fname @@ -130,6 +132,8 @@ def GetInputFilename(fname): if os.path.exists(pathname): return pathname + if allow_missing: + return None raise ValueError("Filename '%s' not found in input path (%s) (cwd='%s')" % (fname, ','.join(indir), os.getcwd())) -- cgit v1.2.3 From 38fdb4cb35e9260a6aa78ffcfa68d39bfc3523de Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:39 -0600 Subject: patman: Update errors and warnings to use stderr When warnings and errors are produced by tools they should be written to stderr. Update the tout implementation to handle this. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/ftest.py | 2 +- tools/patman/tout.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 7c8b3eb3a09..928d3608a31 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3232,7 +3232,7 @@ class TestFunctional(unittest.TestCase): with test_util.capture_sys_output() as (stdout, stderr): control.ReplaceEntries(updated_fname, None, outdir, []) self.assertIn("Skipping entry '/u-boot' from missing file", - stdout.getvalue()) + stderr.getvalue()) def testReplaceCmdMap(self): """Test replacing a file fron an image on the command line""" diff --git a/tools/patman/tout.py b/tools/patman/tout.py index c7e32720965..91a53f4073d 100644 --- a/tools/patman/tout.py +++ b/tools/patman/tout.py @@ -83,7 +83,10 @@ def _Output(level, msg, color=None): ClearProgress() if color: msg = _color.Color(color, msg) - print(msg) + if level < NOTICE: + print(msg, file=sys.stderr) + else: + print(msg) def DoOutput(level, msg): """Output a message to the terminal. -- cgit v1.2.3 From b1cca9552c6c930d094806a64b2096411783d13f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:40 -0600 Subject: binman: Detect when valid images are not produced When external blobs are missing, show a message indicating that the images are not functional. Signed-off-by: Simon Glass --- tools/binman/control.py | 16 ++++++++++++++-- tools/binman/entry.py | 12 ++++++++++++ tools/binman/etype/blob_ext.py | 1 + tools/binman/etype/section.py | 12 ++++++++++++ tools/binman/ftest.py | 14 +++++++++++++- tools/binman/test/159_blob_ext_missing_sect.dts | 23 +++++++++++++++++++++++ tools/patman/tout.py | 1 + 7 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 tools/binman/test/159_blob_ext_missing_sect.dts (limited to 'tools') diff --git a/tools/binman/control.py b/tools/binman/control.py index 8c6eae83f15..343b0a0c35b 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -403,6 +403,9 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, allow_resize: True to allow entries to change size (this does a re-pack of the entries), False to raise an exception allow_missing: Allow blob_ext objects to be missing + + Returns: + True if one or more external blobs are missing, False if all are present """ if get_contents: image.SetAllowMissing(allow_missing) @@ -450,6 +453,12 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, image.BuildImage() if write_map: image.WriteMap() + missing_list = [] + image.CheckMissing(missing_list) + if missing_list: + tout.Warning("Image '%s' is missing external blobs and is non-functional: %s" % + (image.name, ' '.join([e.name for e in missing_list]))) + return bool(missing_list) def Binman(args): @@ -524,14 +533,17 @@ def Binman(args): images = PrepareImagesAndDtbs(dtb_fname, args.image, args.update_fdt) + missing = False for image in images.values(): - ProcessImage(image, args.update_fdt, args.map, - allow_missing=args.allow_missing) + missing |= ProcessImage(image, args.update_fdt, args.map, + allow_missing=args.allow_missing) # Write the updated FDTs to our output files for dtb_item in state.GetAllFdts(): tools.WriteFile(dtb_item._fname, dtb_item.GetContents()) + if missing: + tout.Warning("Some images are invalid") finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 9388586e7cb..3434a3f8048 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -84,6 +84,7 @@ class Entry(object): self.image_pos = None self._expand_size = False self.compress = 'none' + self.missing = False @staticmethod def Lookup(node_path, etype): @@ -803,3 +804,14 @@ features to produce new behaviours. """ # This is meaningless for anything other than sections pass + + def CheckMissing(self, missing_list): + """Check if any entries in this section have missing external blobs + + If there are missing blobs, the entries are added to the list + + Args: + missing_list: List of Entry objects to be added to + """ + if self.missing: + missing_list.append(self) diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index 51779c88c90..8d641001a9e 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -34,5 +34,6 @@ class Entry_blob_ext(Entry_blob): # Allow the file to be missing if not self._pathname: self.SetContents(b'') + self.missing = True return True return super().ObtainContents() diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 9b718f1fa77..dd7f1ccd098 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -55,6 +55,7 @@ class Entry_section(Entry): self._skip_at_start = None self._end_4gb = False self._allow_missing = False + self.missing = False def ReadNode(self): """Read properties from the image node""" @@ -559,3 +560,14 @@ class Entry_section(Entry): True if allowed, False if not allowed """ return self._allow_missing + + def CheckMissing(self, missing_list): + """Check if any entries in this section have missing external blobs + + If there are missing blobs, the entries are added to the list + + Args: + missing_list: List of Entry objects to be added to + """ + for entry in self._entries.values(): + entry.CheckMissing(missing_list) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 928d3608a31..cc551c9f176 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3380,7 +3380,19 @@ class TestFunctional(unittest.TestCase): def testExtblobMissingOk(self): """Test an image with an missing external blob that is allowed""" - self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True) + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True) + err = stderr.getvalue() + self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext") + + def testExtblobMissingOkSect(self): + """Test an image with an missing external blob that is allowed""" + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('159_blob_ext_missing_sect.dts', + allow_missing=True) + err = stderr.getvalue() + self.assertRegex(err, "Image 'main-section'.*missing.*: " + "blob-ext blob-ext2") if __name__ == "__main__": diff --git a/tools/binman/test/159_blob_ext_missing_sect.dts b/tools/binman/test/159_blob_ext_missing_sect.dts new file mode 100644 index 00000000000..5f14c541381 --- /dev/null +++ b/tools/binman/test/159_blob_ext_missing_sect.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x80>; + + section { + blob-ext { + filename = "missing-file"; + }; + }; + + blob-ext2 { + type = "blob-ext"; + filename = "missing-file2"; + }; + }; +}; diff --git a/tools/patman/tout.py b/tools/patman/tout.py index 91a53f4073d..33305263d8e 100644 --- a/tools/patman/tout.py +++ b/tools/patman/tout.py @@ -171,6 +171,7 @@ def Init(_verbose=WARNING, stdout=sys.stdout): # TODO(sjg): Move this into Chromite libraries when we have them stdout_is_tty = hasattr(sys.stdout, 'isatty') and sys.stdout.isatty() + stderr_is_tty = hasattr(sys.stderr, 'isatty') and sys.stderr.isatty() def Uninit(): ClearProgress() -- cgit v1.2.3 From 0ba4b3dfee57a4e5803abd881e9654fb0414a3c1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:41 -0600 Subject: binman: Allow missing Intel blobs Update the Intel blob entries to support missing binaries. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/etype/intel_descriptor.py | 7 +++++- tools/binman/etype/intel_ifwi.py | 17 ++++++++++---- tools/binman/etype/section.py | 4 ++-- tools/binman/ftest.py | 41 ++++++++++++++++++++++++++++------ 4 files changed, 55 insertions(+), 14 deletions(-) (limited to 'tools') diff --git a/tools/binman/etype/intel_descriptor.py b/tools/binman/etype/intel_descriptor.py index 5b18893ccdc..7fe88a9ec1a 100644 --- a/tools/binman/etype/intel_descriptor.py +++ b/tools/binman/etype/intel_descriptor.py @@ -55,6 +55,12 @@ class Entry_intel_descriptor(Entry_blob_ext): return super().Pack(offset) def GetOffsets(self): + info = {} + if self.missing: + # Return zero offsets so that these entries get placed somewhere + if self.HasSibling('intel-me'): + info['intel-me'] = [0, None] + return info offset = self.data.find(FD_SIGNATURE) if offset == -1: self.Raise('Cannot find Intel Flash Descriptor (FD) signature') @@ -66,7 +72,6 @@ class Entry_intel_descriptor(Entry_blob_ext): # Set the offset for ME (Management Engine) and IFWI (Integrated # Firmware Image), for now, since the others are not used. - info = {} if self.HasSibling('intel-me'): info['intel-me'] = [self._regions[REGION_ME].base, self._regions[REGION_ME].size] diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index b0c2b1aaa33..76b3357c252 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -84,7 +84,7 @@ class Entry_intel_ifwi(Entry_blob_ext): return True def ObtainContents(self): - """Get the contects for the IFWI + """Get the contents for the IFWI Unfortunately we cannot create anything from scratch here, as Intel has tools which create precursor binaries with lots of data and settings, @@ -97,13 +97,21 @@ class Entry_intel_ifwi(Entry_blob_ext): After that we delete the OBBP sub-partition and add each of the files that we want in the IFWI file, one for each sub-entry of the IWFI node. """ - self._pathname = tools.GetInputFilename(self._filename) + self._pathname = tools.GetInputFilename(self._filename, + self.section.GetAllowMissing()) + # Allow the file to be missing + if not self._pathname: + self.SetContents(b'') + self.missing = True + return True for entry in self._ifwi_entries.values(): if not entry.ObtainContents(): return False return self._BuildIfwi() def ProcessContents(self): + if self.missing: + return True orig_data = self.data self._BuildIfwi() same = orig_data == self.data @@ -121,5 +129,6 @@ class Entry_intel_ifwi(Entry_blob_ext): def WriteSymbols(self, section): """Write symbol values into binary files for access at run time""" - for entry in self._ifwi_entries.values(): - entry.WriteSymbols(self) + if not self.missing: + for entry in self._ifwi_entries.values(): + entry.WriteSymbols(self) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index dd7f1ccd098..7cd12c0204e 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -442,8 +442,8 @@ class Entry_section(Entry): if not entry: self._Raise("Unable to set offset/size for unknown entry '%s'" % name) - entry.SetOffsetSize(self._skip_at_start + offset if offset else None, - size) + entry.SetOffsetSize(self._skip_at_start + offset if offset is not None + else None, size) def GetEntryOffsets(self): """Handle entries that want to set the offset/size of other entries diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index cc551c9f176..146d4c51d3f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -160,8 +160,7 @@ class TestFunctional(unittest.TestCase): tools.ReadFile(cls.ElfTestFile('u_boot_ucode_ptr'))) # Intel flash descriptor file - with open(cls.TestFile('descriptor.bin'), 'rb') as fd: - TestFunctional._MakeInputFile('descriptor.bin', fd.read()) + cls._SetupDescriptor() shutil.copytree(cls.TestFile('files'), os.path.join(cls._indir, 'files')) @@ -507,6 +506,11 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('tpl/u-boot-tpl', tools.ReadFile(cls.ElfTestFile(src_fname))) + @classmethod + def _SetupDescriptor(cls): + with open(cls.TestFile('descriptor.bin'), 'rb') as fd: + TestFunctional._MakeInputFile('descriptor.bin', fd.read()) + @classmethod def TestFile(cls, fname): return os.path.join(cls._binman_dir, 'test', fname) @@ -933,11 +937,14 @@ class TestFunctional(unittest.TestCase): def testPackX86RomMeNoDesc(self): """Test that an invalid Intel descriptor entry is detected""" - TestFunctional._MakeInputFile('descriptor.bin', b'') - with self.assertRaises(ValueError) as e: - self._DoTestFile('031_x86_rom_me.dts') - self.assertIn("Node '/binman/intel-descriptor': Cannot find Intel Flash Descriptor (FD) signature", - str(e.exception)) + try: + TestFunctional._MakeInputFile('descriptor.bin', b'') + with self.assertRaises(ValueError) as e: + self._DoTestFile('031_x86_rom_me.dts') + self.assertIn("Node '/binman/intel-descriptor': Cannot find Intel Flash Descriptor (FD) signature", + str(e.exception)) + finally: + self._SetupDescriptor() def testPackX86RomBadDesc(self): """Test that the Intel requires a descriptor entry""" @@ -3394,6 +3401,26 @@ class TestFunctional(unittest.TestCase): self.assertRegex(err, "Image 'main-section'.*missing.*: " "blob-ext blob-ext2") + def testPackX86RomMeMissingDesc(self): + """Test that an missing Intel descriptor entry is allowed""" + pathname = os.path.join(self._indir, 'descriptor.bin') + os.remove(pathname) + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('031_x86_rom_me.dts', allow_missing=True) + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing.*: intel-descriptor") + + def testPackX86RomMissingIfwi(self): + """Test that an x86 ROM with Integrated Firmware Image can be created""" + self._SetupIfwi('fitimage.bin') + pathname = os.path.join(self._indir, 'fitimage.bin') + os.remove(pathname) + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('111_x86_rom_ifwi.dts', allow_missing=True) + err = stderr.getvalue() + self.assertRegex(err, "Image 'main-section'.*missing.*: intel-ifwi") + if __name__ == "__main__": unittest.main() -- cgit v1.2.3 From b3295fd4e3d61a7ceabbad10a293db2dcd38b38b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:42 -0600 Subject: binman: Allow zero-length entries to overlap Some binary blobs unfortunately obtain their position in the image from other binary blobs, such as Intel's 'descriptor'. In this case we cannot rely on packing to work. It is not possible to produce a valid image in any case, due to the missing blobs. Allow zero-length overlaps so that this does not cause any problems. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/binman/etype/section.py | 2 +- tools/binman/ftest.py | 4 ++++ tools/binman/test/160_pack_overlap_zero.dts | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/160_pack_overlap_zero.dts (limited to 'tools') diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 7cd12c0204e..73c5553c812 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -226,7 +226,7 @@ class Entry_section(Entry): "at %#x (%d)" % (entry.offset, entry.offset, self._skip_at_start, self._skip_at_start)) - if entry.offset < offset: + if entry.offset < offset and entry.size: entry.Raise("Offset %#x (%d) overlaps with previous entry '%s' " "ending at %#x (%d)" % (entry.offset, entry.offset, prev_name, offset, offset)) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 146d4c51d3f..614ac4ed39d 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3421,6 +3421,10 @@ class TestFunctional(unittest.TestCase): err = stderr.getvalue() self.assertRegex(err, "Image 'main-section'.*missing.*: intel-ifwi") + def testPackOverlap(self): + """Test that zero-size overlapping regions are ignored""" + self._DoTestFile('160_pack_overlap_zero.dts') + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/160_pack_overlap_zero.dts b/tools/binman/test/160_pack_overlap_zero.dts new file mode 100644 index 00000000000..731aa1cbe6d --- /dev/null +++ b/tools/binman/test/160_pack_overlap_zero.dts @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + + fill { + size = <0>; + offset = <3>; + }; + }; +}; -- cgit v1.2.3 From 152b246298bd5f107f5e08ddddee40411aa74cb7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:43 -0600 Subject: mkimage: Allow updating the FIT timestamp Normally the FIT timestamp is created the first time mkimage is run on a FIT, when converting the source .its to the binary .fit file. This corresponds to using the -f flag. But if the original input to mkimage is a binary file (already compiled) then the timestamp is assumed to have been set previously. Add a -t flag to allow setting the timestamp in this case. Signed-off-by: Simon Glass Reviewed-by: Bin Meng --- tools/fit_image.c | 2 +- tools/imagetool.h | 1 + tools/mkimage.c | 5 ++++- 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/fit_image.c b/tools/fit_image.c index a082d9386d2..df310b53da3 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -53,7 +53,7 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc, } /* for first image creation, add a timestamp at offset 0 i.e., root */ - if (params->datafile) { + if (params->datafile || params->reset_timestamp) { time_t time = imagetool_get_source_date(params->cmdname, sbuf.st_mtime); ret = fit_set_timestamp(ptr, 0, time); diff --git a/tools/imagetool.h b/tools/imagetool.h index f54809cd578..acbc48e9be0 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -81,6 +81,7 @@ struct image_tool_params { unsigned int external_offset; /* Add padding to external data */ int bl_len; /* Block length in byte for external data */ const char *engine_id; /* Engine to use for signing */ + bool reset_timestamp; /* Reset the timestamp on an existing image */ }; /* diff --git a/tools/mkimage.c b/tools/mkimage.c index 7cb666d4822..43078d075cf 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -145,7 +145,7 @@ static void process_args(int argc, char **argv) int opt; while ((opt = getopt(argc, argv, - "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) { + "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) { switch (opt) { case 'a': params.addr = strtoull(optarg, &ptr, 16); @@ -269,6 +269,9 @@ static void process_args(int argc, char **argv) case 's': params.skipcpy = 1; break; + case 't': + params.reset_timestamp = 1; + break; case 'T': if (strcmp(optarg, "list") == 0) { show_valid_options(IH_TYPE); -- cgit v1.2.3 From c06391790633e116c9dc6a63c1649b13d7adb34e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:44 -0600 Subject: dtoc: Allow adding variable-sized data to a dtb Add a method for adding a property containing arbitrary bytes. Make sure that the tree can expand as needed in this case. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 17 +++++++++++++++-- tools/dtoc/test_fdt.py | 4 ++++ 2 files changed, 19 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 188490b728f..d058c59e927 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -207,7 +207,8 @@ class Prop: if auto_resize: while fdt_obj.setprop(node.Offset(), self.name, self.bytes, (libfdt.NOSPACE,)) == -libfdt.NOSPACE: - fdt_obj.resize(fdt_obj.totalsize() + 1024) + fdt_obj.resize(fdt_obj.totalsize() + 1024 + + len(self.bytes)) fdt_obj.setprop(node.Offset(), self.name, self.bytes) else: fdt_obj.setprop(node.Offset(), self.name, self.bytes) @@ -410,6 +411,18 @@ class Node: val = val.encode('utf-8') self._CheckProp(prop_name).props[prop_name].SetData(val + b'\0') + def AddData(self, prop_name, val): + """Add a new property to a node + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to add + val: Bytes value of property + """ + self.props[prop_name] = Prop(self, None, prop_name, val) + def AddString(self, prop_name, val): """Add a new string property to a node @@ -422,7 +435,7 @@ class Node: """ if sys.version_info[0] >= 3: # pragma: no cover val = bytes(val, 'utf-8') - self.props[prop_name] = Prop(self, None, prop_name, val + b'\0') + self.AddData(prop_name, val + b'\0') def AddSubnode(self, name): """Add a new subnode to the node diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 375e906424c..b4f9b7f498f 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -417,6 +417,10 @@ class TestProp(unittest.TestCase): self.node.SetData('empty', b'123') self.assertEqual(b'123', prop.bytes) + # Trying adding a lot of data at once + self.node.AddData('data', tools.GetBytes(65, 20000)) + self.dtb.Sync(auto_resize=True) + def testFromData(self): dtb2 = fdt.Fdt.FromData(self.dtb.GetContents()) self.assertEqual(dtb2.GetContents(), self.dtb.GetContents()) -- cgit v1.2.3 From fdc34368ddcd1d2eb46f9a1829f080f8c1760dee Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:45 -0600 Subject: binman: Add support for generating a FIT FIT (Flat Image Tree) is the main image format used by U-Boot. In some cases scripts are used to create FITs within the U-Boot build system. This is not ideal for various reasons: - Each architecture has its own slightly different script - There are no tests - Some are written in shell, some in Python To help address this, add support for FIT generation to binman. This works by putting the FIT source directly in the binman definition, with the ability to adjust parameters, etc. The contents of each FIT image come from sub-entries of the image, as is normal with binman. Signed-off-by: Simon Glass --- tools/binman/README.entries | 40 ++++++++ tools/binman/etype/fit.py | 164 +++++++++++++++++++++++++++++++++ tools/binman/ftest.py | 54 +++++++++++ tools/binman/test/161_fit.dts | 62 +++++++++++++ tools/binman/test/162_fit_external.dts | 64 +++++++++++++ 5 files changed, 384 insertions(+) create mode 100644 tools/binman/etype/fit.py create mode 100644 tools/binman/test/161_fit.dts create mode 100644 tools/binman/test/162_fit_external.dts (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index f45f51428ad..bf8edce02b4 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -311,6 +311,46 @@ byte value of a region. +Entry: fit: Entry containing a FIT +---------------------------------- + +This calls mkimage to create a FIT (U-Boot Flat Image Tree) based on the +input provided. + +Nodes for the FIT should be written out in the binman configuration just as +they would be in a file passed to mkimage. + +For example, this creates an image containing a FIT with U-Boot SPL: + + binman { + fit { + description = "Test FIT"; + + images { + kernel@1 { + description = "SPL"; + os = "u-boot"; + type = "rkspi"; + arch = "arm"; + compression = "none"; + load = <0>; + entry = <0>; + + u-boot-spl { + }; + }; + }; + }; + }; + +Properties: + fit,external-offset: Indicates that the contents of the FIT are external + and provides the external offset. This is passsed to mkimage via + the -E and -p flags. + + + + Entry: fmap: An entry which contains an Fmap section ---------------------------------------------------- diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py new file mode 100644 index 00000000000..75712f44092 --- /dev/null +++ b/tools/binman/etype/fit.py @@ -0,0 +1,164 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# Entry-type module for producing a FIT +# + +from collections import defaultdict, OrderedDict +import libfdt + +from binman.entry import Entry +from dtoc import fdt_util +from dtoc.fdt import Fdt +from patman import tools + +class Entry_fit(Entry): + """Entry containing a FIT + + This calls mkimage to create a FIT (U-Boot Flat Image Tree) based on the + input provided. + + Nodes for the FIT should be written out in the binman configuration just as + they would be in a file passed to mkimage. + + For example, this creates an image containing a FIT with U-Boot SPL: + + binman { + fit { + description = "Test FIT"; + + images { + kernel@1 { + description = "SPL"; + os = "u-boot"; + type = "rkspi"; + arch = "arm"; + compression = "none"; + load = <0>; + entry = <0>; + + u-boot-spl { + }; + }; + }; + }; + }; + + Properties: + fit,external-offset: Indicates that the contents of the FIT are external + and provides the external offset. This is passsed to mkimage via + the -E and -p flags. + + """ + def __init__(self, section, etype, node): + """ + Members: + _fit: FIT file being built + _fit_content: dict: + key: relative path to entry Node (from the base of the FIT) + value: List of Entry objects comprising the contents of this + node + """ + super().__init__(section, etype, node) + self._fit = None + self._fit_content = defaultdict(list) + self._fit_props = {} + + def ReadNode(self): + self._ReadSubnodes() + super().ReadNode() + + def _ReadSubnodes(self): + def _AddNode(base_node, depth, node): + """Add a node to the FIT + + Args: + base_node: Base Node of the FIT (with 'description' property) + depth: Current node depth (0 is the base node) + node: Current node to process + + There are two cases to deal with: + - hash and signature nodes which become part of the FIT + - binman entries which are used to define the 'data' for each + image + """ + for pname, prop in node.props.items(): + if pname.startswith('fit,'): + self._fit_props[pname] = prop + else: + fsw.property(pname, prop.bytes) + + rel_path = node.path[len(base_node.path):] + has_images = depth == 2 and rel_path.startswith('/images/') + for subnode in node.subnodes: + if has_images and not (subnode.name.startswith('hash') or + subnode.name.startswith('signature')): + # This is a content node. We collect all of these together + # and put them in the 'data' property. They do not appear + # in the FIT. + entry = Entry.Create(self.section, subnode) + entry.ReadNode() + self._fit_content[rel_path].append(entry) + else: + with fsw.add_node(subnode.name): + _AddNode(base_node, depth + 1, subnode) + + # Build a new tree with all nodes and properties starting from the + # entry node + fsw = libfdt.FdtSw() + fsw.finish_reservemap() + with fsw.add_node(''): + _AddNode(self._node, 0, self._node) + fdt = fsw.as_fdt() + + # Pack this new FDT and scan it so we can add the data later + fdt.pack() + self._fdt = Fdt.FromData(fdt.as_bytearray()) + self._fdt.Scan() + + def ObtainContents(self): + """Obtain the contents of the FIT + + This adds the 'data' properties to the input ITB (Image-tree Binary) + then runs mkimage to process it. + """ + data = self._BuildInput(self._fdt) + if data == False: + return False + uniq = self.GetUniqueName() + input_fname = tools.GetOutputFilename('%s.itb' % uniq) + output_fname = tools.GetOutputFilename('%s.fit' % uniq) + tools.WriteFile(input_fname, data) + tools.WriteFile(output_fname, data) + + args = [] + ext_offset = self._fit_props.get('fit,external-offset') + if ext_offset is not None: + args += ['-E', '-p', '%x' % fdt_util.fdt32_to_cpu(ext_offset.value)] + tools.Run('mkimage', '-t', '-F', output_fname, *args) + + self.SetContents(tools.ReadFile(output_fname)) + return True + + def _BuildInput(self, fdt): + """Finish the FIT by adding the 'data' properties to it + + Arguments: + fdt: FIT to update + + Returns: + New fdt contents (bytes) + """ + for path, entries in self._fit_content.items(): + node = fdt.GetNode(path) + data = b'' + for entry in entries: + if not entry.ObtainContents(): + return False + data += entry.GetData() + node.AddData('data', data) + + fdt.Sync(auto_resize=True) + data = fdt.GetContents() + return data diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 614ac4ed39d..ea72eff8c5d 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6,10 +6,12 @@ # # python -m unittest func_test.TestFunctional.testHelp +import collections import gzip import hashlib from optparse import OptionParser import os +import re import shutil import struct import sys @@ -3425,6 +3427,58 @@ class TestFunctional(unittest.TestCase): """Test that zero-size overlapping regions are ignored""" self._DoTestFile('160_pack_overlap_zero.dts') + def testSimpleFit(self): + """Test an image with a FIT inside""" + data = self._DoReadFile('161_fit.dts') + self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)]) + self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):]) + fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)] + + # The data should be inside the FIT + dtb = fdt.Fdt.FromData(fit_data) + dtb.Scan() + fnode = dtb.GetNode('/images/kernel') + self.assertIn('data', fnode.props) + + fname = os.path.join(self._indir, 'fit_data.fit') + tools.WriteFile(fname, fit_data) + out = tools.Run('dumpimage', '-l', fname) + + # Check a few features to make sure the plumbing works. We don't need + # to test the operation of mkimage or dumpimage here. First convert the + # output into a dict where the keys are the fields printed by dumpimage + # and the values are a list of values for each field + lines = out.splitlines() + + # Converts "Compression: gzip compressed" into two groups: + # 'Compression' and 'gzip compressed' + re_line = re.compile(r'^ *([^:]*)(?:: *(.*))?$') + vals = collections.defaultdict(list) + for line in lines: + mat = re_line.match(line) + vals[mat.group(1)].append(mat.group(2)) + + self.assertEquals('FIT description: test-desc', lines[0]) + self.assertIn('Created:', lines[1]) + self.assertIn('Image 0 (kernel)', vals) + self.assertIn('Hash value', vals) + data_sizes = vals.get('Data Size') + self.assertIsNotNone(data_sizes) + self.assertEqual(2, len(data_sizes)) + # Format is "4 Bytes = 0.00 KiB = 0.00 MiB" so take the first word + self.assertEqual(len(U_BOOT_DATA), int(data_sizes[0].split()[0])) + self.assertEqual(len(U_BOOT_SPL_DTB_DATA), int(data_sizes[1].split()[0])) + + def testFitExternal(self): + """Test an image with an FIT""" + data = self._DoReadFile('162_fit_external.dts') + fit_data = data[len(U_BOOT_DATA):-2] # _testing is 2 bytes + + # The data should be outside the FIT + dtb = fdt.Fdt.FromData(fit_data) + dtb.Scan() + fnode = dtb.GetNode('/images/kernel') + self.assertNotIn('data', fnode.props) if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/161_fit.dts b/tools/binman/test/161_fit.dts new file mode 100644 index 00000000000..c52d760b735 --- /dev/null +++ b/tools/binman/test/161_fit.dts @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + fit { + description = "test-desc"; + #address-cells = <1>; + + images { + kernel { + description = "Vanilla Linux kernel"; + type = "kernel"; + arch = "ppc"; + os = "linux"; + compression = "gzip"; + load = <00000000>; + entry = <00000000>; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + u-boot { + }; + }; + fdt-1 { + description = "Flattened Device Tree blob"; + type = "flat_dt"; + arch = "ppc"; + compression = "none"; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + u-boot-spl-dtb { + }; + }; + }; + + configurations { + default = "conf-1"; + conf-1 { + description = "Boot Linux kernel with FDT blob"; + kernel = "kernel"; + fdt = "fdt-1"; + }; + }; + }; + u-boot-nodtb { + }; + }; +}; diff --git a/tools/binman/test/162_fit_external.dts b/tools/binman/test/162_fit_external.dts new file mode 100644 index 00000000000..19518e05a58 --- /dev/null +++ b/tools/binman/test/162_fit_external.dts @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + fit { + fit,external-offset = <0>; + description = "test-desc"; + #address-cells = <1>; + + images { + kernel { + description = "Vanilla Linux kernel"; + type = "kernel"; + arch = "ppc"; + os = "linux"; + compression = "gzip"; + load = <00000000>; + entry = <00000000>; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + u-boot { + }; + }; + fdt-1 { + description = "Flattened Device Tree blob"; + type = "flat_dt"; + arch = "ppc"; + compression = "none"; + hash-1 { + algo = "crc32"; + }; + hash-2 { + algo = "sha1"; + }; + _testing { + return-contents-later; + }; + }; + }; + + configurations { + default = "conf-1"; + conf-1 { + description = "Boot Linux kernel with FDT blob"; + kernel = "kernel"; + fdt = "fdt-1"; + }; + }; + }; + u-boot-nodtb { + }; + }; +}; -- cgit v1.2.3 From 7058dd071accc0bafe5c4024e30631fb56fd7126 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Thu, 2 Jul 2020 19:08:24 +0200 Subject: patman: Detect unexpected END Detect unexpected 'END' line when a section is not detected. This patch detect issue when tag name for section start is misspelled, for example 'Commit-note:' for 'Commit-notes:' Commit-note: .... END Then 'Commit-note:' is removed silently by re_remove = "Commit-\w*:" but 'END' is kept in commit message. Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass --- tools/patman/patchstream.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'tools') diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 0c68c861569..70acb096423 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -269,6 +269,10 @@ class PatchStream: else: self.section.append(line) + # If we are not in a section, it is an unexpected END + elif line == 'END': + raise ValueError("'END' wihout section") + # Detect the commit subject elif not is_blank and self.state == STATE_PATCH_SUBJECT: self.commit.subject = line -- cgit v1.2.3 From e5ff9ab70a0409b1a3d263de7346f46568565ba5 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Thu, 2 Jul 2020 19:52:54 +0200 Subject: Add information for skipped commit options The unsupported Commit-xxx option are silently skipped and removed as 're_remove=Commit-\w*', this patch adds warning message in this case to detect misspelled issue for the 2 supported options: Commit-notes: Commit-changes: For example: the final 's' is missing (Commit-note:) NB: no issue for Series-xxx option as only the supported options are accepted (see valid_series in series.py) Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass --- tools/patman/patchstream.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tools') diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 70acb096423..ba0a13f6325 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -351,6 +351,9 @@ class PatchStream: elif name == 'changes': self.in_change = 'Commit' self.change_version = self.ParseVersion(value, line) + else: + self.warn.append('Line %d: Ignoring Commit-%s' % + (self.linenum, name)) # Detect the start of a new commit elif commit_match: -- cgit v1.2.3 From 949775689e58cf7ebf96b9da7f259820e8ad4f32 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Mon, 13 Jul 2020 10:50:00 +0800 Subject: patman: Make sure sendemail.suppresscc is (un)set correctly Setting sendemail.suppresscc to all or cccmd leads to --cc-cmd parameter being ignored, and emails going either nowhere, or just to the To: line maintainer. Signed-off-by: Nicolas Boichat Reviewed-by: Simon Glass --- tools/patman/control.py | 2 ++ tools/patman/gitutil.py | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) (limited to 'tools') diff --git a/tools/patman/control.py b/tools/patman/control.py index e67867b845c..8f4afeab188 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -166,6 +166,8 @@ def send(args): ok = check_patches(series, patch_files, args.check_patch, args.verbose) + ok = ok and gitutil.CheckSuppressCCConfig() + its_a_go = ok or args.ignore_errors if its_a_go: email_patches( diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index b683481a574..192d8e69b32 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -344,6 +344,31 @@ def BuildEmailList(in_list, tag=None, alias=None, raise_on_error=True): return ['%s %s%s%s' % (tag, quote, email, quote) for email in result] return result +def CheckSuppressCCConfig(): + """Check if sendemail.suppresscc is configured correctly. + + Returns: + True if the option is configured correctly, False otherwise. + """ + suppresscc = command.OutputOneLine('git', 'config', 'sendemail.suppresscc', + raise_on_error=False) + + # Other settings should be fine. + if suppresscc == 'all' or suppresscc == 'cccmd': + col = terminal.Color() + + print((col.Color(col.RED, "error") + + ": git config sendemail.suppresscc set to %s\n" % (suppresscc)) + + " patman needs --cc-cmd to be run to set the cc list.\n" + + " Please run:\n" + + " git config --unset sendemail.suppresscc\n" + + " Or read the man page:\n" + + " git send-email --help\n" + + " and set an option that runs --cc-cmd\n") + return False + + return True + def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, cc_fname, self_only=False, alias=None, in_reply_to=None, thread=False, smtp_server=None): -- cgit v1.2.3 From e1db5c9b08b81754c9467d0e515ce613dbab60f9 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Mon, 13 Jul 2020 10:50:01 +0800 Subject: patman: When no tracking branch is provided, tell the user The user can either count the number of patches, or provide a tracking branch. Signed-off-by: Nicolas Boichat Reviewed-by: Simon Glass --- tools/patman/control.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/patman/control.py b/tools/patman/control.py index 8f4afeab188..67e8f397efd 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -48,8 +48,9 @@ def prepare_patches(col, branch, count, start, end, ignore_binary): count = (gitutil.CountCommitsToBranch(branch) - start) if not count: - sys.exit(col.Color(col.RED, - 'No commits found to process - please use -c flag')) + str = 'No commits found to process - please use -c flag, or run:\n' \ + ' git branch --set-upstream-to remote/branch' + sys.exit(col.Color(col.RED, str)) # Read the metadata from the commits to_do = count - end -- cgit v1.2.3 From 52b10dd7def5fa77366ee89bffde597b2e03d1a1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 25 Jul 2020 15:11:19 -0600 Subject: binman: Don't change the descriptor in tests At present testPackX86RomMeNoDesc removes the contents of the descriptor.bin file and testPackX86RomMeMissingDesc removes the file completely. If a test that relies on this file happens to run after it is removed, it will not work. Since we have no control over the selecting of tests that run in parallel and series, we must avoid changing the files. Update this tests to use separate files instead. Signed-off-by: Simon Glass --- tools/binman/ftest.py | 8 +++----- tools/binman/test/163_x86_rom_me_empty.dts | 22 ++++++++++++++++++++++ tools/binman/test/164_x86_rom_me_missing.dts | 22 ++++++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 tools/binman/test/163_x86_rom_me_empty.dts create mode 100644 tools/binman/test/164_x86_rom_me_missing.dts (limited to 'tools') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ea72eff8c5d..bf7f59fb841 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -940,9 +940,9 @@ class TestFunctional(unittest.TestCase): def testPackX86RomMeNoDesc(self): """Test that an invalid Intel descriptor entry is detected""" try: - TestFunctional._MakeInputFile('descriptor.bin', b'') + TestFunctional._MakeInputFile('descriptor-empty.bin', b'') with self.assertRaises(ValueError) as e: - self._DoTestFile('031_x86_rom_me.dts') + self._DoTestFile('163_x86_rom_me_empty.dts') self.assertIn("Node '/binman/intel-descriptor': Cannot find Intel Flash Descriptor (FD) signature", str(e.exception)) finally: @@ -3405,10 +3405,8 @@ class TestFunctional(unittest.TestCase): def testPackX86RomMeMissingDesc(self): """Test that an missing Intel descriptor entry is allowed""" - pathname = os.path.join(self._indir, 'descriptor.bin') - os.remove(pathname) with test_util.capture_sys_output() as (stdout, stderr): - self._DoTestFile('031_x86_rom_me.dts', allow_missing=True) + self._DoTestFile('164_x86_rom_me_missing.dts', allow_missing=True) err = stderr.getvalue() self.assertRegex(err, "Image 'main-section'.*missing.*: intel-descriptor") diff --git a/tools/binman/test/163_x86_rom_me_empty.dts b/tools/binman/test/163_x86_rom_me_empty.dts new file mode 100644 index 00000000000..9349d2d7245 --- /dev/null +++ b/tools/binman/test/163_x86_rom_me_empty.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + end-at-4gb; + size = <0x800000>; + intel-descriptor { + filename = "descriptor-empty.bin"; + }; + + intel-me { + filename = "me.bin"; + offset-unset; + }; + }; +}; diff --git a/tools/binman/test/164_x86_rom_me_missing.dts b/tools/binman/test/164_x86_rom_me_missing.dts new file mode 100644 index 00000000000..dce3be5e057 --- /dev/null +++ b/tools/binman/test/164_x86_rom_me_missing.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + end-at-4gb; + size = <0x800000>; + intel-descriptor { + filename = "descriptor-missing.bin"; + }; + + intel-me { + filename = "me.bin"; + offset-unset; + }; + }; +}; -- cgit v1.2.3 From 347e0f00e850028b4595287d5158c5a8f36ba910 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 9 Jul 2020 18:39:34 -0600 Subject: binman: Re-enable concurrent tests With the change to absolute imports the concurrent tests feature unfortunately broke. Fix it. We cannot easy add a warning, since the output messes up tests which check the output. Signed-off-by: Simon Glass --- tools/patman/test_util.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index 20dc1e49244..4e261755dc6 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -16,7 +16,8 @@ from io import StringIO use_concurrent = True try: - from concurrencytest import ConcurrentTestSuite, fork_for_tests + from concurrencytest.concurrencytest import ConcurrentTestSuite + from concurrencytest.concurrencytest import fork_for_tests except: use_concurrent = False @@ -50,6 +51,7 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None, glob_list = [] glob_list += exclude_list glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*'] + glob_list += ['*concurrencytest*'] test_cmd = 'test' if 'binman' in prog or 'patman' in prog else '-t' prefix = '' if build_dir: -- cgit v1.2.3