* [U-Boot] [PATCH v7 1/5] patman: Support the 'reverse' option for 'git log'
2014-08-14 23:35 [U-Boot] [PATCH v7 0/5] Add some missing buildman features and deprecate MAKEALL Simon Glass
@ 2014-08-14 23:35 ` Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 2/5] patman: Fix indentation in terminal.py Simon Glass
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-08-14 23:35 UTC (permalink / raw)
To: u-boot
This option is currently not supported, but needs to be, for buildman to
operate as expected.
Reported-by: York Sun <yorksun@freescale.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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(+)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 735c8dd..e2b4959 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -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:
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v7 2/5] patman: Fix indentation in terminal.py
2014-08-14 23:35 [U-Boot] [PATCH v7 0/5] Add some missing buildman features and deprecate MAKEALL Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 1/5] patman: Support the 'reverse' option for 'git log' Simon Glass
@ 2014-08-14 23:35 ` Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 3/5] patman: Correct unit tests to run correctly Simon Glass
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-08-14 23:35 UTC (permalink / raw)
To: u-boot
This code came from a different project with 2-character indentation. Fix
it for U-Boot.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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(-)
diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py
index 597d526..11f80d8 100644
--- a/tools/patman/terminal.py
+++ b/tools/patman/terminal.py
@@ -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
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v7 3/5] patman: Correct unit tests to run correctly
2014-08-14 23:35 [U-Boot] [PATCH v7 0/5] Add some missing buildman features and deprecate MAKEALL Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 1/5] patman: Support the 'reverse' option for 'git log' Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 2/5] patman: Fix indentation in terminal.py Simon Glass
@ 2014-08-14 23:35 ` Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 4/5] patman: Remove the -a option Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL Simon Glass
4 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-08-14 23:35 UTC (permalink / raw)
To: u-boot
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>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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(-)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index e2b4959..29e6fdd 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -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:
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 0040468..322374c 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -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
diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py
index 11f80d8..963f2f8 100644
--- a/tools/patman/terminal.py
+++ b/tools/patman/terminal.py
@@ -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.
diff --git a/tools/patman/test.py b/tools/patman/test.py
index 8fcfe53..e8f7472 100644
--- a/tools/patman/test.py
+++ b/tools/patman/test.py
@@ -55,6 +55,7 @@ This adds functions to enable/disable clocks and reset to on-chip peripherals.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
+
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)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v7 4/5] patman: Remove the -a option
2014-08-14 23:35 [U-Boot] [PATCH v7 0/5] Add some missing buildman features and deprecate MAKEALL Simon Glass
` (2 preceding siblings ...)
2014-08-14 23:35 ` [U-Boot] [PATCH v7 3/5] patman: Correct unit tests to run correctly Simon Glass
@ 2014-08-14 23:35 ` Simon Glass
2014-08-14 23:35 ` [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL Simon Glass
4 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-08-14 23:35 UTC (permalink / raw)
To: u-boot
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.
Suggested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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(-)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 29e6fdd..45276e6 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -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.
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index c60aa5a..902fb36 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -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)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-14 23:35 [U-Boot] [PATCH v7 0/5] Add some missing buildman features and deprecate MAKEALL Simon Glass
` (3 preceding siblings ...)
2014-08-14 23:35 ` [U-Boot] [PATCH v7 4/5] patman: Remove the -a option Simon Glass
@ 2014-08-14 23:35 ` Simon Glass
2014-08-15 15:46 ` Stephen Warren
4 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2014-08-14 23:35 UTC (permalink / raw)
To: u-boot
Since buildman now includes most of the features of MAKEALL it is probably
time to talk about deprecating MAKEALL.
Comments welcome.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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(+)
diff --git a/MAKEALL b/MAKEALL
index 929fe88..e412336 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -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
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-14 23:35 ` [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL Simon Glass
@ 2014-08-15 15:46 ` Stephen Warren
2014-08-15 16:13 ` Simon Glass
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Warren @ 2014-08-15 15:46 UTC (permalink / raw)
To: u-boot
On 08/14/2014 05:35 PM, Simon Glass wrote:
> Since buildman now includes most of the features of MAKEALL it is probably
> time to talk about deprecating MAKEALL.
I guess I don't care too much, but I would like to point out that when I
mentioned the annoyance of having to change my scripts:
from:
make ARCH=arm jetson_tk1_config
to:
make ARCH=arm jetson_tk1_defconfig
... together with having to detect which version to use since different
U-Boot commits required a difference command, one of the answers was
that "MAKEALL jetson_tk1" still worked the same on both old and new git
commits, and could be used as a replacement. This patch would invalidate
that.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-15 15:46 ` Stephen Warren
@ 2014-08-15 16:13 ` Simon Glass
2014-08-15 16:20 ` Stephen Warren
0 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2014-08-15 16:13 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On 15 August 2014 09:46, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 08/14/2014 05:35 PM, Simon Glass wrote:
>>
>> Since buildman now includes most of the features of MAKEALL it is probably
>> time to talk about deprecating MAKEALL.
>
>
> I guess I don't care too much, but I would like to point out that when I mentioned the annoyance of having to change my scripts:
>
> from:
> make ARCH=arm jetson_tk1_config
>
> to:
> make ARCH=arm jetson_tk1_defconfig
>
> ... together with having to detect which version to use since different U-Boot commits required a difference command, one of the answers was that "MAKEALL jetson_tk1" still worked the same on both old and new git commits, and could be used as a replacement. This patch would invalidate that.
Well I suppose you could use:
./tools/buildman/buildman jetson_tk1
to get the same effect.
Regards,
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-15 16:13 ` Simon Glass
@ 2014-08-15 16:20 ` Stephen Warren
2014-08-15 16:34 ` York Sun
2014-08-15 16:45 ` Simon Glass
0 siblings, 2 replies; 12+ messages in thread
From: Stephen Warren @ 2014-08-15 16:20 UTC (permalink / raw)
To: u-boot
On 08/15/2014 10:13 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 15 August 2014 09:46, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 08/14/2014 05:35 PM, Simon Glass wrote:
>>>
>>> Since buildman now includes most of the features of MAKEALL it is probably
>>> time to talk about deprecating MAKEALL.
>>
>>
>> I guess I don't care too much, but I would like to point out that when I mentioned the annoyance of having to change my scripts:
>>
>> from:
>> make ARCH=arm jetson_tk1_config
>>
>> to:
>> make ARCH=arm jetson_tk1_defconfig
>>
>> ... together with having to detect which version to use since different U-Boot commits required a difference command, one of the answers was that "MAKEALL jetson_tk1" still worked the same on both old and new git commits, and could be used as a replacement. This patch would invalidate that.
>
> Well I suppose you could use:
>
> ./tools/buildman/buildman jetson_tk1
>
> to get the same effect.
True, although that command has been available for a much shorter time
than MAKEALL...
Replacing the MAKEALL script body with a call to buildman might solve
that? I'm not sure that buildman puts the build results in the same place.
But like I said, I'm not too worried about this personally, so there's
probably no need to change anything.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-15 16:20 ` Stephen Warren
@ 2014-08-15 16:34 ` York Sun
2014-08-23 3:47 ` Simon Glass
2014-08-15 16:45 ` Simon Glass
1 sibling, 1 reply; 12+ messages in thread
From: York Sun @ 2014-08-15 16:34 UTC (permalink / raw)
To: u-boot
On 08/15/2014 09:20 AM, Stephen Warren wrote:
> On 08/15/2014 10:13 AM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On 15 August 2014 09:46, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 08/14/2014 05:35 PM, Simon Glass wrote:
>>>>
>>>> Since buildman now includes most of the features of MAKEALL it is probably
>>>> time to talk about deprecating MAKEALL.
>>>
>>>
>>> I guess I don't care too much, but I would like to point out that when I mentioned the annoyance of having to change my scripts:
>>>
>>> from:
>>> make ARCH=arm jetson_tk1_config
>>>
>>> to:
>>> make ARCH=arm jetson_tk1_defconfig
>>>
>>> ... together with having to detect which version to use since different U-Boot commits required a difference command, one of the answers was that "MAKEALL jetson_tk1" still worked the same on both old and new git commits, and could be used as a replacement. This patch would invalidate that.
>>
>> Well I suppose you could use:
>>
>> ./tools/buildman/buildman jetson_tk1
>>
>> to get the same effect.
>
> True, although that command has been available for a much shorter time
> than MAKEALL...
>
> Replacing the MAKEALL script body with a call to buildman might solve
> that? I'm not sure that buildman puts the build results in the same place.
>
> But like I said, I'm not too worried about this personally, so there's
> probably no need to change anything.
>
I heavily rely on MAKEALL. I have tested buildman recently to confirm it has
most features I need to replace MAKEALL with some command line switches. But I
need to adjust my testing due to some difference
1) The log is different
I can see the err log file, but not the regular log. Buildman runs silently by
default.
2) The return value of buildman doesn't tell if an error happens.
I have to search for err file to identify if any error.
I only tested the features I used most.
York
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-15 16:34 ` York Sun
@ 2014-08-23 3:47 ` Simon Glass
0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-08-23 3:47 UTC (permalink / raw)
To: u-boot
Hi York,
On 15 August 2014 10:34, York Sun <yorksun@freescale.com> wrote:
> On 08/15/2014 09:20 AM, Stephen Warren wrote:
>> On 08/15/2014 10:13 AM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On 15 August 2014 09:46, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 08/14/2014 05:35 PM, Simon Glass wrote:
>>>>>
>>>>> Since buildman now includes most of the features of MAKEALL it is probably
>>>>> time to talk about deprecating MAKEALL.
>>>>
>>>>
>>>> I guess I don't care too much, but I would like to point out that when I mentioned the annoyance of having to change my scripts:
>>>>
>>>> from:
>>>> make ARCH=arm jetson_tk1_config
>>>>
>>>> to:
>>>> make ARCH=arm jetson_tk1_defconfig
>>>>
>>>> ... together with having to detect which version to use since different U-Boot commits required a difference command, one of the answers was that "MAKEALL jetson_tk1" still worked the same on both old and new git commits, and could be used as a replacement. This patch would invalidate that.
>>>
>>> Well I suppose you could use:
>>>
>>> ./tools/buildman/buildman jetson_tk1
>>>
>>> to get the same effect.
>>
>> True, although that command has been available for a much shorter time
>> than MAKEALL...
>>
>> Replacing the MAKEALL script body with a call to buildman might solve
>> that? I'm not sure that buildman puts the build results in the same place.
>>
>> But like I said, I'm not too worried about this personally, so there's
>> probably no need to change anything.
>>
> I heavily rely on MAKEALL. I have tested buildman recently to confirm it has
> most features I need to replace MAKEALL with some command line switches. But I
> need to adjust my testing due to some difference
>
> 1) The log is different
> I can see the err log file, but not the regular log. Buildman runs silently by
> default.
That's right. It's a little tricky to change. Buildman relies on
stderr being empty as a signal that there are no warnings. Even if we
remove the use of make's -s flag then we will get the log in stdout
and the warnings in stderr. They will not be interleaved. We could fix
this by forcing stdout and stderr to the same file, but then we don't
know which is a warning entry and which is just normal output. In fact
we might be able to use leading space as a signal of normal output
most of the time, but I'm not sure.
Buildman relies on being able to sort and uniq the stderr output as a
way of collating warnings and errors, so extraneous stuff just gets in
the way.
Also, now that we have Kbuild we get a full path on each filename that
is built. It used to be that you had to check the directory enter/exit
messages to figure out the source file for warnings and errors, but
that isn't the case now.
In short I don't think this is easy to do correctly, and I'm not sure
how useful it is anyway.
>
> 2) The return value of buildman doesn't tell if an error happens.
> I have to search for err file to identify if any error.
OK I'll make buildman exit with a suitable return code.
>
> I only tested the features I used most.
One day I may expand the unit test coverage which will make it easier
to add new features.
Regards,
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v7 5/5] RFC: Deprecate MAKEALL
2014-08-15 16:20 ` Stephen Warren
2014-08-15 16:34 ` York Sun
@ 2014-08-15 16:45 ` Simon Glass
1 sibling, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-08-15 16:45 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On 15 August 2014 10:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/15/2014 10:13 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 15 August 2014 09:46, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>>
>>> On 08/14/2014 05:35 PM, Simon Glass wrote:
>>>>
>>>>
>>>> Since buildman now includes most of the features of MAKEALL it is
>>>> probably
>>>> time to talk about deprecating MAKEALL.
>>>
>>>
>>>
>>> I guess I don't care too much, but I would like to point out that when I
>>> mentioned the annoyance of having to change my scripts:
>>>
>>> from:
>>> make ARCH=arm jetson_tk1_config
>>>
>>> to:
>>> make ARCH=arm jetson_tk1_defconfig
>>>
>>> ... together with having to detect which version to use since different
>>> U-Boot commits required a difference command, one of the answers was that
>>> "MAKEALL jetson_tk1" still worked the same on both old and new git commits,
>>> and could be used as a replacement. This patch would invalidate that.
>>
>>
>> Well I suppose you could use:
>>
>> ./tools/buildman/buildman jetson_tk1
>>
>> to get the same effect.
>
>
> True, although that command has been available for a much shorter time than
> MAKEALL...
>
> Replacing the MAKEALL script body with a call to buildman might solve that?
> I'm not sure that buildman puts the build results in the same place.
>
No, you would need -o if you want them to go to a particular place.
> But like I said, I'm not too worried about this personally, so there's
> probably no need to change anything.
OK I still have a few things left to improve, but we are getting there,
Regards,
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread