Add some missing buildman features and deprecate MAKEALL
v8 cover Add some missing buildman features and deprecate MAKEALL
Buildman has been around for a little over a year and is used by a fair number of U-Boot developers. However quite a few people still use MAKEALL.
Buildman was intended to replace MAKEALL, so perhaps now is a good time to start that process.
The reasons to deprecate MAKEALL are:
- We don't want to maintain two build systems
- Buildman is typically faster
- Buildman has a lot more features
This series adds a few features to buildman to fill some gaps, adds some information into the README on how to migrate from MAKEALL, and adds a deprecation message to MAKEALL.
Changes in v8:
- Add new patch to disable the pager in git
Changes in v7:
- Add new patch to fix the 'reverse' bug
- Remove already-applied patches from the series
- Add the deprecation message at the end of the build also
- Drop the 'colour' patch sadly
Changes in v6:
- Add new patch to fix indentation in teminal.py
- Add new patch to fix patman unit tests
- Add new patch to remove patman's -a option
Changes in v5:
- Drop patch to search for *cc instead of *gcc for the compiler
Simon Glass (6):
patman: Support the 'reverse' option for 'git log'
patman: Fix indentation in terminal.py
patman: Correct unit tests to run correctly
patman: Remove the -a option
patman: Use --no-pager' to stop git from forking a pager
RFC: Deprecate MAKEALL
MAKEALL | 10 ++++ tools/patman/gitutil.py | 100 +++------------------------------------ tools/patman/patchstream.py | 7 ++- tools/patman/patman.py | 7 --- tools/patman/terminal.py | 112 +++++++++++++++++++++++--------------------- tools/patman/test.py | 13 ++--- 6 files changed, 88 insertions(+), 161 deletions(-)
Signature · 1 line
2.1.0.rc2.206.gedb03e5
PATCH v8 1/6 patman: Support the 'reverse' option for 'git log'
This option is currently not supported, but needs to be, for buildman to operate as expected.
---
Changes in v8: None
Changes in v7:
- Add new patch to fix the 'reverse' bug
Changes in v6: None
Changes in v5: None
tools/patman/gitutil.py | 2 ++ 1 file changed, 2 insertions(+)
tools/patman/gitutil.py +2 -0
@@ -38,6 +38,8 @@ def LogCmd(commit_range, git_dir=None, oneline=False, reverse=False, cmd.append('--oneline') if use_no_decorate: cmd.append('--no-decorate') + if reverse: + cmd.append('--reverse') if count is not None: cmd.append('-n%d' % count) if commit_range:
Signature · 1 line
2.1.0.rc2.206.gedb03e5
PATCH v8 2/6 patman: Fix indentation in terminal.py
In reply to Simon GlassThis code came from a different project with 2-character indentation. Fix it for U-Boot.
---
Changes in v8: None
Changes in v7: None
Changes in v6:
- Add new patch to fix indentation in teminal.py
Changes in v5: None
tools/patman/terminal.py | 108 ++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 53 deletions(-)
tools/patman/terminal.py +55 -53
@@ -15,66 +15,68 @@ import sys COLOR_IF_TERMINAL, COLOR_ALWAYS, COLOR_NEVER = range(3) class Color(object): - """Conditionally wraps text in ANSI color escape sequences.""" - BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8) - BOLD = -1 - BRIGHT_START = '\033[1;%dm' - NORMAL_START = '\033[22;%dm' - BOLD_START = '\033[1m' - RESET = '\033[0m' + """Conditionally wraps text in ANSI color escape sequences.""" + BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8) + BOLD = -1 + BRIGHT_START = '\033[1;%dm' + NORMAL_START = '\033[22;%dm' + BOLD_START = '\033[1m' + RESET = '\033[0m' - def __init__(self, colored=COLOR_IF_TERMINAL): - """Create a new Color object, optionally disabling color output. + def __init__(self, colored=COLOR_IF_TERMINAL): + """Create a new Color object, optionally disabling color output. - Args: - enabled: True if color output should be enabled. If False then this - class will not add color codes at all. - """ - self._enabled = (colored == COLOR_ALWAYS or - (colored == COLOR_IF_TERMINAL and os.isatty(sys.stdout.fileno()))) + Args: + enabled: True if color output should be enabled. If False then this + class will not add color codes at all. + """ + self._enabled = (colored == COLOR_ALWAYS or + (colored == COLOR_IF_TERMINAL and os.isatty(sys.stdout.fileno()))) - def Start(self, color, bright=True): - """Returns a start color code. + def Start(self, color, bright=True): + """Returns a start color code. - Args: - color: Color to use, .e.g BLACK, RED, etc. + Args: + color: Color to use, .e.g BLACK, RED, etc. - Returns: - If color is enabled, returns an ANSI sequence to start the given color, - otherwise returns empty string - """ - if self._enabled: - base = self.BRIGHT_START if bright else self.NORMAL_START - return base % (color + 30) - return '' + Returns: + If color is enabled, returns an ANSI sequence to start the given + color, otherwise returns empty string + """ + if self._enabled: + base = self.BRIGHT_START if bright else self.NORMAL_START + return base % (color + 30) + return '' - def Stop(self): - """Retruns a stop color code. + def Stop(self): + """Retruns a stop color code. - Returns: - If color is enabled, returns an ANSI color reset sequence, otherwise - returns empty string - """ - if self._enabled: - return self.RESET - return '' + Returns: + If color is enabled, returns an ANSI color reset sequence, + otherwise returns empty string + """ + if self._enabled: + return self.RESET + return '' - def Color(self, color, text, bright=True): - """Returns text with conditionally added color escape sequences. + def Color(self, color, text, bright=True): + """Returns text with conditionally added color escape sequences. - Keyword arguments: - color: Text color -- one of the color constants defined in this class. - text: The text to color. + Keyword arguments: + color: Text color -- one of the color constants defined in this + class. + text: The text to color. - Returns: - If self._enabled is False, returns the original text. If it's True, - returns text with color escape sequences based on the value of color. - """ - if not self._enabled: - return text - if color == self.BOLD: - start = self.BOLD_START - else: - base = self.BRIGHT_START if bright else self.NORMAL_START - start = base % (color + 30) - return start + text + self.RESET + Returns: + If self._enabled is False, returns the original text. If it's True, + returns text with color escape sequences based on the value of + color. + """ + if not self._enabled: + return text + if color == self.BOLD: + start = self.BOLD_START + else: + base = self.BRIGHT_START if bright else self.NORMAL_START + start = base % (color + 30) + return start + text + self.RESET
Signature · 1 line
2.1.0.rc2.206.gedb03e5
PATCH v8 3/6 patman: Correct unit tests to run correctly
In reply to Simon GlassIt 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>
---
Changes in v8: None
Changes in v7: None
Changes in v6:
- Add new patch to fix patman unit tests
Changes in v5: None
tools/patman/gitutil.py | 8 ++++---- tools/patman/patchstream.py | 7 +++++-- tools/patman/terminal.py | 8 ++++++-- tools/patman/test.py | 13 +++++++------ 4 files changed, 22 insertions(+), 14 deletions(-)
tools/patman/gitutil.py +4 -4
@@ -478,13 +478,13 @@ def LookupEmail(lookup_name, alias=None, raise_on_error=True, level=0): ... OSError: Recursive email alias at 'other' >>> LookupEmail('odd', alias, raise_on_error=False) - \033[1;31mAlias 'odd' not found\033[0m + Alias 'odd' not found [] >>> # In this case the loop part will effectively be ignored. >>> LookupEmail('loop', alias, raise_on_error=False) - \033[1;31mRecursive email alias at 'other'\033[0m - \033[1;31mRecursive email alias at 'john'\033[0m - \033[1;31mRecursive email alias at 'mary'\033[0m + Recursive email alias at 'other' + Recursive email alias at 'john' + Recursive email alias at 'mary' ['j.bloggs at napier.co.nz', 'm.poppins at cloud.net'] """ if not alias:
tools/patman/patchstream.py +5 -2
@@ -275,7 +275,7 @@ class PatchStream: # Suppress duplicate signoffs elif signoff_match: - if (self.is_log or + if (self.is_log or not self.commit or self.commit.CheckDuplicateSignoff(signoff_match.group(1))): out = [line]
@@ -312,7 +312,10 @@ class PatchStream: out = [] log = self.series.MakeChangeLog(self.commit) out += self.FormatTags(self.tags) - out += [line] + self.commit.notes + [''] + log + out += [line] + if self.commit: + out += self.commit.notes + out += [''] + log elif self.found_test: if not re_allowed_after_test.match(line): self.lines_after_test += 1
tools/patman/terminal.py +6 -2
@@ -30,8 +30,12 @@ class Color(object): enabled: True if color output should be enabled. If False then this class will not add color codes at all. """ - self._enabled = (colored == COLOR_ALWAYS or - (colored == COLOR_IF_TERMINAL and os.isatty(sys.stdout.fileno()))) + try: + self._enabled = (colored == COLOR_ALWAYS or + (colored == COLOR_IF_TERMINAL and + os.isatty(sys.stdout.fileno()))) + except: + self._enabled = False def Start(self, color, bright=True): """Returns a start color code.
tools/patman/test.py +7 -6
@@ -55,6 +55,7 @@ This adds functions to enable/disable clocks and reset to on-chip peripherals. Signed-off-by: Simon Glass <[email protected]> --- + arch/arm/cpu/armv7/tegra2/Makefile | 2 +- arch/arm/cpu/armv7/tegra2/ap20.c | 57 ++---- arch/arm/cpu/armv7/tegra2/clock.c | 163 +++++++++++++++++
@@ -200,7 +201,7 @@ index 0000000..2234c87 self.assertEqual(result.errors, 0) self.assertEqual(result.warnings, 0) self.assertEqual(result.checks, 0) - self.assertEqual(result.lines, 67) + self.assertEqual(result.lines, 56) os.remove(inf) def testNoSignoff(self):
@@ -211,18 +212,18 @@ index 0000000..2234c87 self.assertEqual(result.errors, 1) self.assertEqual(result.warnings, 0) self.assertEqual(result.checks, 0) - self.assertEqual(result.lines, 67) + self.assertEqual(result.lines, 56) os.remove(inf) def testSpaces(self): inf = self.SetupData('spaces') result = checkpatch.CheckPatch(inf) self.assertEqual(result.ok, False) - self.assertEqual(len(result.problems), 1) + self.assertEqual(len(result.problems), 2) self.assertEqual(result.errors, 0) - self.assertEqual(result.warnings, 1) + self.assertEqual(result.warnings, 2) self.assertEqual(result.checks, 0) - self.assertEqual(result.lines, 67) + self.assertEqual(result.lines, 56) os.remove(inf) def testIndent(self):
@@ -233,7 +234,7 @@ index 0000000..2234c87 self.assertEqual(result.errors, 0) self.assertEqual(result.warnings, 0) self.assertEqual(result.checks, 1) - self.assertEqual(result.lines, 67) + self.assertEqual(result.lines, 56) os.remove(inf)
Signature · 1 line
2.1.0.rc2.206.gedb03e5
PATCH v8 4/6 patman: Remove the -a option
In reply to Simon GlassIt 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.
---
Changes in v8: None
Changes in v7: None
Changes in v6:
- Add new patch to remove patman's -a option
Changes in v5: None
tools/patman/gitutil.py | 88 ------------------------------------------------- tools/patman/patman.py | 7 ---- 2 files changed, 95 deletions(-)
tools/patman/gitutil.py +0 -88
@@ -215,94 +215,6 @@ def CreatePatches(start, count, series): else: return None, files -def ApplyPatch(verbose, fname): - """Apply a patch with git am to test it - - TODO: Convert these to use command, with stderr option - - Args: - fname: filename of patch file to apply - """ - col = terminal.Color() - cmd = ['git', 'am', fname] - pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = pipe.communicate() - re_error = re.compile('^error: patch failed: (.+):(\d+)') - for line in stderr.splitlines(): - if verbose: - print line - match = re_error.match(line) - if match: - print checkpatch.GetWarningMsg(col, 'warning', match.group(1), - int(match.group(2)), 'Patch failed') - return pipe.returncode == 0, stdout - -def ApplyPatches(verbose, args, start_point): - """Apply the patches with git am to make sure all is well - - Args: - verbose: Print out 'git am' output verbatim - args: List of patch files to apply - start_point: Number of commits back from HEAD to start applying. - Normally this is len(args), but it can be larger if a start - offset was given. - """ - error_count = 0 - col = terminal.Color() - - # Figure out our current position - cmd = ['git', 'name-rev', 'HEAD', '--name-only'] - pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE) - stdout, stderr = pipe.communicate() - if pipe.returncode: - str = 'Could not find current commit name' - print col.Color(col.RED, str) - print stdout - return False - old_head = stdout.splitlines()[0] - if old_head == 'undefined': - str = "Invalid HEAD '%s'" % stdout.strip() - print col.Color(col.RED, str) - return False - - # Checkout the required start point - cmd = ['git', 'checkout', 'HEAD~%d' % start_point] - pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = pipe.communicate() - if pipe.returncode: - str = 'Could not move to commit before patch series' - print col.Color(col.RED, str) - print stdout, stderr - return False - - # Apply all the patches - for fname in args: - ok, stdout = ApplyPatch(verbose, fname) - if not ok: - print col.Color(col.RED, 'git am returned errors for %s: will ' - 'skip this patch' % fname) - if verbose: - print stdout - error_count += 1 - cmd = ['git', 'am', '--skip'] - pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE) - stdout, stderr = pipe.communicate() - if pipe.returncode != 0: - print col.Color(col.RED, 'Unable to skip patch! Aborting...') - print stdout - break - - # Return to our previous position - cmd = ['git', 'checkout', old_head] - pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = pipe.communicate() - if pipe.returncode: - print col.Color(col.RED, 'Could not move back to head commit') - print stdout, stderr - return error_count == 0 - def BuildEmailList(in_list, tag=None, alias=None, raise_on_error=True): """Build a list of email addresses based on an input list.
tools/patman/patman.py +0 -7
@@ -25,9 +25,6 @@ import test parser = OptionParser() -parser.add_option('-a', '--no-apply', action='store_false', - dest='apply_patches', default=True, - help="Don't test-apply patches with git am") parser.add_option('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') parser.add_option('-c', '--count', dest='count', type='int',
@@ -144,10 +141,6 @@ else: ok = checkpatch.CheckPatches(options.verbose, args) else: ok = True - if options.apply_patches: - if not gitutil.ApplyPatches(options.verbose, args, - options.count + options.start): - ok = False cc_file = series.MakeCcFile(options.process_tags, cover_fname, not options.ignore_bad_tags)
Signature · 1 line
2.1.0.rc2.206.gedb03e5
PATCH v8 5/6 patman: Use --no-pager' to stop git from forking a pager
In reply to Simon GlassIn a headless environment the pager can apparently hang. We don't want a pager anyway so let's request that none be used.
---
Changes in v8:
- Add new patch to disable the pager in git
Changes in v7: None
Changes in v6: None
Changes in v5: None
tools/patman/gitutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/patman/gitutil.py +1 -1
@@ -33,7 +33,7 @@ def LogCmd(commit_range, git_dir=None, oneline=False, reverse=False, cmd = ['git'] if git_dir: cmd += ['--git-dir', git_dir] - cmd += ['log', '--no-color'] + cmd += ['--no-pager', 'log', '--no-color'] if oneline: cmd.append('--oneline') if use_no_decorate:
Signature · 1 line
2.1.0.rc2.206.gedb03e5
PATCH v8 6/6 RFC: Deprecate MAKEALL
In reply to Simon GlassSince buildman now includes most of the features of MAKEALL it is probably time to talk about deprecating MAKEALL.
Comments welcome.
---
Changes in v8: None
Changes in v7:
- Remove already-applied patches from the series
- Add the deprecation message at the end of the build also
- Drop the 'colour' patch sadly
Changes in v6: None
Changes in v5:
- Drop patch to search for *cc instead of *gcc for the compiler
MAKEALL | 10 ++++++++++ 1 file changed, 10 insertions(+)
MAKEALL +10 -0
@@ -60,6 +60,14 @@ usage() exit ${ret} } +deprecation() { + echo "** Note: MAKEALL is deprecated - please use buildman instead" + echo "** See tools/buildman/README for details" + echo +} + +deprecation + SHORT_OPTS="ha:c:v:s:b:lmMCnr" LONG_OPTS="help,arch:,cpu:,vendor:,soc:,board:,list,maintainers,mails,check,continue,rebuild-errors"
@@ -849,6 +857,8 @@ print_stats() { kill_children fi + deprecation + exit $RC }
Signature · 1 line
2.1.0.rc2.206.gedb03e5
v8 cover Add some missing buildman features and deprecate MAKEALL
In reply to Simon GlassQuoted 11 lines · Simon Glass
Buildman has been around for a little over a year and is used by a fair number of U-Boot developers. However quite a few people still use MAKEALL.
Buildman was intended to replace MAKEALL, so perhaps now is a good time to start that process.
The reasons to deprecate MAKEALL are:
- We don't want to maintain two build systems
- Buildman is typically faster
- Buildman has a lot more features
Quoted 7 lines
This series adds a few features to buildman to fill some gaps, adds some information into the README on how to migrate from MAKEALL, and adds a deprecation message to MAKEALL.
Changes in v8:
- Add new patch to disable the pager in git
Simon,
I have tested this set fixed the reversed order issue. Now I can use buildman to do my tests.
York
v8 cover Add some missing buildman features and deprecate MAKEALL
Hi York,
Quoted 16 lines · York Sun
Quoted 15 lines · Simon Glass
Buildman has been around for a little over a year and is used by a fair number of U-Boot developers. However quite a few people still use MAKEALL.
Buildman was intended to replace MAKEALL, so perhaps now is a good time to start that process.
The reasons to deprecate MAKEALL are:
- We don't want to maintain two build systems
- Buildman is typically faster
- Buildman has a lot more featuresThis series adds a few features to buildman to fill some gaps, adds some information into the README on how to migrate from MAKEALL, and adds a deprecation message to MAKEALL.
Quoted 9 lines
Quoted 3 lines
Changes in v8:
- Add new patch to disable the pager in gitSimon,
I have tested this set fixed the reversed order issue. Now I can use buildman to do my tests.
OK good, thanks for the report.
Regards,
Simon