When running unit tests we don't want output to go to the terminal. Provide a way of collecting it so that it can be examined by test code later.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: - Add a function to print out the terminal output recorded

tools/patman/terminal.py | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)

diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py index 963f2f8..e78a7c1 100644 --- a/tools/patman/terminal.py +++ b/tools/patman/terminal.py @@ -14,6 +14,78 @@ import sys # Selection of when we want our output to be colored COLOR_IF_TERMINAL, COLOR_ALWAYS, COLOR_NEVER = range(3)

+# Initially, we are set up to print to the terminal +print_test_mode = False +print_test_list = [] + +class PrintLine: + """A line of text output + + Members: + text: Text line that was printed + newline: True to output a newline after the text + colour: Text colour to use + """ + def __init__(self, text, newline, colour): + self.text = text + self.newline = newline + self.colour = colour + + def __str__(self): + return 'newline=%s, colour=%s, text=%s' % (self.newline, self.colour, + self.text) + +def Print(text='', newline=True, colour=None): + """Handle a line of output to the terminal. + + In test mode this is recorded in a list. Otherwise it is output to the + terminal. + + Args: + text: Text to print + newline: True to add a new line at the end of the text + colour: Colour to use for the text + """ + if print_test_mode: + print_test_list.append(PrintLine(text, newline, colour)) + else: + if colour: + col = Color() + text = col.Color(colour, text) + print text, + if newline: + print + +def SetPrintTestMode(): + """Go into test mode, where all printing is recorded""" + global print_test_mode + + print_test_mode = True + +def GetPrintTestLines(): + """Get a list of all lines output through Print() + + Returns: + A list of PrintLine objects + """ + global print_test_list + + ret = print_test_list + print_test_list = [] + return ret + +def EchoPrintTestLines(): + """Print out the text lines collected""" + for line in print_test_list: + if line.colour: + col = Color() + print col.Color(line.colour, line.text), + else: + print line.text, + if line.newline: + print + + class Color(object): """Conditionally wraps text in ANSI color escape sequences.""" BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8)

To allow us to verify the builder's console output, send it through a function which can collect it when running in test mode.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/builder.py | 58 ++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 324239a..1b6517b 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -20,6 +20,7 @@ import builderthread import command import gitutil import terminal +from terminal import Print import toolchain

@@ -299,8 +300,8 @@ class Builder: length: Length of new line, in characters """ if length < self.last_line_len: - print ' ' * (self.last_line_len - length), - print '\r', + Print(' ' * (self.last_line_len - length), newline=False) + Print('\r', newline=False) self.last_line_len = length sys.stdout.flush()

@@ -351,7 +352,7 @@ class Builder: if result.already_done: self.already_done += 1 if self._verbose: - print '\r', + Print('\r', newline=False) self.ClearLine(0) boards_selected = {target : result.brd} self.ResetResultSummary(boards_selected) @@ -379,7 +380,7 @@ class Builder: self.commit_count)

name += target - print line + name, + Print(line + name, newline=False) length = 14 + len(name) self.ClearLine(length)

@@ -495,7 +496,7 @@ class Builder: try: size, type, name = line[:-1].split() except: - print "Invalid line in file '%s': '%s'" % (fname, line[:-1]) + Print("Invalid line in file '%s': '%s'" % (fname, line[:-1])) continue if type in 'tTdDbB': # function names begin with '.' on 64-bit powerpc @@ -723,16 +724,16 @@ class Builder: return args = [self.ColourNum(x) for x in args] indent = ' ' * 15 - print ('%s%s: add: %s/%s, grow: %s/%s bytes: %s/%s (%s)' % - tuple([indent, self.col.Color(self.col.YELLOW, fname)] + args)) - print '%s %-38s %7s %7s %+7s' % (indent, 'function', 'old', 'new', - 'delta') + Print('%s%s: add: %s/%s, grow: %s/%s bytes: %s/%s (%s)' % + tuple([indent, self.col.Color(self.col.YELLOW, fname)] + args)) + Print('%s %-38s %7s %7s %+7s' % (indent, 'function', 'old', 'new', + 'delta')) for diff, name in delta: if diff: color = self.col.RED if diff > 0 else self.col.GREEN msg = '%s %-38s %7s %7s %+7d' % (indent, name, old.get(name, '-'), new.get(name,'-'), diff) - print self.col.Color(color, msg) + Print(msg, colour=color)

def PrintSizeDetail(self, target_list, show_bloat): @@ -757,11 +758,12 @@ class Builder: color = self.col.RED if diff > 0 else self.col.GREEN msg = ' %s %+d' % (name, diff) if not printed_target: - print '%10s %-15s:' % ('', result['_target']), + Print('%10s %-15s:' % ('', result['_target']), + newline=False) printed_target = True - print self.col.Color(color, msg), + Print(msg, colour=color, newline=False) if printed_target: - print + Print() if show_bloat: target = result['_target'] outcome = result['_outcome'] @@ -866,13 +868,13 @@ class Builder: color = self.col.RED if avg_diff > 0 else self.col.GREEN msg = ' %s %+1.1f' % (name, avg_diff) if not printed_arch: - print '%10s: (for %d/%d boards)' % (arch, count, - arch_count[arch]), + Print('%10s: (for %d/%d boards)' % (arch, count, + arch_count[arch]), newline=False) printed_arch = True - print self.col.Color(color, msg), + Print(msg, colour=color, newline=False)

if printed_arch: - print + Print() if show_detail: self.PrintSizeDetail(target_list, show_bloat)

@@ -977,19 +979,19 @@ class Builder: self.AddOutcome(board_selected, arch_list, unknown, '?', self.col.MAGENTA) for arch, target_list in arch_list.iteritems(): - print '%10s: %s' % (arch, target_list) + Print('%10s: %s' % (arch, target_list)) self._error_lines += 1 if better_err: - print self.col.Color(self.col.GREEN, '\n'.join(better_err)) + Print('\n'.join(better_err), colour=self.col.GREEN) self._error_lines += 1 if worse_err: - print self.col.Color(self.col.RED, '\n'.join(worse_err)) + Print('\n'.join(worse_err), colour=self.col.RED) self._error_lines += 1 if better_warn: - print self.col.Color(self.col.YELLOW, '\n'.join(better_warn)) + Print('\n'.join(better_warn), colour=self.col.CYAN) self._error_lines += 1 if worse_warn: - print self.col.Color(self.col.MAGENTA, '\n'.join(worse_warn)) + Print('\n'.join(worse_warn), colour=self.col.MAGENTA) self._error_lines += 1

if show_sizes: @@ -1009,8 +1011,8 @@ class Builder: if not board in board_dict: not_built.append(board) if not_built: - print "Boards not built (%d): %s" % (len(not_built), - ', '.join(not_built)) + Print("Boards not built (%d): %s" % (len(not_built), + ', '.join(not_built)))

def ProduceResultSummary(self, commit_upto, commits, board_selected): (board_dict, err_lines, err_line_boards, warn_lines, @@ -1020,7 +1022,7 @@ class Builder: if commits: msg = '%02d: %s' % (commit_upto + 1, commits[commit_upto].subject) - print self.col.Color(self.col.BLUE, msg) + Print(msg, colour=self.col.BLUE) self.PrintResultSummary(board_selected, board_dict, err_lines if self._show_errors else [], err_line_boards, warn_lines if self._show_errors else [], warn_line_boards, @@ -1044,7 +1046,7 @@ class Builder: for commit_upto in range(0, self.commit_count, self._step): self.ProduceResultSummary(commit_upto, commits, board_selected) if not self._error_lines: - print self.col.Color(self.col.GREEN, '(no errors to report)') + Print('(no errors to report)', colour=self.col.GREEN)

def SetupBuild(self, board_selected, commits): @@ -1089,7 +1091,7 @@ class Builder: if os.path.exists(git_dir): gitutil.Fetch(git_dir, thread_dir) else: - print 'Cloning repo for thread %d' % thread_num + Print('Cloning repo for thread %d' % thread_num) gitutil.Clone(src_dir, thread_dir)

def _PrepareWorkingSpace(self, max_threads, setup_git): @@ -1160,6 +1162,6 @@ class Builder:

# Wait until we have processed all output self.out_queue.join() - print + Print() self.ClearLine(0) return (self.fail, self.warned)

Adjust the basic test so that it checks all console output. This will help to ensure that the builder is behaving correctly with printing summary information.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/test.py | 101 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/test.py b/tools/buildman/test.py index a51c942..f0c4d0e 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -21,20 +21,21 @@ import builder import control import command import commit +import terminal import toolchain

errors = [ '''main.c: In function 'main_loop': main.c:260:6: warning: unused variable 'joe' [-Wunused-variable] ''', - '''main.c: In function 'main_loop': + '''main.c: In function 'main_loop2': main.c:295:2: error: 'fred' undeclared (first use in this function) main.c:295:2: note: each undeclared identifier is reported only once for each function it appears in make[1]: *** [main.o] Error 1 make: *** [common/libcommon.o] Error 2 Make failed ''', - '''main.c: In function 'main_loop': + '''main.c: In function 'main_loop3': main.c:280:6: warning: unused variable 'mary' [-Wunused-variable] ''', '''powerpc-linux-ld: warning: dot moved backwards before `.bss' @@ -103,6 +104,10 @@ class TestBuild(unittest.TestCase): self.toolchains.Add('powerpc-linux-gcc', test=False) self.toolchains.Add('gcc', test=False)

+ # Avoid sending any output + terminal.SetPrintTestMode() + self._col = terminal.Color() + def Make(self, commit, brd, stage, *args, **kwargs): result = command.CommandResult() boardnum = int(brd.target[-1]) @@ -121,13 +126,26 @@ class TestBuild(unittest.TestCase):

if not os.path.isdir(target_dir): os.mkdir(target_dir) - #time.sleep(.2 + boardnum * .2)

result.combined = result.stdout + result.stderr return result

- def testBasic(self): - """Test basic builder operation""" + def assertSummary(self, text, arch, plus, boards, ok=False): + col = self._col + expected_colour = col.GREEN if ok else col.RED + expect = '%10s: ' % arch + # TODO(sjg@chromium.org): If plus is '', we shouldn't need this + expect += col.Color(expected_colour, plus) + expect += ' ' + for board in boards: + expect += col.Color(expected_colour, ' %s' % board) + self.assertEqual(text, expect) + + def testOutput(self): + """Test basic builder operation and output + + This does a line-by-line verification of the summary output. + """ output_dir = tempfile.mkdtemp() if not os.path.isdir(output_dir): os.mkdir(output_dir) @@ -138,8 +156,81 @@ class TestBuild(unittest.TestCase):

build.BuildBoards(self.commits, board_selected, keep_outputs=False, verbose=False) + lines = terminal.GetPrintTestLines() + count = 0 + for line in lines: + if line.text.strip(): + count += 1 + + # We should get one starting message, then an update for every commit + # built. + self.assertEqual(count, len(commits) * len(boards) + 1) build.SetDisplayOptions(show_errors=True); build.ShowSummary(self.commits, board_selected) + lines = terminal.GetPrintTestLines() + self.assertEqual(lines[0].text, '01: %s' % commits[0][1]) + self.assertEqual(lines[1].text, '02: %s' % commits[1][1]) + + # We expect all archs to fail + col = terminal.Color() + self.assertSummary(lines[2].text, 'sandbox', '+', ['board4']) + self.assertSummary(lines[3].text, 'arm', '+', ['board1']) + self.assertSummary(lines[4].text, 'powerpc', '+', ['board2', 'board3']) + + # Now we should have the compiler warning + self.assertEqual(lines[5].text, 'w+%s' % + errors[0].rstrip().replace('\n', '\nw+')) + self.assertEqual(lines[5].colour, col.MAGENTA) + + self.assertEqual(lines[6].text, '03: %s' % commits[2][1]) + self.assertSummary(lines[7].text, 'sandbox', '+', ['board4']) + self.assertSummary(lines[8].text, 'arm', '', ['board1'], ok=True) + self.assertSummary(lines[9].text, 'powerpc', '+', ['board2', 'board3']) + + # Compiler error + self.assertEqual(lines[10].text, '+%s' % + errors[1].rstrip().replace('\n', '\n+')) + + self.assertEqual(lines[11].text, '04: %s' % commits[3][1]) + self.assertSummary(lines[12].text, 'sandbox', '', ['board4'], ok=True) + self.assertSummary(lines[13].text, 'powerpc', '', ['board2', 'board3'], + ok=True) + + # Compile error fixed + self.assertEqual(lines[14].text, '-%s' % + errors[1].rstrip().replace('\n', '\n-')) + self.assertEqual(lines[14].colour, col.GREEN) + + self.assertEqual(lines[15].text, 'w+%s' % + errors[2].rstrip().replace('\n', '\nw+')) + self.assertEqual(lines[15].colour, col.MAGENTA) + + self.assertEqual(lines[16].text, '05: %s' % commits[4][1]) + self.assertSummary(lines[17].text, 'sandbox', '+', ['board4']) + self.assertSummary(lines[18].text, 'powerpc', '', ['board3'], ok=True) + + # The second line of errors[3] is a duplicate, so buildman will drop it + expect = errors[3].rstrip().split('\n') + expect = [expect[0]] + expect[2:] + self.assertEqual(lines[19].text, '+%s' % + '\n'.join(expect).replace('\n', '\n+')) + + self.assertEqual(lines[20].text, 'w-%s' % + errors[2].rstrip().replace('\n', '\nw-')) + + self.assertEqual(lines[21].text, '06: %s' % commits[5][1]) + self.assertSummary(lines[22].text, 'sandbox', '', ['board4'], ok=True) + + # The second line of errors[3] is a duplicate, so buildman will drop it + expect = errors[3].rstrip().split('\n') + expect = [expect[0]] + expect[2:] + self.assertEqual(lines[23].text, '-%s' % + '\n'.join(expect).replace('\n', '\n-')) + + self.assertEqual(lines[24].text, 'w-%s' % + errors[0].rstrip().replace('\n', '\nw-')) + + self.assertEqual(len(lines), 25)

def _testGit(self): """Test basic builder operation by building a branch"""

RunPipe() currently pipes the output of stdout and stderr to a pty, but this is not the intended behaviour. Fix it.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/patman/command.py | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/tools/patman/command.py b/tools/patman/command.py index 449d3d0..7212fdf 100644 --- a/tools/patman/command.py +++ b/tools/patman/command.py @@ -48,6 +48,8 @@ def RunPipe(pipe_list, infile=None, outfile=None, last_pipe = None pipeline = list(pipe_list) user_pipestr = '|'.join([' '.join(pipe) for pipe in pipe_list]) + kwargs['stdout'] = None + kwargs['stderr'] = None while pipeline: cmd = pipeline.pop(0) if last_pipe is not None:

We want to be able to issue parser commands from within buildman for test purposes. Move the parser code into its own file so we don't end up needing the buildman and test modules to reference each other.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/buildman.py | 73 ++------------------------------------- tools/buildman/cmdline.py | 85 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 71 deletions(-) create mode 100644 tools/buildman/cmdline.py

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index 1258b76..c4de857 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -8,7 +8,6 @@ """See README for more information"""

import multiprocessing -from optparse import OptionParser import os import re import sys @@ -22,7 +21,7 @@ sys.path.append(os.path.join(our_path, '../patman')) import board import builder import checkpatch -import command +import cmdline import control import doctest import gitutil @@ -59,75 +58,7 @@ def RunTests(): print err

-parser = OptionParser() -parser.add_option('-b', '--branch', type='string', - help='Branch name to build') -parser.add_option('-B', '--bloat', dest='show_bloat', - action='store_true', default=False, - help='Show changes in function code size for each board') -parser.add_option('-c', '--count', dest='count', type='int', - default=-1, help='Run build on the top n commits') -parser.add_option('-C', '--force-reconfig', dest='force_reconfig', - action='store_true', default=False, - help='Reconfigure for every commit (disable incremental build)') -parser.add_option('-d', '--detail', dest='show_detail', - action='store_true', default=False, - help='Show detailed information for each board in summary') -parser.add_option('-e', '--show_errors', action='store_true', - default=False, help='Show errors and warnings') -parser.add_option('-f', '--force-build', dest='force_build', - action='store_true', default=False, - help='Force build of boards even if already built') -parser.add_option('-F', '--force-build-failures', dest='force_build_failures', - action='store_true', default=False, - help='Force build of previously-failed build') -parser.add_option('-g', '--git', type='string', - help='Git repo containing branch to build', default='.') -parser.add_option('-G', '--config-file', type='string', - help='Path to buildman config file', default='') -parser.add_option('-H', '--full-help', action='store_true', dest='full_help', - default=False, help='Display the README file') -parser.add_option('-i', '--in-tree', dest='in_tree', - action='store_true', default=False, - help='Build in the source tree instead of a separate directory') -parser.add_option('-j', '--jobs', dest='jobs', type='int', - default=None, help='Number of jobs to run at once (passed to make)') -parser.add_option('-k', '--keep-outputs', action='store_true', - default=False, help='Keep all build output files (e.g. binaries)') -parser.add_option('-l', '--list-error-boards', action='store_true', - default=False, help='Show a list of boards next to each error/warning') -parser.add_option('--list-tool-chains', action='store_true', default=False, - help='List available tool chains') -parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run', - default=False, help="Do a try run (describe actions, but no nothing)") -parser.add_option('-o', '--output-dir', type='string', - dest='output_dir', default='..', - help='Directory where all builds happen and buildman has its workspace (default is ../)') -parser.add_option('-Q', '--quick', action='store_true', - default=False, help='Do a rough build, with limited warning resolution') -parser.add_option('-s', '--summary', action='store_true', - default=False, help='Show a build summary') -parser.add_option('-S', '--show-sizes', action='store_true', - default=False, help='Show image size variation in summary') -parser.add_option('--step', type='int', - default=1, help='Only build every n commits (0=just first and last)') -parser.add_option('-t', '--test', action='store_true', dest='test', - default=False, help='run tests') -parser.add_option('-T', '--threads', type='int', - default=None, help='Number of builder threads to use') -parser.add_option('-u', '--show_unknown', action='store_true', - default=False, help='Show boards with unknown build result') -parser.add_option('-v', '--verbose', action='store_true', - default=False, help='Show build results while the build progresses') -parser.add_option('-x', '--exclude', dest='exclude', - type='string', action='append', - help='Specify a list of boards to exclude, separated by comma') - -parser.usage += """ - -Build U-Boot for all commits in a branch. Use -n to do a dry run""" - -(options, args) = parser.parse_args() +options, args = cmdline.ParseArgs()

# Run our meagre tests if options.test: diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py new file mode 100644 index 0000000..fad9a1c --- /dev/null +++ b/tools/buildman/cmdline.py @@ -0,0 +1,85 @@ +# +# Copyright (c) 2014 Google, Inc +# +# SPDX-License-Identifier: GPL-2.0+ +# + +from optparse import OptionParser + +def ParseArgs(): + """Parse command line arguments from sys.argv[] + + Returns: + tuple containing: + options: command line options + args: command lin arguments + """ + parser = OptionParser() + parser.add_option('-b', '--branch', type='string', + help='Branch name to build') + parser.add_option('-B', '--bloat', dest='show_bloat', + action='store_true', default=False, + help='Show changes in function code size for each board') + parser.add_option('-c', '--count', dest='count', type='int', + default=-1, help='Run build on the top n commits') + parser.add_option('-C', '--force-reconfig', dest='force_reconfig', + action='store_true', default=False, + help='Reconfigure for every commit (disable incremental build)') + parser.add_option('-d', '--detail', dest='show_detail', + action='store_true', default=False, + help='Show detailed information for each board in summary') + parser.add_option('-e', '--show_errors', action='store_true', + default=False, help='Show errors and warnings') + parser.add_option('-f', '--force-build', dest='force_build', + action='store_true', default=False, + help='Force build of boards even if already built') + parser.add_option('-F', '--force-build-failures', dest='force_build_failures', + action='store_true', default=False, + help='Force build of previously-failed build') + parser.add_option('-g', '--git', type='string', + help='Git repo containing branch to build', default='.') + parser.add_option('-G', '--config-file', type='string', + help='Path to buildman config file', default='') + parser.add_option('-H', '--full-help', action='store_true', dest='full_help', + default=False, help='Display the README file') + parser.add_option('-i', '--in-tree', dest='in_tree', + action='store_true', default=False, + help='Build in the source tree instead of a separate directory') + parser.add_option('-j', '--jobs', dest='jobs', type='int', + default=None, help='Number of jobs to run at once (passed to make)') + parser.add_option('-k', '--keep-outputs', action='store_true', + default=False, help='Keep all build output files (e.g. binaries)') + parser.add_option('-l', '--list-error-boards', action='store_true', + default=False, help='Show a list of boards next to each error/warning') + parser.add_option('--list-tool-chains', action='store_true', default=False, + help='List available tool chains') + parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run', + default=False, help="Do a try run (describe actions, but no nothing)") + parser.add_option('-o', '--output-dir', type='string', + dest='output_dir', default='..', + help='Directory where all builds happen and buildman has its workspace (default is ../)') + parser.add_option('-Q', '--quick', action='store_true', + default=False, help='Do a rough build, with limited warning resolution') + parser.add_option('-s', '--summary', action='store_true', + default=False, help='Show a build summary') + parser.add_option('-S', '--show-sizes', action='store_true', + default=False, help='Show image size variation in summary') + parser.add_option('--step', type='int', + default=1, help='Only build every n commits (0=just first and last)') + parser.add_option('-t', '--test', action='store_true', dest='test', + default=False, help='run tests') + parser.add_option('-T', '--threads', type='int', + default=None, help='Number of builder threads to use') + parser.add_option('-u', '--show_unknown', action='store_true', + default=False, help='Show boards with unknown build result') + parser.add_option('-v', '--verbose', action='store_true', + default=False, help='Show build results while the build progresses') + parser.add_option('-x', '--exclude', dest='exclude', + type='string', action='append', + help='Specify a list of boards to exclude, separated by comma') + + parser.usage += """ + + Build U-Boot for all commits in a branch. Use -n to do a dry run""" + + return parser.parse_args()

There is no good reason to keep this code separate. Move it into control.py so it is easier to test.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/buildman.py | 6 ------ tools/buildman/control.py | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index c4de857..70c2142 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -63,12 +63,6 @@ options, args = cmdline.ParseArgs() # Run our meagre tests if options.test: RunTests() -elif options.full_help: - pager = os.getenv('PAGER') - if not pager: - pager = 'more' - fname = os.path.join(os.path.dirname(sys.argv[0]), 'README') - command.Run(pager, fname)

# Build selected commits for selected boards else: diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 06c9229..408d9b1 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -84,6 +84,14 @@ def DoBuildman(options, args): options: Command line options object args: Command line arguments (list of strings) """ + if options.full_help: + pager = os.getenv('PAGER') + if not pager: + pager = 'more' + fname = os.path.join(os.path.dirname(sys.argv[0]), 'README') + command.Run(pager, fname) + return 0 + gitutil.Setup()

bsettings.Setup(options.config_file)

Add a test point for the command module. This allows tests to emulate the execution of commands. This provides more control (since we can make the fake 'commands' do whatever we like), makes it faster to write tests since we don't need to set up as much environment, and speeds up test execution.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/patman/command.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/tools/patman/command.py b/tools/patman/command.py index 7212fdf..d586f11 100644 --- a/tools/patman/command.py +++ b/tools/patman/command.py @@ -20,9 +20,25 @@ class CommandResult: def __init__(self): self.stdout = None self.stderr = None + self.combined = None self.return_code = None self.exception = None

+ def __init__(self, stdout='', stderr='', combined='', return_code=0, + exception=None): + self.stdout = stdout + self.stderr = stderr + self.combined = combined + self.return_code = return_code + self.exception = exception + + +# This permits interception of RunPipe for test purposes. If it is set to +# a function, then that function is called with the pipe list being +# executed. Otherwise, it is assumed to be a CommandResult object, and is +# returned as the result for every RunPipe() call. +# When this value is None, commands are executed as normal. +test_result = None

def RunPipe(pipe_list, infile=None, outfile=None, capture=False, capture_stderr=False, oneline=False, @@ -44,6 +60,10 @@ def RunPipe(pipe_list, infile=None, outfile=None, Returns: CommandResult object """ + if test_result: + if hasattr(test_result, '__call__'): + return test_result(pipe_list=pipe_list) + return test_result result = CommandResult() last_pipe = None pipeline = list(pipe_list)

Buildman currently lacks testing in many areas, including its use of git, make and many command-line flags.

Add a functional test which covers some of these areas. So far it does a fake 'build' of all boards for the current source tree.

This version reads the real ~/.buildman and boards.cfg files. Future work will improve this.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: - Add a comment to _HandleCommandGit - Make sure the test temporary directory is removed

tools/buildman/buildman.py | 17 ++--- tools/buildman/control.py | 21 +++-- tools/buildman/func_test.py | 182 ++++++++++++++++++++++++++++++++++++++++++++ tools/buildman/toolchain.py | 4 +- 4 files changed, 206 insertions(+), 18 deletions(-) create mode 100644 tools/buildman/func_test.py

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index 70c2142..6771c86 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -30,27 +30,20 @@ import terminal import toolchain

def RunTests(): + import func_test import test import doctest

result = unittest.TestResult() - for module in ['toolchain']: + for module in ['toolchain', 'gitutil']: 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.argv = [sys.argv[0]] - suite = unittest.TestLoader().loadTestsFromTestCase(test.TestBuild) - result = unittest.TestResult() - suite.run(result) + for module in (test.TestBuild, func_test.TestFunctional): + suite = unittest.TestLoader().loadTestsFromTestCase(module) + suite.run(result)

- # TODO: Surely we can just 'print' result? print result for test, err in result.errors: print err diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 408d9b1..213e235 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -13,6 +13,7 @@ from builder import Builder import gitutil import patchstream import terminal +from terminal import Print import toolchain import command import subprocess @@ -77,12 +78,18 @@ def ShowActions(series, why_selected, boards_selected, builder, options): print ('Total boards to build for each commit: %d\n' % why_selected['all'])

-def DoBuildman(options, args): +def DoBuildman(options, args, toolchains=None, make_func=None): """The main control code for buildman

Args: options: Command line options object args: Command line arguments (list of strings) + toolchains: Toolchains to use - this should be a Toolchains() + object. If None, then it will be created and scanned + make_func: Make function to use for the builder. This is called + to execute 'make'. If this is None, the normal function + will be used, which calls the 'make' tool with suitable + arguments. This setting is useful for tests. """ if options.full_help: pager = os.getenv('PAGER') @@ -97,8 +104,10 @@ def DoBuildman(options, args): bsettings.Setup(options.config_file) options.git_dir = os.path.join(options.git, '.git')

- toolchains = toolchain.Toolchains() - toolchains.Scan(options.list_tool_chains) + if not toolchains: + toolchains = toolchain.Toolchains() + toolchains.GetSettings() + toolchains.Scan(options.list_tool_chains) if options.list_tool_chains: toolchains.List() print @@ -202,6 +211,8 @@ def DoBuildman(options, args): options.threads, options.jobs, gnu_make=gnu_make, checkout=True, show_unknown=options.show_unknown, step=options.step) builder.force_config_on_failure = not options.quick + if make_func: + builder.do_make = make_func

# For a dry run, just show our actions as a sanity check if options.dry_run: @@ -220,8 +231,8 @@ def DoBuildman(options, args): else: commits = None

- print GetActionSummary(options.summary, commits, board_selected, - options) + Print(GetActionSummary(options.summary, commits, board_selected, + options))

builder.SetDisplayOptions(options.show_errors, options.show_sizes, options.show_detail, options.show_bloat, diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py new file mode 100644 index 0000000..8711f9c --- /dev/null +++ b/tools/buildman/func_test.py @@ -0,0 +1,182 @@ +# +# Copyright (c) 2014 Google, Inc +# +# SPDX-License-Identifier: GPL-2.0+ +# + +import os +import shutil +import sys +import tempfile +import unittest + +import cmdline +import command +import control +import gitutil +import terminal +import toolchain + +class TestFunctional(unittest.TestCase): + """Functional test for buildman. + + This aims to test from just below the invocation of buildman (parsing + of arguments) to 'make' and 'git' invocation. It is not a true + emd-to-end test, as it mocks git, make and the tool chain. But this + makes it easier to detect when the builder is doing the wrong thing, + since in many cases this test code will fail. For example, only a + very limited subset of 'git' arguments is supported - anything + unexpected will fail. + """ + def setUp(self): + self._base_dir = tempfile.mkdtemp() + self._git_dir = os.path.join(self._base_dir, 'src') + self._buildman_pathname = sys.argv[0] + self._buildman_dir = os.path.dirname(sys.argv[0]) + command.test_result = self._HandleCommand + self._toolchains = toolchain.Toolchains() + self._toolchains.Add('gcc', test=False) + + def tearDown(self): + shutil.rmtree(self._base_dir) + + def _RunBuildman(self, *args): + return command.RunPipe([[self._buildman_pathname] + list(args)], + capture=True, capture_stderr=True) + + def _RunControl(self, *args): + sys.argv = [sys.argv[0]] + list(args) + options, args = cmdline.ParseArgs() + return control.DoBuildman(options, args, toolchains=self._toolchains, + make_func=self._HandleMake) + + def testFullHelp(self): + command.test_result = None + result = self._RunBuildman('-H') + help_file = os.path.join(self._buildman_dir, 'README') + self.assertEqual(len(result.stdout), os.path.getsize(help_file)) + self.assertEqual(0, len(result.stderr)) + self.assertEqual(0, result.return_code) + + def testHelp(self): + command.test_result = None + result = self._RunBuildman('-h') + help_file = os.path.join(self._buildman_dir, 'README') + self.assertTrue(len(result.stdout) > 1000) + self.assertEqual(0, len(result.stderr)) + self.assertEqual(0, result.return_code) + + def testGitSetup(self): + """Test gitutils.Setup(), from outside the module itself""" + command.test_result = command.CommandResult(return_code=1) + gitutil.Setup() + self.assertEqual(gitutil.use_no_decorate, False) + + command.test_result = command.CommandResult(return_code=0) + gitutil.Setup() + self.assertEqual(gitutil.use_no_decorate, True) + + def _HandleCommandGitLog(self, args): + if '-n0' in args: + return command.CommandResult(return_code=0) + + # Not handled, so abort + print 'git log', args + sys.exit(1) + + def _HandleCommandGit(self, in_args): + """Handle execution of a git command + + This uses a hacked-up parser. + + Args: + in_args: Arguments after 'git' from the command line + """ + git_args = [] # Top-level arguments to git itself + sub_cmd = None # Git sub-command selected + args = [] # Arguments to the git sub-command + for arg in in_args: + if sub_cmd: + args.append(arg) + elif arg[0] == '-': + git_args.append(arg) + else: + sub_cmd = arg + if sub_cmd == 'config': + return command.CommandResult(return_code=0) + elif sub_cmd == 'log': + return self._HandleCommandGitLog(args) + + # Not handled, so abort + print 'git', git_args, sub_cmd, args + sys.exit(1) + + def _HandleCommandNm(self, args): + return command.CommandResult(return_code=0) + + def _HandleCommandObjdump(self, args): + return command.CommandResult(return_code=0) + + def _HandleCommandSize(self, args): + return command.CommandResult(return_code=0) + + def _HandleCommand(self, **kwargs): + """Handle a command execution. + + The command is in kwargs['pipe-list'], as a list of pipes, each a + list of commands. The command should be emulated as required for + testing purposes. + + Returns: + A CommandResult object + """ + pipe_list = kwargs['pipe_list'] + if len(pipe_list) != 1: + print 'invalid pipe', kwargs + sys.exit(1) + cmd = pipe_list[0][0] + args = pipe_list[0][1:] + if cmd == 'git': + return self._HandleCommandGit(args) + elif cmd == './scripts/show-gnu-make': + return command.CommandResult(return_code=0, stdout='make') + elif cmd == 'nm': + return self._HandleCommandNm(args) + elif cmd == 'objdump': + return self._HandleCommandObjdump(args) + elif cmd == 'size': + return self._HandleCommandSize(args) + + # Not handled, so abort + print 'unknown command', kwargs + sys.exit(1) + return command.CommandResult(return_code=0) + + def _HandleMake(self, commit, brd, stage, cwd, *args, **kwargs): + """Handle execution of 'make' + + Args: + commit: Commit object that is being built + brd: Board object that is being built + stage: Stage that we are at (mrproper, config, build) + cwd: Directory where make should be run + args: Arguments to pass to make + kwargs: Arguments to pass to command.RunPipe() + """ + if stage == 'mrproper': + return command.CommandResult(return_code=0) + elif stage == 'config': + return command.CommandResult(return_code=0, + combined='Test configuration complete') + elif stage == 'build': + return command.CommandResult(return_code=0) + + # Not handled, so abort + print 'make', stage + sys.exit(1) + + def testCurrentSource(self): + """Very simple test to invoke buildman on the current source""" + self._RunControl() + lines = terminal.GetPrintTestLines() + self.assertTrue(lines[0].text.startswith('Building current source')) diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index 0e91294..27dc318 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -99,6 +99,9 @@ class Toolchains: def __init__(self): self.toolchains = {} self.paths = [] + self._make_flags = dict(bsettings.GetItems('make-flags')) + + def GetSettings(self): toolchains = bsettings.GetItems('toolchain') if not toolchains: print ("Warning: No tool chains - please add a [toolchain] section" @@ -110,7 +113,6 @@ class Toolchains: self.paths += glob.glob(value) else: self.paths.append(value) - self._make_flags = dict(bsettings.GetItems('make-flags'))

def Add(self, fname, test=True, verbose=False): """Add a toolchain to our list

Move the bsettings code back to the main buildman.py file, so we can do something different when testing.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/buildman.py | 2 ++ tools/buildman/control.py | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index 6771c86..d0afeda 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -19,6 +19,7 @@ sys.path.append(os.path.join(our_path, '../patman'))

# Our modules import board +import bsettings import builder import checkpatch import cmdline @@ -59,5 +60,6 @@ if options.test:

# Build selected commits for selected boards else: + bsettings.Setup(options.config_file) ret_code = control.DoBuildman(options, args) sys.exit(ret_code) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 213e235..c8eeb6a 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -101,7 +101,6 @@ def DoBuildman(options, args, toolchains=None, make_func=None):

gitutil.Setup()

- bsettings.Setup(options.config_file) options.git_dir = os.path.join(options.git, '.git')

if not toolchains:

These files may not exist in the environment, or may not be suitable for testing. Provide our own config file and our own toolchains when running tests.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/bsettings.py | 15 ++++++++++----- tools/buildman/func_test.py | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py index 9164798..fdd875b 100644 --- a/tools/buildman/bsettings.py +++ b/tools/buildman/bsettings.py @@ -5,6 +5,7 @@

import ConfigParser import os +import StringIO

def Setup(fname=''): @@ -17,11 +18,15 @@ def Setup(fname=''): global config_fname

settings = ConfigParser.SafeConfigParser() - config_fname = fname - if config_fname == '': - config_fname = '%s/.buildman' % os.getenv('HOME') - if config_fname: - settings.read(config_fname) + if fname is not None: + config_fname = fname + if config_fname == '': + config_fname = '%s/.buildman' % os.getenv('HOME') + if config_fname: + settings.read(config_fname) + +def AddFile(data): + settings.readfp(StringIO.StringIO(data))

def GetItems(section): """Get the items from a section of the config. diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 8711f9c..b92cde3 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -10,6 +10,7 @@ import sys import tempfile import unittest

+import bsettings import cmdline import command import control @@ -17,6 +18,22 @@ import gitutil import terminal import toolchain

+settings_data = ''' +# Buildman settings file + +[toolchain] + +[toolchain-alias] + +[make-flags] +src=/home/sjg/c/src +chroot=/home/sjg/c/chroot +vboot=USE_STDINT=1 VBOOT_DEBUG=1 MAKEFLAGS_VBOOT=DEBUG=1 CFLAGS_EXTRA_VBOOT=-DUNROLL_LOOPS VBOOT_SOURCE=${src}/platform/vboot_reference +chromeos_coreboot=VBOOT=${chroot}/build/link/usr ${vboot} +chromeos_daisy=VBOOT=${chroot}/build/daisy/usr ${vboot} +chromeos_peach=VBOOT=${chroot}/build/peach_pit/usr ${vboot} +''' + class TestFunctional(unittest.TestCase): """Functional test for buildman.

@@ -36,6 +53,8 @@ class TestFunctional(unittest.TestCase): command.test_result = self._HandleCommand self._toolchains = toolchain.Toolchains() self._toolchains.Add('gcc', test=False) + bsettings.Setup(None) + bsettings.AddFile(settings_data)

def tearDown(self): shutil.rmtree(self._base_dir)

Rather than reading boards.cfg, which may take time to generate and is not necessarily suitable for running tests, create our own list of boards.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/control.py | 21 ++++++++++++--------- tools/buildman/func_test.py | 20 +++++++++++++++++++- 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py index c8eeb6a..fb15ae8 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -78,7 +78,7 @@ def ShowActions(series, why_selected, boards_selected, builder, options): print ('Total boards to build for each commit: %d\n' % why_selected['all'])

-def DoBuildman(options, args, toolchains=None, make_func=None): +def DoBuildman(options, args, toolchains=None, make_func=None, boards=None): """The main control code for buildman

Args: @@ -90,6 +90,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None): to execute 'make'. If this is None, the normal function will be used, which calls the 'make' tool with suitable arguments. This setting is useful for tests. + board: Boards() object to use, containing a list of available + boards. If this is None it will be created and scanned. """ if options.full_help: pager = os.getenv('PAGER') @@ -135,14 +137,15 @@ def DoBuildman(options, args, toolchains=None, make_func=None): sys.exit(col.Color(col.RED, str))

# Work out what subset of the boards we are building - board_file = os.path.join(options.git, 'boards.cfg') - status = subprocess.call([os.path.join(options.git, - 'tools/genboardscfg.py')]) - if status != 0: - sys.exit("Failed to generate boards.cfg") - - boards = board.Boards() - boards.ReadBoards(os.path.join(options.git, 'boards.cfg')) + if not boards: + board_file = os.path.join(options.git, 'boards.cfg') + status = subprocess.call([os.path.join(options.git, + 'tools/genboardscfg.py')]) + if status != 0: + sys.exit("Failed to generate boards.cfg") + + boards = board.Boards() + boards.ReadBoards(os.path.join(options.git, 'boards.cfg'))

exclude = [] if options.exclude: diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index b92cde3..237a80b 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -10,6 +10,7 @@ import sys import tempfile import unittest

+import board import bsettings import cmdline import command @@ -34,6 +35,14 @@ chromeos_daisy=VBOOT=${chroot}/build/daisy/usr ${vboot} chromeos_peach=VBOOT=${chroot}/build/peach_pit/usr ${vboot} '''

+boards = [ + ['Active', 'arm', 'armv7', '', 'Tester', 'ARM Board 1', 'board0', ''], + ['Active', 'arm', 'armv7', '', 'Tester', 'ARM Board 2', 'board1', ''], + ['Active', 'powerpc', 'powerpc', '', 'Tester', 'PowerPC board 1', 'board2', ''], + ['Active', 'powerpc', 'mpc5xx', '', 'Tester', 'PowerPC board 2', 'board3', ''], + ['Active', 'sandbox', 'sandbox', '', 'Tester', 'Sandbox board', 'board4', ''], +] + class TestFunctional(unittest.TestCase): """Functional test for buildman.

@@ -55,6 +64,9 @@ class TestFunctional(unittest.TestCase): self._toolchains.Add('gcc', test=False) bsettings.Setup(None) bsettings.AddFile(settings_data) + self._boards = board.Boards() + for brd in boards: + self._boards.AddBoard(board.Board(*brd))

def tearDown(self): shutil.rmtree(self._base_dir) @@ -67,7 +79,7 @@ class TestFunctional(unittest.TestCase): sys.argv = [sys.argv[0]] + list(args) options, args = cmdline.ParseArgs() return control.DoBuildman(options, args, toolchains=self._toolchains, - make_func=self._HandleMake) + make_func=self._HandleMake, boards=self._boards)

def testFullHelp(self): command.test_result = None @@ -194,6 +206,12 @@ class TestFunctional(unittest.TestCase): print 'make', stage sys.exit(1)

+ def testNoBoards(self): + """Test that buildman aborts when there are no boards""" + self._boards = board.Boards() + with self.assertRaises(SystemExit): + self._RunControl() + def testCurrentSource(self): """Very simple test to invoke buildman on the current source""" self._RunControl()

When a build is to be performed, buildman checks to see if it has already been done. In most cases it will not bother trying again. However, it was not reading the return code from the 'done' file, so if the result was a failure, it would not be counted. This depresses the 'failure' count stats that buildman prints in this case.

Fix this bug by always reading the return code.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/builderthread.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 0246375..261919f 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -138,16 +138,17 @@ class BuilderThread(threading.Thread): result.already_done = os.path.exists(done_file) will_build = (force_build or force_build_failures or not result.already_done) - if result.already_done and will_build: + if result.already_done: # Get the return code from that build and use it with open(done_file, 'r') as fd: result.return_code = int(fd.readline()) - err_file = self.builder.GetErrFile(commit_upto, brd.target) - if os.path.exists(err_file) and os.stat(err_file).st_size: - result.stderr = 'bad' - elif not force_build: - # The build passed, so no need to build it again - will_build = False + if will_build: + err_file = self.builder.GetErrFile(commit_upto, brd.target) + if os.path.exists(err_file) and os.stat(err_file).st_size: + result.stderr = 'bad' + elif not force_build: + # The build passed, so no need to build it again + will_build = False

if will_build: # We are going to have to build it. First, get a toolchain

For testing it is useful to clean the output directory before running a test. This avoids a test interfering with the results of a subsequent test by leaving data around.

Add this feature as an optional parameter to the control logic.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/control.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py index fb15ae8..2f51249 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -5,6 +5,7 @@

import multiprocessing import os +import shutil import sys

import board @@ -78,7 +79,8 @@ def ShowActions(series, why_selected, boards_selected, builder, options): print ('Total boards to build for each commit: %d\n' % why_selected['all'])

-def DoBuildman(options, args, toolchains=None, make_func=None, boards=None): +def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, + clean_dir=False): """The main control code for buildman

Args: @@ -93,6 +95,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None): board: Boards() object to use, containing a list of available boards. If this is None it will be created and scanned. """ + global builder + if options.full_help: pager = os.getenv('PAGER') if not pager: @@ -209,6 +213,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None): else: dirname = 'current' output_dir = os.path.join(options.output_dir, dirname) + if clean_dir: + shutil.rmtree(output_dir) builder = Builder(toolchains, output_dir, options.git_dir, options.threads, options.jobs, gnu_make=gnu_make, checkout=True, show_unknown=options.show_unknown, step=options.step) @@ -230,6 +236,9 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):

if series: commits = series.commits + # Number the commits for test purposes + for commit in range(len(commits)): + commits[commit].sequence = commit else: commits = None

For reasons that are not well-understood, GetMetaDataForList() can end up adding to an existing series even when it appears that it should be starting a new one.

Change from using a default constructor parameter to an explicit one, to work around this problem.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/patman/patchstream.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index b0b8153..b3e66c3 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -355,7 +355,7 @@ class PatchStream:

def GetMetaDataForList(commit_range, git_dir=None, count=None, - series = Series()): + series = None): """Reads out patch series metadata from the commits

This does a 'git log' on the relevant commits and pulls out the tags we @@ -370,6 +370,8 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None, Returns: A Series object containing information about the commits. """ + if not series: + series = Series() params = gitutil.LogCmd(commit_range,reverse=True, count=count, git_dir=git_dir) stdout = command.RunPipe([params], capture=True).stdout

This adds coverage of core features of the builder, including the command-line options which affect building.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: None

tools/buildman/func_test.py | 324 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 306 insertions(+), 18 deletions(-)

diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 237a80b..2cb5cf0 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -43,6 +43,125 @@ boards = [ ['Active', 'sandbox', 'sandbox', '', 'Tester', 'Sandbox board', 'board4', ''], ]

+commit_shortlog = """4aca821 patman: Avoid changing the order of tags +39403bb patman: Use --no-pager' to stop git from forking a pager +db6e6f2 patman: Remove the -a option +f2ccf03 patman: Correct unit tests to run correctly +1d097f9 patman: Fix indentation in terminal.py +d073747 patman: Support the 'reverse' option for 'git log +""" + +commit_log = ["""commit 7f6b8315d18f683c5181d0c3694818c1b2a20dcd +Author: Masahiro Yamada yamada.m@jp.panasonic.com +Date: Fri Aug 22 19:12:41 2014 +0900 + + buildman: refactor help message + + "buildman [options]" is displayed by default. + + Append the rest of help messages to parser.usage + instead of replacing it. + + Besides, "-b <branch>" is not mandatory since commit fea5858e. + Drop it from the usage. + + Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com +""", +"""commit d0737479be6baf4db5e2cdbee123e96bc5ed0ba8 +Author: Simon Glass sjg@chromium.org +Date: Thu Aug 14 16:48:25 2014 -0600 + + patman: Support the 'reverse' option for 'git log' + + This option is currently not supported, but needs to be, for buildman to + operate as expected. + + Series-changes: 7 + - Add new patch to fix the 'reverse' bug + + + Change-Id: I79078f792e8b390b8a1272a8023537821d45feda + Reported-by: York Sun yorksun@freescale.com + Signed-off-by: Simon Glass sjg@chromium.org + +""", +"""commit 1d097f9ab487c5019152fd47bda126839f3bf9fc +Author: Simon Glass sjg@chromium.org +Date: Sat Aug 9 11:44:32 2014 -0600 + + patman: Fix indentation in terminal.py + + This code came from a different project with 2-character indentation. Fix + it for U-Boot. + + Series-changes: 6 + - Add new patch to fix indentation in teminal.py + + Change-Id: I5a74d2ebbb3cc12a665f5c725064009ac96e8a34 + Signed-off-by: Simon Glass sjg@chromium.org + +""", +"""commit f2ccf03869d1e152c836515a3ceb83cdfe04a105 +Author: Simon Glass sjg@chromium.org +Date: Sat Aug 9 11:08:24 2014 -0600 + + patman: Correct unit tests to run correctly + + It seems that doctest behaves differently now, and some of the unit tests + do not run. Adjust the tests to work correctly. + + ./tools/patman/patman --test + <unittest.result.TestResult run=10 errors=0 failures=0> + + Series-changes: 6 + - Add new patch to fix patman unit tests + + Change-Id: I3d2ca588f4933e1f9d6b1665a00e4ae58269ff3b + +""", +"""commit db6e6f2f9331c5a37647d6668768d4a40b8b0d1c +Author: Simon Glass sjg@chromium.org +Date: Sat Aug 9 12:06:02 2014 -0600 + + patman: Remove the -a option + + It seems that this is no longer needed, since checkpatch.pl will catch + whitespace problems in patches. Also the option is not widely used, so + it seems safe to just remove it. + + Series-changes: 6 + - Add new patch to remove patman's -a option + + Suggested-by: Masahiro Yamada yamada.m@jp.panasonic.com + Change-Id: I5821a1c75154e532c46513486ca40b808de7e2cc + +""", +"""commit 39403bb4f838153028a6f21ca30bf100f3791133 +Author: Simon Glass sjg@chromium.org +Date: Thu Aug 14 21:50:52 2014 -0600 + + patman: Use --no-pager' to stop git from forking a pager + +""", +"""commit 4aca821e27e97925c039e69fd37375b09c6f129c +Author: Simon Glass sjg@chromium.org +Date: Fri Aug 22 15:57:39 2014 -0600 + + patman: Avoid changing the order of tags + + patman collects tags that it sees in the commit and places them nicely + sorted at the end of the patch. However, this is not really necessary and + in fact is apparently not desirable. + + Series-changes: 9 + - Add new patch to avoid changing the order of tags + + Suggested-by: Masahiro Yamada yamada.m@jp.panasonic.com + Change-Id: Ib1518588c1a189ad5c3198aae76f8654aed8d0db +"""] + +TEST_BRANCH = '__testbranch' + class TestFunctional(unittest.TestCase): """Functional test for buildman.

@@ -60,26 +179,49 @@ class TestFunctional(unittest.TestCase): self._buildman_pathname = sys.argv[0] self._buildman_dir = os.path.dirname(sys.argv[0]) command.test_result = self._HandleCommand - self._toolchains = toolchain.Toolchains() - self._toolchains.Add('gcc', test=False) + self.setupToolchains() + self._toolchains.Add('arm-gcc', test=False) + self._toolchains.Add('powerpc-gcc', test=False) bsettings.Setup(None) bsettings.AddFile(settings_data) self._boards = board.Boards() for brd in boards: self._boards.AddBoard(board.Board(*brd))

+ # Directories where the source been cloned + self._clone_dirs = [] + self._commits = len(commit_shortlog.splitlines()) + 1 + self._total_builds = self._commits * len(boards) + + # Number of calls to make + self._make_calls = 0 + + # Map of [board, commit] to error messages + self._error = {} + + # Avoid sending any output and clear all terminal output + terminal.SetPrintTestMode() + terminal.GetPrintTestLines() + def tearDown(self): shutil.rmtree(self._base_dir)

+ def setupToolchains(self): + self._toolchains = toolchain.Toolchains() + self._toolchains.Add('gcc', test=False) + def _RunBuildman(self, *args): return command.RunPipe([[self._buildman_pathname] + list(args)], capture=True, capture_stderr=True)

- def _RunControl(self, *args): + def _RunControl(self, *args, **kwargs): sys.argv = [sys.argv[0]] + list(args) options, args = cmdline.ParseArgs() - return control.DoBuildman(options, args, toolchains=self._toolchains, - make_func=self._HandleMake, boards=self._boards) + result = control.DoBuildman(options, args, toolchains=self._toolchains, + make_func=self._HandleMake, boards=self._boards, + clean_dir=kwargs.get('clean_dir', True)) + self._builder = control.builder + return result

def testFullHelp(self): command.test_result = None @@ -110,11 +252,34 @@ class TestFunctional(unittest.TestCase): def _HandleCommandGitLog(self, args): if '-n0' in args: return command.CommandResult(return_code=0) + elif args[-1] == 'upstream/master..%s' % TEST_BRANCH: + return command.CommandResult(return_code=0, stdout=commit_shortlog) + elif args[:3] == ['--no-color', '--no-decorate', '--reverse']: + if args[-1] == TEST_BRANCH: + count = int(args[3][2:]) + return command.CommandResult(return_code=0, + stdout=''.join(commit_log[:count]))

# Not handled, so abort print 'git log', args sys.exit(1)

+ def _HandleCommandGitConfig(self, args): + config = args[0] + if config == 'sendemail.aliasesfile': + return command.CommandResult(return_code=0) + elif config.startswith('branch.badbranch'): + return command.CommandResult(return_code=1) + elif config == 'branch.%s.remote' % TEST_BRANCH: + return command.CommandResult(return_code=0, stdout='upstream\n') + elif config == 'branch.%s.merge' % TEST_BRANCH: + return command.CommandResult(return_code=0, + stdout='refs/heads/master\n') + + # Not handled, so abort + print 'git config', args + sys.exit(1) + def _HandleCommandGit(self, in_args): """Handle execution of a git command

@@ -132,11 +297,18 @@ class TestFunctional(unittest.TestCase): elif arg[0] == '-': git_args.append(arg) else: - sub_cmd = arg + if git_args and git_args[-1] in ['--git-dir', '--work-tree']: + git_args.append(arg) + else: + sub_cmd = arg if sub_cmd == 'config': - return command.CommandResult(return_code=0) + return self._HandleCommandGitConfig(args) elif sub_cmd == 'log': return self._HandleCommandGitLog(args) + elif sub_cmd == 'clone': + return command.CommandResult(return_code=0) + elif sub_cmd == 'checkout': + return command.CommandResult(return_code=0)

# Not handled, so abort print 'git', git_args, sub_cmd, args @@ -162,26 +334,35 @@ class TestFunctional(unittest.TestCase): A CommandResult object """ pipe_list = kwargs['pipe_list'] + wc = False if len(pipe_list) != 1: - print 'invalid pipe', kwargs - sys.exit(1) + if pipe_list[1] == ['wc', '-l']: + wc = True + else: + print 'invalid pipe', kwargs + sys.exit(1) cmd = pipe_list[0][0] args = pipe_list[0][1:] + result = None if cmd == 'git': - return self._HandleCommandGit(args) + result = self._HandleCommandGit(args) elif cmd == './scripts/show-gnu-make': return command.CommandResult(return_code=0, stdout='make') - elif cmd == 'nm': + elif cmd.endswith('nm'): return self._HandleCommandNm(args) - elif cmd == 'objdump': + elif cmd.endswith('objdump'): return self._HandleCommandObjdump(args) - elif cmd == 'size': + elif cmd.endswith( 'size'): return self._HandleCommandSize(args)

- # Not handled, so abort - print 'unknown command', kwargs - sys.exit(1) - return command.CommandResult(return_code=0) + if not result: + # Not handled, so abort + print 'unknown command', kwargs + sys.exit(1) + + if wc: + result.stdout = len(result.stdout.splitlines()) + return result

def _HandleMake(self, commit, brd, stage, cwd, *args, **kwargs): """Handle execution of 'make' @@ -194,18 +375,31 @@ class TestFunctional(unittest.TestCase): args: Arguments to pass to make kwargs: Arguments to pass to command.RunPipe() """ + self._make_calls += 1 if stage == 'mrproper': return command.CommandResult(return_code=0) elif stage == 'config': return command.CommandResult(return_code=0, combined='Test configuration complete') elif stage == 'build': + stderr = '' + if type(commit) is not str: + stderr = self._error.get((brd.target, commit.sequence)) + if stderr: + return command.CommandResult(return_code=1, stderr=stderr) return command.CommandResult(return_code=0)

# Not handled, so abort print 'make', stage sys.exit(1)

+ # Example function to print output lines + def print_lines(self, lines): + print len(lines) + for line in lines: + print line + #self.print_lines(terminal.GetPrintTestLines()) + def testNoBoards(self): """Test that buildman aborts when there are no boards""" self._boards = board.Boards() @@ -214,6 +408,100 @@ class TestFunctional(unittest.TestCase):

def testCurrentSource(self): """Very simple test to invoke buildman on the current source""" + self.setupToolchains(); self._RunControl() lines = terminal.GetPrintTestLines() - self.assertTrue(lines[0].text.startswith('Building current source')) + self.assertIn('Building current source for %d boards' % len(boards), + lines[0].text) + + def testBadBranch(self): + """Test that we can detect an invalid branch""" + with self.assertRaises(ValueError): + self._RunControl('-b', 'badbranch') + + def testBadToolchain(self): + """Test that missing toolchains are detected""" + self.setupToolchains(); + ret_code = self._RunControl('-b', TEST_BRANCH) + lines = terminal.GetPrintTestLines() + + # Buildman always builds the upstream commit as well + self.assertIn('Building %d commits for %d boards' % + (self._commits, len(boards)), lines[0].text) + self.assertEqual(self._builder.count, self._total_builds) + + # Only sandbox should succeed, the others don't have toolchains + self.assertEqual(self._builder.fail, + self._total_builds - self._commits) + self.assertEqual(ret_code, 128) + + for commit in range(self._commits): + for board in self._boards.GetList(): + if board.arch != 'sandbox': + errfile = self._builder.GetErrFile(commit, board.target) + fd = open(errfile) + self.assertEqual(fd.readlines(), + ['No tool chain for %s\n' % board.arch]) + fd.close() + + def testBranch(self): + """Test building a branch with all toolchains present""" + self._RunControl('-b', TEST_BRANCH) + self.assertEqual(self._builder.count, self._total_builds) + self.assertEqual(self._builder.fail, 0) + + def testCount(self): + """Test building a specific number of commitst""" + self._RunControl('-b', TEST_BRANCH, '-c2') + self.assertEqual(self._builder.count, 2 * len(boards)) + self.assertEqual(self._builder.fail, 0) + # Each board has a mrproper, config, and then one make per commit + self.assertEqual(self._make_calls, len(boards) * (2 + 2)) + + def testIncremental(self): + """Test building a branch twice - the second time should do nothing""" + self._RunControl('-b', TEST_BRANCH) + + # Each board has a mrproper, config, and then one make per commit + self.assertEqual(self._make_calls, len(boards) * (self._commits + 2)) + self._make_calls = 0 + self._RunControl('-b', TEST_BRANCH, clean_dir=False) + self.assertEqual(self._make_calls, 0) + self.assertEqual(self._builder.count, self._total_builds) + self.assertEqual(self._builder.fail, 0) + + def testForceBuild(self): + """The -f flag should force a rebuild""" + self._RunControl('-b', TEST_BRANCH) + self._make_calls = 0 + self._RunControl('-b', TEST_BRANCH, '-f', clean_dir=False) + # Each board has a mrproper, config, and then one make per commit + self.assertEqual(self._make_calls, len(boards) * (self._commits + 2)) + + def testForceReconfigure(self): + """The -f flag should force a rebuild""" + self._RunControl('-b', TEST_BRANCH, '-C') + # Each commit has a mrproper, config and make + self.assertEqual(self._make_calls, len(boards) * self._commits * 3) + + def testErrors(self): + """Test handling of build errors""" + self._error['board2', 1] = 'fred\n' + self._RunControl('-b', TEST_BRANCH) + self.assertEqual(self._builder.count, self._total_builds) + self.assertEqual(self._builder.fail, 1) + + # Remove the error. This should have no effect since the commit will + # not be rebuilt + del self._error['board2', 1] + self._make_calls = 0 + self._RunControl('-b', TEST_BRANCH, clean_dir=False) + self.assertEqual(self._builder.count, self._total_builds) + self.assertEqual(self._make_calls, 0) + self.assertEqual(self._builder.fail, 1) + + # Now use the -F flag to force rebuild of the bad commit + self._RunControl('-b', TEST_BRANCH, '-F', clean_dir=False) + self.assertEqual(self._builder.count, self._total_builds) + self.assertEqual(self._builder.fail, 0) + self.assertEqual(self._make_calls, 3)

Now that buildman supports removing the build directory prefix from output, add a test for it. Also ensure that output directories are removed when the test completes.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: None Changes in v2: - Add patch to expand output test to cover directory prefixes

tools/buildman/test.py | 54 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/test.py b/tools/buildman/test.py index f0c4d0e..a2a85ac 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -46,6 +46,20 @@ powerpc-linux-ld: u-boot: section .reloc lma 0xffffa400 overlaps previous sectio powerpc-linux-ld: u-boot: section .data lma 0xffffcd38 overlaps previous sections powerpc-linux-ld: u-boot: section .u_boot_cmd lma 0xffffeb40 overlaps previous sections powerpc-linux-ld: u-boot: section .bootpg lma 0xfffff198 overlaps previous sections +''', + '''In file included from %(basedir)sarch/sandbox/cpu/cpu.c:9:0: +%(basedir)sarch/sandbox/include/asm/state.h:44:0: warning: "xxxx" redefined [enabled by default] +%(basedir)sarch/sandbox/include/asm/state.h:43:0: note: this is the location of the previous definition +%(basedir)sarch/sandbox/cpu/cpu.c: In function 'do_reset': +%(basedir)sarch/sandbox/cpu/cpu.c:27:1: error: unknown type name 'blah' +%(basedir)sarch/sandbox/cpu/cpu.c:28:12: error: expected declaration specifiers or '...' before numeric constant +make[2]: *** [arch/sandbox/cpu/cpu.o] Error 1 +make[1]: *** [arch/sandbox/cpu] Error 2 +make[1]: *** Waiting for unfinished jobs.... +In file included from %(basedir)scommon/board_f.c:55:0: +%(basedir)sarch/sandbox/include/asm/state.h:44:0: warning: "xxxx" redefined [enabled by default] +%(basedir)sarch/sandbox/include/asm/state.h:43:0: note: this is the location of the previous definition +make: *** [sub-make] Error 2 ''' ]

@@ -57,7 +71,8 @@ commits = [ ['9012', 'Third commit, error', 1, errors[0:2]], ['3456', 'Fourth commit, warning', 0, [errors[0], errors[2]]], ['7890', 'Fifth commit, link errors', 1, [errors[0], errors[3]]], - ['abcd', 'Sixth commit, fixes all errors', 0, []] + ['abcd', 'Sixth commit, fixes all errors', 0, []], + ['ef01', 'Seventh commit, check directory suppression', 1, [errors[4]]], ]

boards = [ @@ -109,15 +124,19 @@ class TestBuild(unittest.TestCase): self._col = terminal.Color()

def Make(self, commit, brd, stage, *args, **kwargs): + global base_dir + result = command.CommandResult() boardnum = int(brd.target[-1]) result.return_code = 0 result.stderr = '' result.stdout = ('This is the test output for board %s, commit %s' % (brd.target, commit.hash)) - if boardnum >= 1 and boardnum >= commit.sequence: + if ((boardnum >= 1 and boardnum >= commit.sequence) or + boardnum == 4 and commit.sequence == 6): result.return_code = commit.return_code - result.stderr = ''.join(commit.error_list) + result.stderr = (''.join(commit.error_list) + % {'basedir' : base_dir + '/.bm-work/00/'}) if stage == 'build': target_dir = None for arg in args: @@ -146,10 +165,12 @@ class TestBuild(unittest.TestCase):

This does a line-by-line verification of the summary output. """ - output_dir = tempfile.mkdtemp() - if not os.path.isdir(output_dir): - os.mkdir(output_dir) - build = builder.Builder(self.toolchains, output_dir, None, 1, 2, + global base_dir + + base_dir = tempfile.mkdtemp() + if not os.path.isdir(base_dir): + os.mkdir(base_dir) + build = builder.Builder(self.toolchains, base_dir, None, 1, 2, checkout=False, show_unknown=False) build.do_make = self.Make board_selected = self.boards.GetSelectedDict() @@ -167,6 +188,7 @@ class TestBuild(unittest.TestCase): self.assertEqual(count, len(commits) * len(boards) + 1) build.SetDisplayOptions(show_errors=True); build.ShowSummary(self.commits, board_selected) + #terminal.EchoPrintTestLines() lines = terminal.GetPrintTestLines() self.assertEqual(lines[0].text, '01: %s' % commits[0][1]) self.assertEqual(lines[1].text, '02: %s' % commits[1][1]) @@ -230,7 +252,22 @@ class TestBuild(unittest.TestCase): self.assertEqual(lines[24].text, 'w-%s' % errors[0].rstrip().replace('\n', '\nw-'))

- self.assertEqual(len(lines), 25) + self.assertEqual(lines[25].text, '07: %s' % commits[6][1]) + self.assertSummary(lines[26].text, 'sandbox', '+', ['board4']) + + # Pick out the correct error lines + expect_str = errors[4].rstrip().replace('%(basedir)s', '').split('\n') + expect = expect_str[3:8] + [expect_str[-1]] + self.assertEqual(lines[27].text, '+%s' % + '\n'.join(expect).replace('\n', '\n+')) + + # Now the warnings lines + expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]] + self.assertEqual(lines[28].text, 'w+%s' % + '\n'.join(expect).replace('\n', '\nw+')) + + self.assertEqual(len(lines), 29) + shutil.rmtree(base_dir)

def _testGit(self): """Test basic builder operation by building a branch""" @@ -255,6 +292,7 @@ class TestBuild(unittest.TestCase): options.keep_outputs = False args = ['tegra20'] control.DoBuildman(options, args) + shutil.rmtree(base_dir)

def testBoardSingle(self): """Test single board selection"""

At present buildman naively uses the branch name as part of its directory path, which causes problems if the name has an embedded '/'.

Replace these with '_' to fix the problem.

Reported-by: Steve Rae srae@broadcom.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Add new patch to permit branch names with an embedded '/'

Changes in v2: None

tools/buildman/control.py | 2 +- tools/buildman/func_test.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 2f51249..06ca606 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -209,7 +209,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,

# Create a new builder with the selected options if options.branch: - dirname = options.branch + dirname = options.branch.replace('/', '_') else: dirname = 'current' output_dir = os.path.join(options.output_dir, dirname) diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 2cb5cf0..c37f1b6 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -199,6 +199,8 @@ class TestFunctional(unittest.TestCase): # Map of [board, commit] to error messages self._error = {}

+ self._test_branch = TEST_BRANCH + # Avoid sending any output and clear all terminal output terminal.SetPrintTestMode() terminal.GetPrintTestLines() @@ -252,10 +254,10 @@ class TestFunctional(unittest.TestCase): def _HandleCommandGitLog(self, args): if '-n0' in args: return command.CommandResult(return_code=0) - elif args[-1] == 'upstream/master..%s' % TEST_BRANCH: + elif args[-1] == 'upstream/master..%s' % self._test_branch: return command.CommandResult(return_code=0, stdout=commit_shortlog) elif args[:3] == ['--no-color', '--no-decorate', '--reverse']: - if args[-1] == TEST_BRANCH: + if args[-1] == self._test_branch: count = int(args[3][2:]) return command.CommandResult(return_code=0, stdout=''.join(commit_log[:count])) @@ -270,9 +272,9 @@ class TestFunctional(unittest.TestCase): return command.CommandResult(return_code=0) elif config.startswith('branch.badbranch'): return command.CommandResult(return_code=1) - elif config == 'branch.%s.remote' % TEST_BRANCH: + elif config == 'branch.%s.remote' % self._test_branch: return command.CommandResult(return_code=0, stdout='upstream\n') - elif config == 'branch.%s.merge' % TEST_BRANCH: + elif config == 'branch.%s.merge' % self._test_branch: return command.CommandResult(return_code=0, stdout='refs/heads/master\n')

@@ -505,3 +507,10 @@ class TestFunctional(unittest.TestCase): self.assertEqual(self._builder.count, self._total_builds) self.assertEqual(self._builder.fail, 0) self.assertEqual(self._make_calls, 3) + + def testBranchWithSlash(self): + """Test building a branch with a '/' in the name""" + self._test_branch = '/__dev/__testbranch' + self._RunControl('-b', self._test_branch, clean_dir=False) + self.assertEqual(self._builder.count, self._total_builds) + self.assertEqual(self._builder.fail, 0)

Tags like Series-version are normally expected to appear once, and with a unique value. But buildman doesn't actually look at these tags. So ignore conflicts.

This allows bulidman to build a branch containing multiple patman series.

Reported-by: Steve Rae srae@broadcom.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v3: - Add new patch to ignore conflicting tags in buildman

Changes in v2: None

tools/buildman/control.py | 15 +++++++-------- tools/buildman/func_test.py | 3 +++ tools/patman/patchstream.py | 4 +++- 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 06ca606..a2259c3 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -166,6 +166,10 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, # upstream/master~..branch but that isn't possible if upstream/master is # a merge commit (it will list all the commits that form part of the # merge) + # Conflicting tags are not a problem for buildman, since it does not use + # them. For example, Series-version is not useful for buildman. On the + # other hand conflicting tags will cause an error. So allow later tags + # to overwrite earlier ones by setting allow_overwrite=True if options.branch: if count == -1: range_expr = gitutil.GetRangeInBranch(options.git_dir, @@ -173,19 +177,14 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, upstream_commit = gitutil.GetUpstream(options.git_dir, options.branch) series = patchstream.GetMetaDataForList(upstream_commit, - options.git_dir, 1) + options.git_dir, 1, series=None, allow_overwrite=True)

- # Conflicting tags are not a problem for buildman, since it does - # not use them. For example, Series-version is not useful for - # buildman. On the other hand conflicting tags will cause an - # error. So allow later tags to overwrite earlier ones. - series.allow_overwrite = True series = patchstream.GetMetaDataForList(range_expr, - options.git_dir, None, series) + options.git_dir, None, series, allow_overwrite=True) else: # Honour the count series = patchstream.GetMetaDataForList(options.branch, - options.git_dir, count) + options.git_dir, count, series=None, allow_overwrite=True) else: series = None options.verbose = True diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index c37f1b6..75eb3a9 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -79,6 +79,7 @@ Date: Thu Aug 14 16:48:25 2014 -0600 Series-changes: 7 - Add new patch to fix the 'reverse' bug

+ Series-version: 8

Change-Id: I79078f792e8b390b8a1272a8023537821d45feda Reported-by: York Sun yorksun@freescale.com @@ -156,6 +157,8 @@ Date: Fri Aug 22 15:57:39 2014 -0600 Series-changes: 9 - Add new patch to avoid changing the order of tags

+ Series-version: 9 + Suggested-by: Masahiro Yamada yamada.m@jp.panasonic.com Change-Id: Ib1518588c1a189ad5c3198aae76f8654aed8d0db """] diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index b3e66c3..d630157 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -355,7 +355,7 @@ class PatchStream:

def GetMetaDataForList(commit_range, git_dir=None, count=None, - series = None): + series = None, allow_overwrite=False): """Reads out patch series metadata from the commits

This does a 'git log' on the relevant commits and pulls out the tags we @@ -367,11 +367,13 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None, count: Number of commits to list, or None for no limit series: Series object to add information into. By default a new series is started. + allow_overwrite: Allow tags to overwrite an existing tag Returns: A Series object containing information about the commits. """ if not series: series = Series() + series.allow_overwrite = allow_overwrite params = gitutil.LogCmd(commit_range,reverse=True, count=count, git_dir=git_dir) stdout = command.RunPipe([params], capture=True).stdout