diff mbox

[U-Boot,v10,11/14] buildman: Add an option to show which boards caused which errors

Message ID 1409240626-6924-12-git-send-email-sjg@chromium.org
State Accepted
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Aug. 28, 2014, 3:43 p.m. UTC
Add a -l option to display a list of offending boards against each
error/warning line. The information will be shown in brackets as below:

02: wip
   sandbox: +   sandbox
       arm: +   seaboard
+(sandbox) arch/sandbox/cpu/cpu.c: In function 'timer_get_us':
+(sandbox) arch/sandbox/cpu/cpu.c:40:9: warning: unused variable 'i' [-Wunused-variable]
+(seaboard) board/nvidia/seaboard/seaboard.c: In function 'pin_mux_mmc':
+(seaboard) board/nvidia/seaboard/seaboard.c:36:9: warning: unused variable 'fred' [-Wunused-variable]
+(seaboard)      int fred;
+(seaboard)          ^

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

Changes in v10: None
Changes in v9:
- Add new patch to support showing which boards caused which errors

Changes in v8: None
Changes in v7: None
Changes in v6: None
Changes in v5: None

 tools/buildman/README      |  7 ++++---
 tools/buildman/builder.py  | 51 ++++++++++++++++++++++++++++++++++++++--------
 tools/buildman/buildman.py |  2 ++
 tools/buildman/control.py  |  3 ++-
 4 files changed, 50 insertions(+), 13 deletions(-)

Comments

Tom Rini Sept. 2, 2014, 12:36 p.m. UTC | #1
On Thu, Aug 28, 2014 at 09:43:43AM -0600, Simon Glass wrote:

> Add a -l option to display a list of offending boards against each
> error/warning line. The information will be shown in brackets as below:
[snip]
> diff --git a/tools/buildman/README b/tools/buildman/README
> index b8c2bd6..fbc8449 100644
> --- a/tools/buildman/README
> +++ b/tools/buildman/README
[snip]
>  If you really want to see build results as they happen, use -v when doing a
> -build (and -e if you want to see errors as well).
> +build (-e will be enabled automatically).

OK, so here is my confusion.  I didn't get from the help that -s implies
-v.  If I do:
-s -v -e -l
I get a verbose summary with per-board warning/error information.  If I
do:
-s -v -l
I don't get that information.  If I follow what you're saying and the
help right, I should do:
$ export COMMON="-b master -c 1 -T 1 -j 24 -o /tmp/trini/eldk521 -G ~/.buildman.eldk521"
$ ./tools/buildman/buildman $COMMON 'arm|powerpc'
$ ./tools/buildman/buildman $COMMON 'arm|powerpc' -s

And that should give me errors as well as which boards gave which?
Simon Glass Sept. 2, 2014, 7:17 p.m. UTC | #2
Hi Tom,

On 2 September 2014 06:36, Tom Rini <trini@ti.com> wrote:
>
> On Thu, Aug 28, 2014 at 09:43:43AM -0600, Simon Glass wrote:
>
> > Add a -l option to display a list of offending boards against each
> > error/warning line. The information will be shown in brackets as below:
> [snip]
> > diff --git a/tools/buildman/README b/tools/buildman/README
> > index b8c2bd6..fbc8449 100644
> > --- a/tools/buildman/README
> > +++ b/tools/buildman/README
> [snip]
> >  If you really want to see build results as they happen, use -v when doing a
> > -build (and -e if you want to see errors as well).
> > +build (-e will be enabled automatically).
>
> OK, so here is my confusion.  I didn't get from the help that -s implies
> -v.  If I do:
> -s -v -e -l
> I get a verbose summary with per-board warning/error information.  If I
> do:
> -s -v -l
> I don't get that information.  If I follow what you're saying and the
> help right, I should do:
> $ export COMMON="-b master -c 1 -T 1 -j 24 -o /tmp/trini/eldk521 -G ~/.buildman.eldk521"
> $ ./tools/buildman/buildman $COMMON 'arm|powerpc'
> $ ./tools/buildman/buildman $COMMON 'arm|powerpc' -s
>
> And that should give me errors as well as which boards gave which?

I don't think that's quite right. There are two completely separate
modes for buildman:

building (no -s)
not building (-s)

I added the -v feature to let you see build results while building. So
-v only applies to the 'building' mode, and implies -e.

For the 'not building' mode, you get a basic summary for free, and can
tell buildman all sorts of things you want to see - like errors (-e)
sizes (-S) function bloat (-B) and list of broken boards (-l)

Regards,
Simon
Simon Glass Sept. 6, 2014, 4:46 p.m. UTC | #3
Applied to u-boot-x86 branch 'patman'
diff mbox

Patch

diff --git a/tools/buildman/README b/tools/buildman/README
index b8c2bd6..fbc8449 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -442,7 +442,8 @@  is fixed, but there is a new one at line 126. This is probably only because
 we added some code and moved the broken line father down the file.
 
 If many boards have the same error, then -e will display the error only
-once. This makes the output as concise as possible.
+once. This makes the output as concise as possible. To see which boards have
+each error, use -l.
 
 The full build output in this case is available in:
 
@@ -745,10 +746,10 @@  followed by (afterwards, or perhaps concurrently in another terminal):
 to see the results of the build. Rather than showing you all the output,
 buildman just shows a summary, with red indicating that a commit introduced
 an error and green indicating that a commit fixed an error. Use the -e
-flag to see the full errors.
+flag to see the full errors and -l to see which boards caused which errors.
 
 If you really want to see build results as they happen, use -v when doing a
-build (and -e if you want to see errors as well).
+build (-e will be enabled automatically).
 
 You don't need to stick around on that branch while buildman is running. It
 checks out its own copy of the source code, so you can change branches,
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 106fde0..b90d7e1 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -237,18 +237,21 @@  class Builder:
             del t
 
     def SetDisplayOptions(self, show_errors=False, show_sizes=False,
-                          show_detail=False, show_bloat=False):
+                          show_detail=False, show_bloat=False,
+                          list_error_boards=False):
         """Setup display options for the builder.
 
         show_errors: True to show summarised error/warning info
         show_sizes: Show size deltas
         show_detail: Show detail for each board
         show_bloat: Show detail for each function
+        list_error_boards: Show the boards which caused each error/warning
         """
         self._show_errors = show_errors
         self._show_sizes = show_sizes
         self._show_detail = show_detail
         self._show_bloat = show_bloat
+        self._list_error_boards = list_error_boards
 
     def _AddTimestamp(self):
         """Add a new timestamp to the list and record the build period.
@@ -570,18 +573,26 @@  class Builder:
                 Dict containing boards which passed building this commit.
                     keyed by board.target
                 List containing a summary of error/warning lines
+                Dict keyed by error line, containing a list of the Board
+                    objects with that error
         """
         board_dict = {}
         err_lines_summary = []
+        err_lines_boards = {}
 
         for board in boards_selected.itervalues():
             outcome = self.GetBuildOutcome(commit_upto, board.target,
                                            read_func_sizes)
             board_dict[board.target] = outcome
             for err in outcome.err_lines:
-                if err and not err.rstrip() in err_lines_summary:
-                    err_lines_summary.append(err.rstrip())
-        return board_dict, err_lines_summary
+                if err:
+                    err = err.rstrip()
+                    if err in err_lines_boards:
+                        err_lines_boards[err].append(board)
+                    else:
+                        err_lines_boards[err] = [board]
+                        err_lines_summary.append(err.rstrip())
+        return board_dict, err_lines_summary, err_lines_boards
 
     def AddOutcome(self, board_dict, arch_list, changes, char, color):
         """Add an output to our list of outcomes for each architecture
@@ -828,7 +839,8 @@  class Builder:
 
 
     def PrintResultSummary(self, board_selected, board_dict, err_lines,
-                           show_sizes, show_detail, show_bloat):
+                           err_line_boards, show_sizes, show_detail,
+                           show_bloat):
         """Compare results with the base results and display delta.
 
         Only boards mentioned in board_selected will be considered. This
@@ -843,10 +855,30 @@  class Builder:
                 commit, keyed by board.target. The value is an Outcome object.
             err_lines: A list of errors for this commit, or [] if there is
                 none, or we don't want to print errors
+            err_line_boards: Dict keyed by error line, containing a list of
+                the Board objects with that error
             show_sizes: Show image size deltas
             show_detail: Show detail for each board
             show_bloat: Show detail for each function
         """
+        def _BoardList(line):
+            """Helper function to get a line of boards containing a line
+
+            Args:
+                line: Error line to search for
+            Return:
+                String containing a list of boards with that error line, or
+                '' if the user has not requested such a list
+            """
+            if self._list_error_boards:
+                names = []
+                for board in err_line_boards[line]:
+                    names.append(board.target)
+                names_str = '(%s) ' % ','.join(names)
+            else:
+                names_str = ''
+            return names_str
+
         better = []     # List of boards fixed since last commit
         worse = []      # List of new broken boards since last commit
         new = []        # List of boards that didn't exist last time
@@ -874,7 +906,7 @@  class Builder:
         worse_err = []
         for line in err_lines:
             if line not in self._base_err_lines:
-                worse_err.append('+' + line)
+                worse_err.append('+' + _BoardList(line) + line)
         for line in self._base_err_lines:
             if line not in err_lines:
                 better_err.append('-' + line)
@@ -918,14 +950,15 @@  class Builder:
                     ', '.join(not_built))
 
     def ProduceResultSummary(self, commit_upto, commits, board_selected):
-            board_dict, err_lines = self.GetResultSummary(board_selected,
-                    commit_upto, read_func_sizes=self._show_bloat)
+            board_dict, err_lines, err_line_boards = self.GetResultSummary(
+                    board_selected, commit_upto,
+                    read_func_sizes=self._show_bloat)
             if commits:
                 msg = '%02d: %s' % (commit_upto + 1,
                         commits[commit_upto].subject)
                 print self.col.Color(self.col.BLUE, msg)
             self.PrintResultSummary(board_selected, board_dict,
-                    err_lines if self._show_errors else [],
+                    err_lines if self._show_errors else [], err_line_boards,
                     self._show_sizes, self._show_detail, self._show_bloat)
 
     def ShowSummary(self, commits, board_selected):
diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py
index 53592e5..1258b76 100755
--- a/tools/buildman/buildman.py
+++ b/tools/buildman/buildman.py
@@ -94,6 +94,8 @@  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',
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7991c74..0785840 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -218,7 +218,8 @@  def DoBuildman(options, args):
                                options)
 
         builder.SetDisplayOptions(options.show_errors, options.show_sizes,
-                                  options.show_detail, options.show_bloat)
+                                  options.show_detail, options.show_bloat,
+                                  options.list_error_boards)
         if options.summary:
             # We can't show function sizes without board details at present
             if options.show_bloat: