At present buildman defaults to running 'mrproper' on every thread before
it starts building commits for each board. This can add a delay of about 5
seconds to the start of the process, since the tools and other invariants
must be rebuilt.
In particular, a build without '-b', to build current source, runs much
slower without -I, since any existing build is removed, thus losing the
possibility of an incremental build.
Partly this behaviour was to avoid strange build-system problems caused by
running 'make defconfig' for one board and then one with a different
architecture. But these problems were fixed quite a while ago.
The -I option (which disabled mrproper) was introduced four years ago and
does not seem to cause any problems with builds.
So make -I the default and deprecate the option. To allow use of
'mrproper', add a new -m flag.
Signed-off-by: Simon Glass <s...@chromium.org>
---
Changes in v2: None
tools/buildman/README | 13 ++++++-------
tools/buildman/builder.py | 7 +++----
tools/buildman/builderthread.py | 6 +++---
tools/buildman/cmdline.py | 5 ++++-
tools/buildman/control.py | 6 +++++-
tools/buildman/func_test.py | 28 ++++++++++++++++++++--------
6 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/tools/buildman/README b/tools/buildman/README
index ca0d1f64460..f299b0c2972 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -958,12 +958,11 @@ will build commits in us-buildman that are not in
upstream/master.
Building Faster
===============
-By default, buildman executes 'make mrproper' prior to building the first
-commit for each board. This causes everything to be built from scratch. If you
-trust the build system's incremental build capabilities, you can pass the -I
-flag to skip the 'make mproper' invocation, which will reduce the amount of
-work 'make' does, and hence speed up the build. This flag will speed up any
-buildman invocation, since it reduces the amount of work done on any build.
+By default, buildman doesn't execute 'make mrproper' prior to building the
+first commit for each board. This reduces the amount of work 'make' does, and
+hence speeds up the build. To force use of 'make mrproper', use -the -m flag.
+This flag will slow down any buildman invocation, since it increases the amount
+of work done on any build.
One possible application of buildman is as part of a continual edit, build,
edit, build, ... cycle; repeatedly applying buildman to the same change or
@@ -994,7 +993,7 @@ Combining all of these options together yields the
command-line shown below.
This will provide the quickest possible feedback regarding the current content
of the source tree, thus allowing rapid tested evolution of the code.
- SOURCE_DATE_EPOCH=0 ./tools/buildman/buildman -I -P tegra
+ SOURCE_DATE_EPOCH=0 ./tools/buildman/buildman -P tegra
Checking configuration
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 6819e5b40da..597a03ffb08 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -231,7 +231,7 @@ 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,
+ mrproper=False, per_board_out_dir=False,
config_only=False, squash_config_y=False,
warnings_as_errors=False, work_in_output=False):
"""Create a new Builder object
@@ -252,8 +252,7 @@ class Builder:
full_path: Return the full path in CROSS_COMPILE and don't set
PATH
verbose_build: Run build with V=1 and don't use 'make -s'
- incremental: Always perform incremental builds; don't run make
- mrproper when configuring
+ mrproper: Always run 'make 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
@@ -311,7 +310,7 @@ class Builder:
self.queue = queue.Queue()
self.out_queue = queue.Queue()
for i in range(self.num_threads):
- t = builderthread.BuilderThread(self, i, incremental,
+ t = builderthread.BuilderThread(self, i, mrproper,
per_board_out_dir)
t.setDaemon(True)
t.start()
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 7561f399428..fc6e1ab25da 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -90,12 +90,12 @@ class BuilderThread(threading.Thread):
thread_num: Our thread number (0-n-1), used to decide on a
temporary directory
"""
- def __init__(self, builder, thread_num, incremental, per_board_out_dir):
+ def __init__(self, builder, thread_num, mrproper, per_board_out_dir):
"""Set up a new builder thread"""
threading.Thread.__init__(self)
self.builder = builder
self.thread_num = thread_num
- self.incremental = incremental
+ self.mrproper = mrproper
self.per_board_out_dir = per_board_out_dir
def Make(self, commit, brd, stage, cwd, *args, **kwargs):
@@ -243,7 +243,7 @@ class BuilderThread(threading.Thread):
# If we need to reconfigure, do that now
if do_config:
config_out = ''
- if not self.incremental:
+ if self.mrproper:
result = self.Make(commit, brd, 'mrproper', cwd,
'mrproper', *args, env=env)
config_out += result.combined
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 17ea015a956..0178c6e8845 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -55,8 +55,9 @@ def ParseArgs():
parser.add_option('-i', '--in-tree', dest='in_tree',
action='store_true', default=False,
help='Build in the source tree instead of a separate directory')
+ # -I will be removed after April 2021
parser.add_option('-I', '--incremental', action='store_true',
- default=False, help='Do not run make mrproper (when reconfiguring)')
+ default=False, help='Deprecated, does nothing. See -m')
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',
@@ -69,6 +70,8 @@ def ParseArgs():
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 (use -v to see probing detail)')
+ parser.add_option('-m', '--mrproper', action='store_true',
+ default=False, help="Run 'make mrproper before reconfiguring")
parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help="Do a dry run (describe actions, but do
nothing)")
parser.add_option('-N', '--no-subdirs', action='store_true',
dest='no_subdirs',
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 5ddc598c952..384e62dbc56 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -172,6 +172,10 @@ def DoBuildman(options, args, toolchains=None,
make_func=None, boards=None,
print()
return 0
+ if options.incremental:
+ print(col.Color(col.RED,
+ 'Warning: -I has been removed. See documentation'))
+
# Work out what subset of the boards we are building
if not boards:
if not os.path.exists(options.output_dir):
@@ -309,7 +313,7 @@ def DoBuildman(options, args, toolchains=None,
make_func=None, boards=None,
show_unknown=options.show_unknown, step=options.step,
no_subdirs=options.no_subdirs, full_path=options.full_path,
verbose_build=options.verbose_build,
- incremental=options.incremental,
+ mrproper=options.mrproper,
per_board_out_dir=options.per_board_out_dir,
config_only=options.config_only,
squash_config_y=not options.preserve_config_y,
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 2a256a92639..b9e347ecb01 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -476,15 +476,15 @@ class TestFunctional(unittest.TestCase):
self._RunControl('-b', TEST_BRANCH, '-c2', '-o', self._output_dir)
self.assertEqual(self._builder.count, 2 * len(boards))
self.assertEqual(self._builder.fail, 0)
- # Each board has a mrproper, config, and then one make per commit
- self.assertEqual(self._make_calls, len(boards) * (2 + 2))
+ # Each board has a config, and then one make per commit
+ self.assertEqual(self._make_calls, len(boards) * (1 + 2))
def testIncremental(self):
"""Test building a branch twice - the second time should do nothing"""
self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir)
# Each board has a mrproper, config, and then one make per commit
- self.assertEqual(self._make_calls, len(boards) * (self._commits + 2))
+ self.assertEqual(self._make_calls, len(boards) * (self._commits + 1))
self._make_calls = 0
self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir,
clean_dir=False)
self.assertEqual(self._make_calls, 0)
@@ -496,14 +496,26 @@ class TestFunctional(unittest.TestCase):
self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir)
self._make_calls = 0
self._RunControl('-b', TEST_BRANCH, '-f', '-o', self._output_dir,
clean_dir=False)
- # Each board has a mrproper, config, and then one make per commit
- self.assertEqual(self._make_calls, len(boards) * (self._commits + 2))
+ # Each board has a config and one make per commit
+ self.assertEqual(self._make_calls, len(boards) * (self._commits + 1))
def testForceReconfigure(self):
"""The -f flag should force a rebuild"""
self._RunControl('-b', TEST_BRANCH, '-C', '-o', self._output_dir)
- # Each commit has a mrproper, config and make
- self.assertEqual(self._make_calls, len(boards) * self._commits * 3)
+ # Each commit has a config and make
+ self.assertEqual(self._make_calls, len(boards) * self._commits * 2)
+
+ def testForceReconfigure(self):
+ """The -f flag should force a rebuild"""
+ self._RunControl('-b', TEST_BRANCH, '-C', '-o', self._output_dir)
+ # Each commit has a config and make
+ self.assertEqual(self._make_calls, len(boards) * self._commits * 2)
+
+ def testMrproper(self):
+ """The -f flag should force a rebuild"""
+ self._RunControl('-b', TEST_BRANCH, '-m', '-o', self._output_dir)
+ # Each board has a mkproper, config and then one make per commit
+ self.assertEqual(self._make_calls, len(boards) * (self._commits + 2))
def testErrors(self):
"""Test handling of build errors"""
@@ -525,7 +537,7 @@ class TestFunctional(unittest.TestCase):
self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir, '-F',
clean_dir=False)
self.assertEqual(self._builder.count, self._total_builds)
self.assertEqual(self._builder.fail, 0)
- self.assertEqual(self._make_calls, 3)
+ self.assertEqual(self._make_calls, 2)
def testBranchWithSlash(self):
"""Test building a branch with a '/' in the name"""
--
2.26.0.110.g2183baf09c-goog
