[U-Boot] [PATCH 00/13] buildman: Threading and output improvements
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(-)
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 $@
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
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 =============
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
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"""
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.
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.
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?
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
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.
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
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.
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.
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):
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.
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()
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.
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
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.
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()
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 :)
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.
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()
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.
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):
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.
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()
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.
participants (2)
-
Simon Glass -
Tom Rini