[U-Boot] [PATCH 00/13] buildman: Threading and output improvements

[U-Boot] [PATCH 00/13] buildman: Threading and output improvements

[Simon Glass]

This series includes a few minor improvements to buildman:

- Cleaner output and updates on what buildman is doing at the start
- a way to generate only the CONFIG options in a build, for speed (-D)
- a fix for pressing Ctrl-C during a build (so that it exits cleanly).


Simon Glass (13):
  Makefile: Add a target to create the .cfg files
  buildman: Add an option to just create the config
  buildman: Add documentation for CONFIG checking
  buildman: Squash useless output from -K
  patman: Flush output when there is no newline
  buildman: Tidy up the 'cloning' message
  buildman: Print a message when removing old directories
  buildman: Print a message indicating the build is starting
  buildman: Put our local libraries first in the path
  buildman: Allow builds to terminate cleanly
  buildman: Drop the 'active' flag in the builder
  buildman: Don't show a stacktrace on Ctrl-C
  buildman: Drop the 'alive' flag in BuilderThread

 Makefile                        |  9 ++++++
 scripts/Makefile.spl            |  2 ++
 tools/buildman/README           | 49 +++++++++++++++++++++++++++++
 tools/buildman/builder.py       | 70 ++++++++++++++++++++++++++++-------------
 tools/buildman/builderthread.py | 29 +++++++----------
 tools/buildman/buildman.py      |  2 +-
 tools/buildman/cmdline.py       |  4 +++
 tools/buildman/control.py       |  4 ++-
 tools/buildman/test.py          |  4 +--
 tools/patman/terminal.py        |  2 ++
 10 files changed, 132 insertions(+), 43 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 01/13] Makefile: Add a target to create the .cfg files

[Simon Glass]

A common requirement when converting CONFIG options to Kconfig is to check
that the effective configuration has not changed due to the conversion. Add
a target which creates this configuration (in the form of u-boot.cfg) but
does not build U-Boot. This speeds up the checking.

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

 Makefile             | 9 +++++++++
 scripts/Makefile.spl | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/Makefile b/Makefile
index fffc188..f835780 100644
--- a/Makefile
+++ b/Makefile
@@ -812,6 +812,14 @@ append = cat $(filter-out $< $(PHONY), $^) >> $@
 quiet_cmd_pad_cat = CAT     $@
 cmd_pad_cat = $(cmd_objcopy) && $(append) || rm -f $@
 
+cfg: u-boot.cfg $(if $(CONFIG_SPL),cfg-spl) $(if $(CONFIG_TPL),cfg-tpl)
+
+cfg-spl:
+	$(Q)$(MAKE) obj=spl -f $(srctree)/scripts/Makefile.spl cfg
+
+cfg-tpl:
+	$(Q)$(MAKE) obj=tpl -f $(srctree)/scripts/Makefile.spl cfg
+
 all:		$(ALL-y)
 ifeq ($(CONFIG_DM_I2C_COMPAT)$(CONFIG_SANDBOX),y)
 	@echo "===================== WARNING ======================"
@@ -1517,6 +1525,7 @@ help:
 	@echo  '  cscope	  - Generate cscope index'
 	@echo  '  ubootrelease	  - Output the release version string (use with make -s)'
 	@echo  '  ubootversion	  - Output the version stored in Makefile (use with make -s)'
+	@echo  "  cfg		  - Don't build, just create the .cfg files"
 	@echo  ''
 	@echo  'Static analysers'
 	@echo  '  checkstack      - Generate a list of stack hogs'
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 4994fa8..21bdcf2 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -219,6 +219,8 @@ cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 $(obj)/$(SPL_BIN).cfg:	include/config.h FORCE
 	$(call if_changed,cpp_cfg)
 
+cfg: $(obj)/$(SPL_BIN).cfg
+
 pythonpath = PYTHONPATH=tools
 
 quiet_cmd_dtocc = DTOC C  $@
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 02/13] buildman: Add an option to just create the config

[Simon Glass]

Normally buildman does a full build of a board. This includes creating the
u-boot.cfg file which contains all the configuration options. Buildman uses
this file with the -K option, to show differences in effective configuration
for each commit.

Doing a full build of U-Boot just to create the u-boot.cfg file is wasteful.
Add a -D option which causes buildman to only create the configuration. This
is enough to support use of -K and can be done much more quickly (typically
5-10 times faster).

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

 tools/buildman/builder.py       |  5 ++++-
 tools/buildman/builderthread.py | 17 +++++++++++------
 tools/buildman/cmdline.py       |  2 ++
 tools/buildman/control.py       |  3 ++-
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 8ec3551..98f1091 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -206,7 +206,8 @@ class Builder:
     def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs,
                  gnu_make='make', checkout=True, show_unknown=True, step=1,
                  no_subdirs=False, full_path=False, verbose_build=False,
-                 incremental=False, per_board_out_dir=False):
+                 incremental=False, per_board_out_dir=False,
+                 config_only=False):
         """Create a new Builder object
 
         Args:
@@ -229,6 +230,7 @@ class Builder:
                 mrproper when configuring
             per_board_out_dir: Build in a separate persistent directory per
                 board rather than a thread-specific directory
+            config_only: Only configure each build, don't build it
         """
         self.toolchains = toolchains
         self.base_dir = base_dir
@@ -257,6 +259,7 @@ class Builder:
         self.no_subdirs = no_subdirs
         self.full_path = full_path
         self.verbose_build = verbose_build
+        self.config_only = config_only
 
         self.col = terminal.Color()
 
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index c512d3b..a30e4fd 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -110,8 +110,8 @@ class BuilderThread(threading.Thread):
         return self.builder.do_make(commit, brd, stage, cwd, *args,
                 **kwargs)
 
-    def RunCommit(self, commit_upto, brd, work_dir, do_config, force_build,
-                  force_build_failures):
+    def RunCommit(self, commit_upto, brd, work_dir, do_config, do_build,
+                  force_build, force_build_failures):
         """Build a particular commit.
 
         If the build is already done, and we are not forcing a build, we skip
@@ -122,6 +122,7 @@ class BuilderThread(threading.Thread):
             brd: Board object to build
             work_dir: Directory to which the source will be checked out
             do_config: True to run a make <board>_defconfig on the source
+            do_build: Try to build the source
             force_build: Force a build even if one was previously done
             force_build_failures: Force a bulid if the previous result showed
                 failure
@@ -231,6 +232,8 @@ class BuilderThread(threading.Thread):
                     config_out += result.combined
                     do_config = False   # No need to configure next time
                 if result.return_code == 0:
+                    if not do_build:
+                        args.append('cfg')
                     result = self.Make(commit, brd, 'build', cwd, *args,
                             env=env)
                 result.stderr = result.stderr.replace(src_dir + '/', '')
@@ -405,7 +408,7 @@ class BuilderThread(threading.Thread):
             force_build = False
             for commit_upto in range(0, len(job.commits), job.step):
                 result, request_config = self.RunCommit(commit_upto, brd,
-                        work_dir, do_config,
+                        work_dir, do_config, not self.builder.config_only,
                         force_build or self.builder.force_build,
                         self.builder.force_build_failures)
                 failed = result.return_code or result.stderr
@@ -415,9 +418,10 @@ class BuilderThread(threading.Thread):
                     # with a reconfig.
                     if self.builder.force_config_on_failure:
                         result, request_config = self.RunCommit(commit_upto,
-                            brd, work_dir, True, True, False)
+                            brd, work_dir, True, False, True, False)
                         did_config = True
-                if not self.builder.force_reconfig:
+                if (not self.builder.force_reconfig and
+                    not self.builder.config_only):
                     do_config = request_config
 
                 # If we built that commit, then config is done. But if we got
@@ -459,7 +463,8 @@ class BuilderThread(threading.Thread):
         else:
             # Just build the currently checked-out build
             result, request_config = self.RunCommit(None, brd, work_dir, True,
-                        True, self.builder.force_build_failures)
+                        not self.builder.config_only, True,
+                        self.builder.force_build_failures)
             result.commit_upto = 0
             self._WriteResult(result, job.keep_outputs)
             self.builder.out_queue.put(result)
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 3e3bd63..bd3776a 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -28,6 +28,8 @@ def ParseArgs():
     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('-D', '--config-only', action='store_true', default=False,
+          help="Don't build, just configure each commit")
     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',
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b86d7b3..8e347d4 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -258,7 +258,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
             no_subdirs=options.no_subdirs, full_path=options.full_path,
             verbose_build=options.verbose_build,
             incremental=options.incremental,
-            per_board_out_dir=options.per_board_out_dir,)
+            per_board_out_dir=options.per_board_out_dir,
+            config_only=options.config_only)
     builder.force_config_on_failure = not options.quick
     if make_func:
         builder.do_make = make_func
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 03/13] buildman: Add documentation for CONFIG checking

[Simon Glass]

The -K option is not mentioned in the README at present. Add some notes
to describe how this is used.

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

 tools/buildman/README | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/buildman/README b/tools/buildman/README
index 8c5f861..ee330ba 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -959,6 +959,43 @@ of the source tree, thus allowing rapid tested evolution of the code.
     SOURCE_DATE_EPOCH=0 ./tools/buildman/buildman -I -P tegra
 
 
+Checking configuration
+======================
+
+A common requirement when converting CONFIG options to Kconfig is to check
+that the effective configuration has not changed due to the conversion.
+Buildman supports this with the -K option, used after a build. This shows
+differences in effective configuration between one commit and the next.
+
+For example:
+
+    $ buildman -b kc4 -sK
+    ...
+    43: Convert CONFIG_SPL_USBETH_SUPPORT to Kconfig
+    arm:
+    + u-boot.cfg: CONFIG_SPL_ENV_SUPPORT=1 CONFIG_SPL_NET_SUPPORT=1
+    + u-boot-spl.cfg: CONFIG_SPL_MMC_SUPPORT=1 CONFIG_SPL_NAND_SUPPORT=1
+    + all: CONFIG_SPL_ENV_SUPPORT=1 CONFIG_SPL_MMC_SUPPORT=1 CONFIG_SPL_NAND_SUPPORT=1 CONFIG_SPL_NET_SUPPORT=1
+    am335x_evm_usbspl :
+    + u-boot.cfg: CONFIG_SPL_ENV_SUPPORT=1 CONFIG_SPL_NET_SUPPORT=1
+    + u-boot-spl.cfg: CONFIG_SPL_MMC_SUPPORT=1 CONFIG_SPL_NAND_SUPPORT=1
+    + all: CONFIG_SPL_ENV_SUPPORT=1 CONFIG_SPL_MMC_SUPPORT=1 CONFIG_SPL_NAND_SUPPORT=1 CONFIG_SPL_NET_SUPPORT=1
+    44: Convert CONFIG_SPL_USB_HOST_SUPPORT to Kconfig
+    ...
+
+This shows that commit 44 enabled three new options for the board
+am335x_evm_usbspl which were not enabled in commit 43. There is also a
+summary for 'arm' showing all the changes detected for that architecture.
+In this case there is only one board with changes, so 'arm' output is the
+same as 'am335x_evm_usbspl'/
+
+The -K option uses the u-boot.cfg, spl/u-boot-spl.cfg and tpl/u-boot-tpl.cfg
+files which are produced by a build. If all you want is to check the
+configuration you can in fact avoid doing a full build, using -D. This tells
+buildman to configuration U-Boot and create the .cfg files, but not actually
+build the source. This is 5-10 times faster than doing a full build.
+
+
 Other options
 =============
 
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 04/13] buildman: Squash useless output from -K

[Simon Glass]

When using #define CONFIG_SOME_OPTION, the value it set to '1'. When using
defconfig (i.e. CONFIG_SOME_OPTION=y) the value is set to 'y'. This results
in differences showing up with -K. These differences are seldom useful.

Adjust buildman to suppress these differences by default.

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

 tools/buildman/README     | 12 ++++++++++++
 tools/buildman/builder.py | 34 ++++++++++++++++++++++------------
 tools/buildman/cmdline.py |  2 ++
 tools/buildman/control.py |  3 ++-
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/tools/buildman/README b/tools/buildman/README
index ee330ba..4b34cdb 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -995,6 +995,18 @@ configuration you can in fact avoid doing a full build, using -D. This tells
 buildman to configuration U-Boot and create the .cfg files, but not actually
 build the source. This is 5-10 times faster than doing a full build.
 
+By default buildman considers the follow two configuration methods
+equivalent:
+
+   #define CONFIG_SOME_OPTION
+
+   CONFIG_SOME_OPTION=y
+
+The former would appear in a header filer and the latter in a defconfig
+file. The achieve this, buildman considers 'y' to be '1' in configuration
+variables. This avoids lots of useless output when converting a CONFIG
+option to Kconfig. To disable this behaviour, use --squash-config-y.
+
 
 Other options
 =============
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 98f1091..2aca189 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -96,19 +96,22 @@ OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN = range(4)
 # Translate a commit subject into a valid filename
 trans_valid_chars = string.maketrans("/: ", "---")
 
-CONFIG_FILENAMES = [
+BASE_CONFIG_FILENAMES = [
+    'u-boot.cfg', 'u-boot-spl.cfg', 'u-boot-tpl.cfg'
+]
+
+EXTRA_CONFIG_FILENAMES = [
     '.config', '.config-spl', '.config-tpl',
     'autoconf.mk', 'autoconf-spl.mk', 'autoconf-tpl.mk',
     'autoconf.h', 'autoconf-spl.h','autoconf-tpl.h',
-    'u-boot.cfg', 'u-boot-spl.cfg', 'u-boot-tpl.cfg'
 ]
 
 class Config:
     """Holds information about configuration settings for a board."""
-    def __init__(self, target):
+    def __init__(self, config_filename, target):
         self.target = target
         self.config = {}
-        for fname in CONFIG_FILENAMES:
+        for fname in config_filename:
             self.config[fname] = {}
 
     def Add(self, fname, key, value):
@@ -207,7 +210,7 @@ class Builder:
                  gnu_make='make', checkout=True, show_unknown=True, step=1,
                  no_subdirs=False, full_path=False, verbose_build=False,
                  incremental=False, per_board_out_dir=False,
-                 config_only=False):
+                 config_only=False, squash_config_y=False):
         """Create a new Builder object
 
         Args:
@@ -231,6 +234,7 @@ class Builder:
             per_board_out_dir: Build in a separate persistent directory per
                 board rather than a thread-specific directory
             config_only: Only configure each build, don't build it
+            squash_config_y: Convert CONFIG options with the value 'y' to '1'
         """
         self.toolchains = toolchains
         self.base_dir = base_dir
@@ -260,6 +264,10 @@ class Builder:
         self.full_path = full_path
         self.verbose_build = verbose_build
         self.config_only = config_only
+        self.squash_config_y = squash_config_y
+        self.config_filenames = BASE_CONFIG_FILENAMES
+        if not self.squash_config_y:
+            self.config_filenames += EXTRA_CONFIG_FILENAMES
 
         self.col = terminal.Color()
 
@@ -585,13 +593,15 @@ class Builder:
                             key, value = values
                         else:
                             key = values[0]
-                            value = ''
+                            value = '1' if self.squash_config_y else ''
                         if not key.startswith('CONFIG_'):
                             continue
                     elif not line or line[0] in ['#', '*', '/']:
                         continue
                     else:
                         key, value = line.split('=', 1)
+                    if self.squash_config_y and value == 'y':
+                        value = '1'
                     config[key] = value
         return config
 
@@ -658,7 +668,7 @@ class Builder:
 
             if read_config:
                 output_dir = self.GetBuildDir(commit_upto, target)
-                for name in CONFIG_FILENAMES:
+                for name in self.config_filenames:
                     fname = os.path.join(output_dir, name)
                     config[name] = self._ProcessConfig(fname)
 
@@ -735,8 +745,8 @@ class Builder:
                                     line, board)
                         last_was_warning = is_warning
                         last_func = None
-            tconfig = Config(board.target)
-            for fname in CONFIG_FILENAMES:
+            tconfig = Config(self.config_filenames, board.target)
+            for fname in self.config_filenames:
                 if outcome.config:
                     for key, value in outcome.config[fname].iteritems():
                         tconfig.Add(fname, key, value)
@@ -1192,7 +1202,7 @@ class Builder:
                 arch_config_plus[arch] = {}
                 arch_config_minus[arch] = {}
                 arch_config_change[arch] = {}
-                for name in CONFIG_FILENAMES:
+                for name in self.config_filenames:
                     arch_config_plus[arch][name] = {}
                     arch_config_minus[arch][name] = {}
                     arch_config_change[arch][name] = {}
@@ -1209,7 +1219,7 @@ class Builder:
                 tbase = self._base_config[target]
                 tconfig = config[target]
                 lines = []
-                for name in CONFIG_FILENAMES:
+                for name in self.config_filenames:
                     if not tconfig.config[name]:
                         continue
                     config_plus = {}
@@ -1253,7 +1263,7 @@ class Builder:
                 all_plus = {}
                 all_minus = {}
                 all_change = {}
-                for name in CONFIG_FILENAMES:
+                for name in self.config_filenames:
                     all_plus.update(arch_config_plus[arch][name])
                     all_minus.update(arch_config_minus[arch][name])
                     all_change.update(arch_config_change[arch][name])
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index bd3776a..0060e03 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -59,6 +59,8 @@ def ParseArgs():
           default=False, help='Keep all build output files (e.g. binaries)')
     parser.add_option('-K', '--show-config', action='store_true',
           default=False, help='Show configuration changes in summary (both board config files and Kconfig)')
+    parser.add_option('--preserve-config-y', action='store_true',
+          default=False, help="Don't convert y to 1 in configs")
     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,
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 8e347d4..e2cddef 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -259,7 +259,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
             verbose_build=options.verbose_build,
             incremental=options.incremental,
             per_board_out_dir=options.per_board_out_dir,
-            config_only=options.config_only)
+            config_only=options.config_only,
+            squash_config_y=not options.preserve_config_y)
     builder.force_config_on_failure = not options.quick
     if make_func:
         builder.do_make = make_func
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 05/13] patman: Flush output when there is no newline

[Simon Glass]

Output which does not include a newline will not be displayed unless
flushed. Add a flush to ensure that it becomes visible.

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

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

diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py
index e78a7c1..af3593e 100644
--- a/tools/patman/terminal.py
+++ b/tools/patman/terminal.py
@@ -55,6 +55,8 @@ def Print(text='', newline=True, colour=None):
         print text,
         if newline:
             print
+        else:
+            sys.stdout.flush()
 
 def SetPrintTestMode():
     """Go into test mode, where all printing is recorded"""
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Simon Glass]

On a machine with a lot of CPUs this prints a lot of useless lines of the
form:

   Cloning repo for thread <n>

Adjust the output so that these all appear on one line, and disappear when
the cloning is complete.

Note: This cloning is actually unnecessary and very wasteful on disk space
(about 3.5GB each time). It would be better to create symlinks.

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

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

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 2aca189..7c237a5 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1379,8 +1379,10 @@ class Builder:
             if os.path.exists(git_dir):
                 gitutil.Fetch(git_dir, thread_dir)
             else:
-                Print('Cloning repo for thread %d' % thread_num)
+                Print('\rCloning repo for thread %d' % thread_num,
+                      newline=False)
                 gitutil.Clone(src_dir, thread_dir)
+                Print('\r%s\r' % (' ' * 30), newline=False)
 
     def _PrepareWorkingSpace(self, max_threads, setup_git):
         """Prepare the working directory for use.
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 07/13] buildman: Print a message when removing old directories

[Simon Glass]

When buildman starts, it prepares its output directory by removing any old
build directories which will not be used this time. This can happen if a
previous build left directories around for commit hashes which are no-longer
part of the branch.

This can take quite a while, so print a message to indicate what is going
on.

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

 tools/buildman/builder.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 7c237a5..a80333f 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1410,8 +1410,14 @@ class Builder:
         for commit_upto in range(self.commit_count):
             dir_list.append(self._GetOutputDir(commit_upto))
 
+        to_remove = []
         for dirname in glob.glob(os.path.join(self.base_dir, '*')):
             if dirname not in dir_list:
+                to_remove.append(dirname)
+        if to_remove:
+            Print('Removing %d old build directories' % len(to_remove),
+                  newline=False)
+            for dirname in to_remove:
                 shutil.rmtree(dirname)
 
     def BuildBoards(self, commits, board_selected, keep_outputs, verbose):
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 08/13] buildman: Print a message indicating the build is starting

[Simon Glass]

Make it clear when buildman actually starts building. This happens when it
has prepared the threads, working directory and output directories.

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

 tools/buildman/builder.py | 1 +
 tools/buildman/test.py    | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index a80333f..d4ea13e 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1443,6 +1443,7 @@ class Builder:
         self._PrepareWorkingSpace(min(self.num_threads, len(board_selected)),
                 commits is not None)
         self._PrepareOutputSpace()
+        Print('\rStarting build...', newline=False)
         self.SetupBuild(board_selected, commits)
         self.ProcessResult(None)
 
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index d8f3c81..ed2a3a8 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -198,9 +198,9 @@ class TestBuild(unittest.TestCase):
             if line.text.strip():
                 count += 1
 
-        # We should get one starting message, then an update for every commit
+        # We should get two starting messages, then an update for every commit
         # built.
-        self.assertEqual(count, len(commits) * len(boards) + 1)
+        self.assertEqual(count, len(commits) * len(boards) + 2)
         build.SetDisplayOptions(show_errors=True);
         build.ShowSummary(self.commits, board_selected)
         #terminal.EchoPrintTestLines()
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 09/13] buildman: Put our local libraries first in the path

[Simon Glass]

If patman is installed on the machine (e.g. in the standard dist-packages
directory), it will find libraries from there in preference to our local
libraries. Adjust the order of the path to ensure that local libraries are
found first.

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

 tools/buildman/buildman.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py
index d0afeda..607429d 100755
--- a/tools/buildman/buildman.py
+++ b/tools/buildman/buildman.py
@@ -15,7 +15,7 @@ import unittest
 
 # Bring in the patman libraries
 our_path = os.path.dirname(os.path.realpath(__file__))
-sys.path.append(os.path.join(our_path, '../patman'))
+sys.path.insert(1, os.path.join(our_path, '../patman'))
 
 # Our modules
 import board
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 10/13] buildman: Allow builds to terminate cleanly

[Simon Glass]

It is annoying that buildman does not respond cleanly to Ctrl-C or SIGINT,
particularly on machines with lots of CPUS. Unfortunately queue.join()
blocks the main thread and does not allow it to see the signal. Use a
separate thread instead,

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

 tools/buildman/builder.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index d4ea13e..e774c2d 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -14,6 +14,7 @@ import Queue
 import shutil
 import string
 import sys
+import threading
 import time
 
 import builderthread
@@ -1456,8 +1457,11 @@ class Builder:
             job.step = self._step
             self.queue.put(job)
 
-        # Wait until all jobs are started
-        self.queue.join()
+        term = threading.Thread(target=self.queue.join)
+        term.setDaemon(True)
+        term.start()
+        while term.isAlive():
+            term.join(100)
 
         # Wait until we have processed all output
         self.out_queue.join()
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 11/13] buildman: Drop the 'active' flag in the builder

[Simon Glass]

This serves no real purpose, since when we are not active, we exit. Drop it.

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

 tools/buildman/builder.py       |  7 -------
 tools/buildman/builderthread.py | 10 +---------
 2 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index e774c2d..95369f0 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -130,7 +130,6 @@ class Builder:
     """Class for building U-Boot for a particular commit.
 
     Public members: (many should ->private)
-        active: True if the builder is active and has not been stopped
         already_done: Number of builds already completed
         base_dir: Base directory to use for builder
         checkout: True to check out source, False to skip that step.
@@ -241,7 +240,6 @@ class Builder:
         self.base_dir = base_dir
         self._working_dir = os.path.join(base_dir, '.bm-work')
         self.threads = []
-        self.active = True
         self.do_make = self.Make
         self.gnu_make = gnu_make
         self.checkout = checkout
@@ -401,11 +399,6 @@ class Builder:
         if result:
             target = result.brd.target
 
-            if result.return_code < 0:
-                self.active = False
-                command.StopAll()
-                return
-
             self.upto += 1
             if result.return_code != 0:
                 self.fail += 1
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index a30e4fd..06be5d9 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -478,14 +478,6 @@ class BuilderThread(threading.Thread):
         alive = True
         while True:
             job = self.builder.queue.get()
-            if self.builder.active and alive:
+            if alive:
                 self.RunJob(job)
-            '''
-            try:
-                if self.builder.active and alive:
-                    self.RunJob(job)
-            except Exception as err:
-                alive = False
-                print err
-            '''
             self.builder.queue.task_done()
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 12/13] buildman: Don't show a stacktrace on Ctrl-C

[Simon Glass]

When Ctrl-C is pressed, just exited quietly. There is no sense in displaying
a stack trace since buildman will always be in the same place: waiting for
threads to complete building all the jobs on the queue.

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

 tools/buildman/builder.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 95369f0..6905ed9 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -12,6 +12,7 @@ import os
 import re
 import Queue
 import shutil
+import signal
 import string
 import sys
 import threading
@@ -293,11 +294,17 @@ class Builder:
         ignore_lines = ['(make.*Waiting for unfinished)', '(Segmentation fault)']
         self.re_make_err = re.compile('|'.join(ignore_lines))
 
+        # Handle existing graceful with SIGINT / Ctrl-C
+        signal.signal(signal.SIGINT, self.signal_handler)
+
     def __del__(self):
         """Get rid of all threads created by the builder"""
         for t in self.threads:
             del t
 
+    def signal_handler(self, signal, frame):
+        sys.exit(1)
+
     def SetDisplayOptions(self, show_errors=False, show_sizes=False,
                           show_detail=False, show_bloat=False,
                           list_error_boards=False, show_config=False):
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 13/13] buildman: Drop the 'alive' flag in BuilderThread

[Simon Glass]

This is not used, so drop it.

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

 tools/buildman/builderthread.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 06be5d9..a02aa19 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -475,9 +475,7 @@ class BuilderThread(threading.Thread):
         This thread picks a job from the queue, runs it, and then goes to the
         next job.
         """
-        alive = True
         while True:
             job = self.builder.queue.get()
-            if alive:
-                self.RunJob(job)
+            self.RunJob(job)
             self.builder.queue.task_done()
-- 
2.8.0.rc3.226.g39d4020

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Tom Rini]

On Sun, Sep 18, 2016 at 04:48:31PM -0600, Simon Glass wrote:

> On a machine with a lot of CPUs this prints a lot of useless lines of the
> form:
> 
>    Cloning repo for thread <n>
> 
> Adjust the output so that these all appear on one line, and disappear when
> the cloning is complete.
> 
> Note: This cloning is actually unnecessary and very wasteful on disk space
> (about 3.5GB each time). It would be better to create symlinks.

When do we actually need to clone?  Or why, even?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160919/b68af208/attachment.sig>

[U-Boot] [PATCH 10/13] buildman: Allow builds to terminate cleanly

[Tom Rini]

On Sun, Sep 18, 2016 at 04:48:35PM -0600, Simon Glass wrote:

> It is annoying that buildman does not respond cleanly to Ctrl-C or SIGINT,
> particularly on machines with lots of CPUS. Unfortunately queue.join()
> blocks the main thread and does not allow it to see the signal. Use a
> separate thread instead,
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Yay for fixing this, killing off the running make's when I have a bad
test build going is one of the minor pitas in my workflow :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160919/02648547/attachment.sig>

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Simon Glass]

Hi Tom,

On 19 September 2016 at 13:14, Tom Rini <trini@konsulko.com> wrote:
> On Sun, Sep 18, 2016 at 04:48:31PM -0600, Simon Glass wrote:
>
>> On a machine with a lot of CPUs this prints a lot of useless lines of the
>> form:
>>
>>    Cloning repo for thread <n>
>>
>> Adjust the output so that these all appear on one line, and disappear when
>> the cloning is complete.
>>
>> Note: This cloning is actually unnecessary and very wasteful on disk space
>> (about 3.5GB each time). It would be better to create symlinks.
>
> When do we actually need to clone?  Or why, even?

Each thread can check out a different commit. I think it might be
possible to put the checkout in a separate thread so it can avoid
contention. I'm not quite sure...

Regards,
Simon

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Tom Rini]

On Thu, Sep 22, 2016 at 10:14:01PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 19 September 2016 at 13:14, Tom Rini <trini@konsulko.com> wrote:
> > On Sun, Sep 18, 2016 at 04:48:31PM -0600, Simon Glass wrote:
> >
> >> On a machine with a lot of CPUs this prints a lot of useless lines of the
> >> form:
> >>
> >>    Cloning repo for thread <n>
> >>
> >> Adjust the output so that these all appear on one line, and disappear when
> >> the cloning is complete.
> >>
> >> Note: This cloning is actually unnecessary and very wasteful on disk space
> >> (about 3.5GB each time). It would be better to create symlinks.
> >
> > When do we actually need to clone?  Or why, even?
> 
> Each thread can check out a different commit. I think it might be
> possible to put the checkout in a separate thread so it can avoid
> contention. I'm not quite sure...

Ah, ok.  So we have a few places under tools/ that should be doing 'git
clone --local', at first glance.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/bca6f760/attachment.sig>

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Simon Glass]

Hi Tom,

On 23 September 2016 at 12:42, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Sep 22, 2016 at 10:14:01PM -0600, Simon Glass wrote:
>> Hi Tom,
>>
>> On 19 September 2016 at 13:14, Tom Rini <trini@konsulko.com> wrote:
>> > On Sun, Sep 18, 2016 at 04:48:31PM -0600, Simon Glass wrote:
>> >
>> >> On a machine with a lot of CPUs this prints a lot of useless lines of the
>> >> form:
>> >>
>> >>    Cloning repo for thread <n>
>> >>
>> >> Adjust the output so that these all appear on one line, and disappear when
>> >> the cloning is complete.
>> >>
>> >> Note: This cloning is actually unnecessary and very wasteful on disk space
>> >> (about 3.5GB each time). It would be better to create symlinks.
>> >
>> > When do we actually need to clone?  Or why, even?
>>
>> Each thread can check out a different commit. I think it might be
>> possible to put the checkout in a separate thread so it can avoid
>> contention. I'm not quite sure...
>
> Ah, ok.  So we have a few places under tools/ that should be doing 'git
> clone --local', at first glance.

Actually I think I am wrong about this. The 3.5GB is due to 32 copies
of the code, not the cloning. In fact --local is the default and that
is what buildman seems to use.

So in fact I'm not sure we can change it. Each thread can be building
a different commit.

Regards,
Simon

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Tom Rini]

On Fri, Sep 23, 2016 at 03:09:53PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 23 September 2016 at 12:42, Tom Rini <trini@konsulko.com> wrote:
> > On Thu, Sep 22, 2016 at 10:14:01PM -0600, Simon Glass wrote:
> >> Hi Tom,
> >>
> >> On 19 September 2016 at 13:14, Tom Rini <trini@konsulko.com> wrote:
> >> > On Sun, Sep 18, 2016 at 04:48:31PM -0600, Simon Glass wrote:
> >> >
> >> >> On a machine with a lot of CPUs this prints a lot of useless lines of the
> >> >> form:
> >> >>
> >> >>    Cloning repo for thread <n>
> >> >>
> >> >> Adjust the output so that these all appear on one line, and disappear when
> >> >> the cloning is complete.
> >> >>
> >> >> Note: This cloning is actually unnecessary and very wasteful on disk space
> >> >> (about 3.5GB each time). It would be better to create symlinks.
> >> >
> >> > When do we actually need to clone?  Or why, even?
> >>
> >> Each thread can check out a different commit. I think it might be
> >> possible to put the checkout in a separate thread so it can avoid
> >> contention. I'm not quite sure...
> >
> > Ah, ok.  So we have a few places under tools/ that should be doing 'git
> > clone --local', at first glance.
> 
> Actually I think I am wrong about this. The 3.5GB is due to 32 copies
> of the code, not the cloning. In fact --local is the default and that
> is what buildman seems to use.
> 
> So in fact I'm not sure we can change it. Each thread can be building
> a different commit.

Ah, OK, oh well.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160923/571e8779/attachment.sig>

[U-Boot] [PATCH 05/13] patman: Flush output when there is no newline

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> Output which does not include a newline will not be displayed unless
> flushed. Add a flush to ensure that it becomes visible.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/patman/terminal.py | 2 ++
>  1 file changed, 2 insertions(+)

Applied to u-boot-dm.

[U-Boot] [PATCH 06/13] buildman: Tidy up the 'cloning' message

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> On a machine with a lot of CPUs this prints a lot of useless lines of the
> form:
>
>    Cloning repo for thread <n>
>
> Adjust the output so that these all appear on one line, and disappear when
> the cloning is complete.
>
> Note: This cloning is actually unnecessary and very wasteful on disk space
> (about 3.5GB each time). It would be better to create symlinks.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/builder.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 07/13] buildman: Print a message when removing old directories

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> When buildman starts, it prepares its output directory by removing any old
> build directories which will not be used this time. This can happen if a
> previous build left directories around for commit hashes which are no-longer
> part of the branch.
>
> This can take quite a while, so print a message to indicate what is going
> on.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/builder.py | 6 ++++++
>  1 file changed, 6 insertions(+)

Applied to u-boot-dm.

[U-Boot] [PATCH 08/13] buildman: Print a message indicating the build is starting

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> Make it clear when buildman actually starts building. This happens when it
> has prepared the threads, working directory and output directories.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/builder.py | 1 +
>  tools/buildman/test.py    | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 09/13] buildman: Put our local libraries first in the path

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> If patman is installed on the machine (e.g. in the standard dist-packages
> directory), it will find libraries from there in preference to our local
> libraries. Adjust the order of the path to ensure that local libraries are
> found first.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/buildman.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 10/13] buildman: Allow builds to terminate cleanly

[Simon Glass]

Hi Tom,

On 19 September 2016 at 13:16, Tom Rini <trini@konsulko.com> wrote:
> On Sun, Sep 18, 2016 at 04:48:35PM -0600, Simon Glass wrote:
>
>> It is annoying that buildman does not respond cleanly to Ctrl-C or SIGINT,
>> particularly on machines with lots of CPUS. Unfortunately queue.join()
>> blocks the main thread and does not allow it to see the signal. Use a
>> separate thread instead,
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Yay for fixing this, killing off the running make's when I have a bad
> test build going is one of the minor pitas in my workflow :)

Yes, me too.

Applied to u-boot-dm.

[U-Boot] [PATCH 11/13] buildman: Drop the 'active' flag in the builder

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> This serves no real purpose, since when we are not active, we exit. Drop it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/builder.py       |  7 -------
>  tools/buildman/builderthread.py | 10 +---------
>  2 files changed, 1 insertion(+), 16 deletions(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 12/13] buildman: Don't show a stacktrace on Ctrl-C

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> When Ctrl-C is pressed, just exited quietly. There is no sense in displaying
> a stack trace since buildman will always be in the same place: waiting for
> threads to complete building all the jobs on the queue.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/builder.py | 7 +++++++
>  1 file changed, 7 insertions(+)

Applied to u-boot-dm.

[U-Boot] [PATCH 13/13] buildman: Drop the 'alive' flag in BuilderThread

[Simon Glass]

On 18 September 2016 at 16:48, Simon Glass <sjg@chromium.org> wrote:
> This is not used, so drop it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  tools/buildman/builderthread.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Applied to u-boot-dm.