[U-Boot] [PATCH 0/6] Add some missing buildman features and deprecate MAKEALL

Buildman has been around for a little over a year and is used by a fair number of U-Boot developers. However quite a few people still use MAKEALL.
Buildman was intended to replace MAKEALL, so perhaps now is a good time to start that process.
The reasons to deprecate MAKEALL are: - We don't want to maintain two build systems - Buildman is typically faster - Buildman has a lot more features
This series adds a few features to buildman to fill some gaps, adds some infomation into the README on how to migrate from MAKEALL, and adds a deprecation message to MAKEALL.
Simon Glass (6): buildman: Add -F flag to retry failed builds buildman: Avoid retrying a build if it definitely failed buildman: Add -C option to force a reconfigure for each commit buildman: Support in-tree builds buildman: Add some notes about moving from MAKEALL RFC: Deprecate MAKEALL
MAKEALL | 4 ++ tools/buildman/README | 92 ++++++++++++++++++++++++++++++++++++++++++++++ tools/buildman/builder.py | 51 ++++++++++++++++++++----- tools/buildman/buildman.py | 9 +++++ tools/buildman/control.py | 3 ++ 5 files changed, 150 insertions(+), 9 deletions(-)

Generally a build failure with a particular commit cannot be fixed except by changing that commit. Changing the commit will automatically cause buildman to retry when you run it again: buildman sees that the commit hash is different and that it has no previous build result for the new commit hash.
However sometimes the build failure is due to a toolchain issue or some other environment problem. In that case, retrying failed builds may yield a different result.
Add a flag to retry failed builds. This differs from the force rebuild flag (-f) in that it will not rebuild commits which are already marked as succeeded.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/buildman/builder.py | 23 ++++++++++++++++++----- tools/buildman/buildman.py | 3 +++ tools/buildman/control.py | 1 + 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 4a2d753..2990c45 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -188,7 +188,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): + def RunCommit(self, commit_upto, brd, work_dir, do_config, force_build, + force_build_failures): """Build a particular commit.
If the build is already done, and we are not forcing a build, we skip @@ -200,6 +201,8 @@ class BuilderThread(threading.Thread): work_dir: Directory to which the source will be checked out do_config: True to run a make <board>_config on 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
Returns: tuple containing: @@ -215,14 +218,20 @@ class BuilderThread(threading.Thread): # Check if the job was already completed last time done_file = self.builder.GetDoneFile(commit_upto, brd.target) result.already_done = os.path.exists(done_file) - if result.already_done and not force_build: + will_build = (force_build or force_build_failures or + not result.already_done) + if result.already_done and will_build: # Get the return code from that build and use it with open(done_file, 'r') as fd: result.return_code = int(fd.readline()) err_file = self.builder.GetErrFile(commit_upto, brd.target) if os.path.exists(err_file) and os.stat(err_file).st_size: result.stderr = 'bad' - else: + elif not force_build: + # The build passed, so no need to build it again + will_build = False + + if will_build: # We are going to have to build it. First, get a toolchain if not self.toolchain: try: @@ -411,14 +420,15 @@ class BuilderThread(threading.Thread): for commit_upto in range(0, len(job.commits), job.step): result, request_config = self.RunCommit(commit_upto, brd, work_dir, do_config, - force_build or self.builder.force_build) + force_build or self.builder.force_build, + self.builder.force_build_failures) failed = result.return_code or result.stderr if failed and not do_config: # If our incremental build failed, try building again # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.RunCommit(commit_upto, - brd, work_dir, True, True) + brd, work_dir, True, True, False) do_config = request_config
# If we built that commit, then config is done. But if we got @@ -498,6 +508,8 @@ class Builder: force_config_on_failure: If a commit fails for a board, disable incremental building for the next commit we build for that board, so that we will see all warnings/errors again. + force_build_failures: If a previously-built build (i.e. built on + a previous run of buildman) is marked as failed, rebuild it. git_dir: Git directory containing source repository last_line_len: Length of the last line we printed (used for erasing it with new progress information) @@ -578,6 +590,7 @@ class Builder: self._complete_delay = None self._next_delay_update = datetime.now() self.force_config_on_failure = True + self.force_build_failures = False self._step = step
self.col = terminal.Color() diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index 73a5483..0da6797 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -72,6 +72,9 @@ parser.add_option('-e', '--show_errors', action='store_true', parser.add_option('-f', '--force-build', dest='force_build', action='store_true', default=False, help='Force build of boards even if already built') +parser.add_option('-F', '--force-build-failures', dest='force_build_failures', + action='store_true', default=False, + help='Force build of previously-failed build') parser.add_option('-d', '--detail', dest='show_detail', action='store_true', default=False, help='Show detailed information for each board in summary') diff --git a/tools/buildman/control.py b/tools/buildman/control.py index d2f4102..cfad535 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -156,6 +156,7 @@ def DoBuildman(options, args): ShowActions(series, why_selected, selected, builder, options) else: builder.force_build = options.force_build + builder.force_build_failures = options.force_build_failures
# Work out which boards to build board_selected = boards.GetSelectedDict()

After a build fails buildman will reconfigure and try again, if it did not reconfigure before the build. However it doesn't actually keep track of whether it did reconfigure on the previous attempt.
Fix that logic to avoid a pointless rebuild. This speeds things up quite a bit for failing builds. Previously they would always be built twice.
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 2990c45..39a6e8a 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -423,12 +423,14 @@ class BuilderThread(threading.Thread): force_build or self.builder.force_build, self.builder.force_build_failures) failed = result.return_code or result.stderr + did_config = do_config if failed and not do_config: # If our incremental build failed, try building again # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.RunCommit(commit_upto, brd, work_dir, True, True, False) + did_config = True do_config = request_config
# If we built that commit, then config is done. But if we got @@ -445,7 +447,7 @@ class BuilderThread(threading.Thread): # Of course this is substantially slower if there are build # errors/warnings (e.g. 2-3x slower even if only 10% of builds # have problems). - if (failed and not result.already_done and not do_config and + if (failed and not result.already_done and not did_config and self.builder.force_config_on_failure): # If this build failed, try the next one with a # reconfigure.

On 15 July 2014 00:51, Simon Glass sjg@chromium.org wrote:
After a build fails buildman will reconfigure and try again, if it did not reconfigure before the build. However it doesn't actually keep track of whether it did reconfigure on the previous attempt.
Fix that logic to avoid a pointless rebuild. This speeds things up quite a bit for failing builds. Previously they would always be built twice.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to dm/master.

Normally buildman wil try to configure U-Boot for a particular board on the first commit that it builds in a series. Subsequent commits are built without reconfiguring which normally works. Where it doesn't, buildman automatically reconfigures and retries.
To fully emulate the way MAKEALL works, we should have an option to disable this optimisation.
Add a -C option to cause buildman to always reconfigure on each commit.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/buildman/builder.py | 10 +++++++++- tools/buildman/buildman.py | 3 +++ tools/buildman/control.py | 1 + 3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 39a6e8a..767b1f6 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -431,7 +431,8 @@ class BuilderThread(threading.Thread): result, request_config = self.RunCommit(commit_upto, brd, work_dir, True, True, False) did_config = True - do_config = request_config + if not self.builder.force_reconfig: + do_config = request_config
# If we built that commit, then config is done. But if we got # an warning, reconfig next time to force it to build the same @@ -524,6 +525,12 @@ class Builder: toolchains: Toolchains object to use for building upto: Current commit number we are building (0.count-1) warned: Number of builds that produced at least one warning + force_reconfig: Reconfigure U-Boot on each comiit. This disables + incremental building, where buildman reconfigures on the first + commit for a baord, and then just does an incremental build for + the following commits. In fact buildman will reconfigure and + retry for any failing commits, so generally the only effect of + this option is to slow things down.
Private members: _base_board_dict: Last-summarised Dict of boards @@ -593,6 +600,7 @@ class Builder: self._next_delay_update = datetime.now() self.force_config_on_failure = True self.force_build_failures = False + self.force_reconfig = False self._step = step
self.col = terminal.Color() diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index 0da6797..5046b16 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -67,6 +67,9 @@ parser.add_option('-B', '--bloat', dest='show_bloat', help='Show changes in function code size for each board') parser.add_option('-c', '--count', dest='count', type='int', default=-1, help='Run build on the top n commits') +parser.add_option('-C', '--force-reconfig', dest='force_reconfig', + action='store_true', default=False, + help='Reconfigure for every commit (disable incremental build)') 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 cfad535..a737fd1 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -157,6 +157,7 @@ def DoBuildman(options, args): else: builder.force_build = options.force_build builder.force_build_failures = options.force_build_failures + builder.force_reconfig = options.force_reconfig
# Work out which boards to build board_selected = boards.GetSelectedDict()

At present buildman always builds out-of-tree, that is it uses a separate output directory from the source directory. Normally this is what you want, but it is important that in-tree builds work also. Some Makefile changes may break this.
Add a -i option to tell buildman to use in-tree builds, so that it is easy to test this feature.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/buildman/builder.py | 14 ++++++++++++-- tools/buildman/buildman.py | 3 +++ tools/buildman/control.py | 1 + 3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 767b1f6..0a3900c 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -213,7 +213,10 @@ class BuilderThread(threading.Thread): # self.Make() below, in the event that we do a build. result = command.CommandResult() result.return_code = 0 - out_dir = os.path.join(work_dir, 'build') + if self.builder.in_tree: + out_dir = work_dir + else: + out_dir = os.path.join(work_dir, 'build')
# Check if the job was already completed last time done_file = self.builder.GetDoneFile(commit_upto, brd.target) @@ -257,7 +260,10 @@ class BuilderThread(threading.Thread): # Set up the environment and command line env = self.toolchain.MakeEnvironment() Mkdir(out_dir) - args = ['O=build', '-s'] + args = [] + if not self.builder.in_tree: + args.append('O=build') + args.append('-s') if self.builder.num_jobs is not None: args.extend(['-j', str(self.builder.num_jobs)]) config_args = ['%s_config' % brd.target] @@ -531,6 +537,9 @@ class Builder: the following commits. In fact buildman will reconfigure and retry for any failing commits, so generally the only effect of this option is to slow things down. + in_tree: Build U-Boot in-tree instead of specifying an output + directory separate from the source code. This option is really + only useful for testing in-tree builds.
Private members: _base_board_dict: Last-summarised Dict of boards @@ -602,6 +611,7 @@ class Builder: self.force_build_failures = False self.force_reconfig = False self._step = step + self.in_tree = False
self.col = terminal.Color()
diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py index 5046b16..42847ac 100755 --- a/tools/buildman/buildman.py +++ b/tools/buildman/buildman.py @@ -85,6 +85,9 @@ parser.add_option('-g', '--git', type='string', help='Git repo containing branch to build', default='.') parser.add_option('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') +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') 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', diff --git a/tools/buildman/control.py b/tools/buildman/control.py index a737fd1..2dd8043 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -158,6 +158,7 @@ def DoBuildman(options, args): builder.force_build = options.force_build builder.force_build_failures = options.force_build_failures builder.force_reconfig = options.force_reconfig + builder.in_tree = options.in_tree
# Work out which boards to build board_selected = boards.GetSelectedDict()

For those used to MAKEALL, buildman seems strange. Add some notes to ease the transition.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/buildman/README | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)
diff --git a/tools/buildman/README b/tools/buildman/README index c30c1d4..0b141e4 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -3,6 +3,8 @@ # SPDX-License-Identifier: GPL-2.0+ #
+(Please read 'How to change from MAKEALL' if you are used to that tool) + What is this? =============
@@ -663,6 +665,96 @@ Other options Buildman has various other command line options. Try --help to see them.
+How to change from MAKEALL +========================== + +Buildman includes most of the features of MAKEALL, and is generally faster +and easier to use. In particular it builds entire branches - if a particular +commit introduces an error in a particular board, buildman can easily show +you this. + +The reasons to deprecate MAKEALL are: +- We don't want to maintain two buld systems +- Buildman is typically faster +- Buildman has a lot more features + +But still, many people will be sad to lose MAKEALL. If you are used to +MAKEALL, here are a few pointers. + +First you need to set up your tool chains - see the 'Setting up' section +for details. Once you have your required toolchain(s) detected then you are +ready to go. + +Buildman works on entire branches, so the normal use is: + + ./tools/buildman/buildman -b <branch_name> <list of things to build> + +followed by (afterwards, or perhaps concurrently in another terminal): + + ./tools/buildman/buildman -b <branch_name> -s <list of things to build> + +to see the results of the build. Rather than showing you all the output, +buildman just shows a summary with red indicating that a commit introduced +and error and green indicating that a commit fixed an error. Use the -e +flag to see the full errors. + +You don't need to stick around on that branch while buildman is running. It +checks out its own copy of the source code, so you can change branches, +add commits, etc. without affecting the build. + +The <list of things to build> can include board names, architectures or the +like. There are no flags to disambiguate since ambiguities are rare. Using +the examples from MAKEALL: + +Examples: + - build all Power Architecture boards: + MAKEALL -a powerpc + MAKEALL --arch powerpc + MAKEALL powerpc + ** buildman -b <branch> powerpc + - build all PowerPC boards manufactured by vendor "esd": + MAKEALL -a powerpc -v esd + ** buildman -b <branch> esd + - build all PowerPC boards manufactured either by "keymile" or "siemens": + MAKEALL -a powerpc -v keymile -v siemens + ** buildman -b <branch> keymile siemens + - build all Freescale boards with MPC83xx CPUs, plus all 4xx boards: + MAKEALL -c mpc83xx -v freescale 4xx + ** buildman -b <branch> mpc83xx freescale 4xx + +Buildman automatically tries to use all the CPUs in your machine. If you +are building a lot of boards it will use one thread for every CPU core +it detects in your machine. This is like MAKEALL's BUILD_NBUILDS option. +You can use the -T flag to change the number of threads. If you are only +building a few boards, buildman will automatically run make with the -j +flag to increase the number of concurrent make tasks. It isn't normally +that helpful to fiddle with this option, but if you use the BUILD_NCPUS +option in MAKEALL then -j is the equivalent in buildman. + +Buildman puts its output in ../<branch_name> but you can change this with +the -o option. Buildman normally does out-of-tree builds: use -i to disable +that if you really want to. But be careful that once you have used -i you +pollute buildman's copies of the source tree, and you will need to remove +the build directory (normally ../<branch_name>) to run buildman in normal +mode (without -i). + +Buildman doesn't keep the output result normally, but use the -k option to +do this. + +Please read 'Theory of Operation' a few times as it will make a lot of +things clearer. + +Some options you might like are: + + -B shows which functions are growing/shrinking in which commit - great + for finding code bloat. + -S shows image sizes for each commit (just an overall summary) + -u shows boards that you haven't built yet + --step 0 will build just the upstream commit and the last commit of your + branch. This is often a quick sanity check that your branch doesn't + break anything. But note this does not check bisectability! + + TODO ====

Since buildman now includes most of the features of MAKEALL it is proabably time to talk about deprecating MAKEALL.
Comments welcome.
Signed-off-by: Simon Glass sjg@chromium.org ---
MAKEALL | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/MAKEALL b/MAKEALL index 020e65f..6443814 100755 --- a/MAKEALL +++ b/MAKEALL @@ -60,6 +60,10 @@ usage() exit ${ret} }
+echo "** Note: MAKEALL is deprecated - please use buildman instead" +echo "** See tools/buildman/README for details" +echo + SHORT_OPTS="ha:c:v:s:b:lmMCnr" LONG_OPTS="help,arch:,cpu:,vendor:,soc:,board:,list,maintainers,mails,check,continue,rebuild-errors"
participants (1)
-
Simon Glass