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 sjg@chromium.org ---
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 caa98e0ebbb..43e667c89f9 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"""