At present CONFIG options are split across Kconfig and board config headers files. Also we have multiple files containing these CONFIG options.

In order to see exactly what is being used for building, create a .cfg file which holds these options as reported by the C preprocessor.

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

Makefile | 10 +++++++++- scripts/Makefile.spl | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile index 92faed6..737c0ba 100644 --- a/Makefile +++ b/Makefile @@ -708,7 +708,7 @@ DO_STATIC_RELA = endif

# Always append ALL so that arch config.mk's can add custom ones -ALL-y += u-boot.srec u-boot.bin System.map binary_size_check +ALL-y += u-boot.srec u-boot.bin System.map u-boot.cfg binary_size_check

ALL-$(CONFIG_ONENAND_U_BOOT) += u-boot-onenand.bin ifeq ($(CONFIG_SPL_FSL_PBL),y) @@ -849,6 +849,11 @@ ifndef CONFIG_SYS_UBOOT_START CONFIG_SYS_UBOOT_START := 0 endif

+# Create a file containing the configuration options the image was built with +quiet_cmd_cpp_cfg = CFG $@ +cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \ + -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $< + MKIMAGEFLAGS_u-boot.img = -A $(ARCH) -T firmware -C none -O u-boot \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \ -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" @@ -873,6 +878,9 @@ u-boot.sha1: u-boot.bin u-boot.dis: u-boot $(OBJDUMP) -d $< > $@

+u-boot.cfg: include/config.h + $(call if_changed,cpp_cfg) + ifdef CONFIG_TPL SPL_PAYLOAD := tpl/u-boot-with-tpl.bin else diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index ecf3037..d832c52 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -146,7 +146,7 @@ endif boot.bin: $(obj)/u-boot-spl.bin $(call if_changed,mkimage)

-ALL-y += $(obj)/$(SPL_BIN).bin +ALL-y += $(obj)/$(SPL_BIN).bin $(obj)/$(SPL_BIN).cfg

ifdef CONFIG_SAMSUNG ALL-y += $(obj)/$(BOARD)-spl.bin @@ -164,6 +164,13 @@ endif

all: $(ALL-y)

+quiet_cmd_cpp_cfg = CFG $@ +cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \ + -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $< + +$(obj)/$(SPL_BIN).cfg: include/config.h + $(call if_changed,cpp_cfg) + ifdef CONFIG_SAMSUNG ifdef CONFIG_VAR_SIZE_SPL VAR_SIZE_PARAM = --vs

Tweak the output slightly so we don't get things like:

- board1 board2+ board3 board4

There should be a space before the '+'.

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

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

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 1b0ad99..54f3292 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -664,7 +664,7 @@ class Builder: arch = 'unknown' str = self.col.Color(color, ' ' + target) if not arch in done_arch: - str = self.col.Color(color, char) + ' ' + str + str = ' %s %s' % (self.col.Color(color, char), str) done_arch[arch] = True if not arch in arch_list: arch_list[arch] = str diff --git a/tools/buildman/test.py b/tools/buildman/test.py index c0ad5d0..7642d94 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -169,7 +169,7 @@ class TestBuild(unittest.TestCase): expected_colour = col.GREEN if ok else col.RED expect = '%10s: ' % arch # TODO(sjg@chromium.org): If plus is '', we shouldn't need this - expect += col.Color(expected_colour, plus) + expect += ' ' + col.Color(expected_colour, plus) expect += ' ' for board in boards: expect += col.Color(expected_colour, ' %s' % board)

When a verbose build it selected, show the make command before the output of that command.

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

tools/buildman/builder.py | 3 +++ tools/buildman/builderthread.py | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 54f3292..72353b9 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -335,6 +335,9 @@ class Builder: cmd = [self.gnu_make] + list(args) result = command.RunPipe([cmd], capture=True, capture_stderr=True, cwd=cwd, raise_on_error=False, **kwargs) + if self.verbose_build: + result.stdout = '%s\n' % (' '.join(cmd)) + result.stdout + result.combined = '%s\n' % (' '.join(cmd)) + result.combined return result

def ProcessResult(self, result): diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index efb62f1..6ad240d 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -209,14 +209,17 @@ class BuilderThread(threading.Thread): if do_config: result = self.Make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) + config_out = result.combined result = self.Make(commit, brd, 'config', cwd, *(args + config_args), env=env) - config_out = result.combined + config_out += result.combined do_config = False # No need to configure next time if result.return_code == 0: result = self.Make(commit, brd, 'build', cwd, *args, env=env) result.stderr = result.stderr.replace(src_dir + '/', '') + if self.builder.verbose_build: + result.stdout = config_out + result.stdout else: result.return_code = 1 result.stderr = 'No tool chain for %s\n' % brd.arch

At present buildman tries to detect an aborted build and doesn't record a result in that case. This is to make sure that an abort (e.g. with Ctrl-C) does not mark the build as done. Without this option, buildman would never retry the build unless -f/-F are provided. The effect is that aborting the build creates 'fake errors' on whatever builds buildman happens to be working on at the time.

Unfortunately the current test is not reliable and this detection can trigger if a required toolchain tool is missing. In this case the toolchain problem is never reported.

Adjust the logic to continue processing the build result, mark the build as done (and failed), but with a return code which indicates that it should be retried.

The correct fix is to fully and correctly detect an aborted build, quit buildman immediately and not write any partial build results in this case. Unfortunately this is currently beyond my powers and is left as an exercise for the reader (and patches are welcome).

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

tools/buildman/builderthread.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 6ad240d..bd8635c 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -12,6 +12,8 @@ import threading import command import gitutil

+RETURN_CODE_RETRY = -1 + def Mkdir(dirname, parents = False): """Make a directory if it doesn't already exist.

@@ -145,7 +147,11 @@ class BuilderThread(threading.Thread): # Get the return code from that build and use it with open(done_file, 'r') as fd: result.return_code = int(fd.readline()) - if will_build: + + # Check the signal that the build needs to be retried + if result.return_code == RETURN_CODE_RETRY: + will_build = True + elif will_build: 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' @@ -243,9 +249,10 @@ class BuilderThread(threading.Thread): if result.return_code < 0: return

- # Aborted? - if result.stderr and 'No child processes' in result.stderr: - return + # If we think this might have been aborted with Ctrl-C, record the + # failure but not that we are 'done' with this board. A retry may fix + # it. + maybe_aborted = result.stderr and 'No child processes' in result.stderr

if result.already_done: return @@ -275,7 +282,11 @@ class BuilderThread(threading.Thread): done_file = self.builder.GetDoneFile(result.commit_upto, result.brd.target) with open(done_file, 'w') as fd: - fd.write('%s' % result.return_code) + if maybe_aborted: + # Special code to indicate we need to retry + fd.write('%s' % RETURN_CODE_RETRY) + else: + fd.write('%s' % result.return_code) with open(os.path.join(build_dir, 'toolchain'), 'w') as fd: print >>fd, 'gcc', result.toolchain.gcc print >>fd, 'path', result.toolchain.path

Store all config file output so that we can compare changes if requested.

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

tools/buildman/builderthread.py | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index bd8635c..7384a72 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -345,16 +345,38 @@ class BuilderThread(threading.Thread): with open(sizes, 'w') as fd: print >>fd, '\n'.join(lines)

+ # Write out the configuration files, with a special case for SPL + for dirname in ['', 'spl', 'tpl']: + self.CopyFiles(result.out_dir, build_dir, dirname, ['u-boot.cfg', + 'spl/u-boot-spl.cfg', 'tpl/u-boot-tpl.cfg', '.config', + 'include/autoconf.mk', 'include/generated/autoconf.h']) + # Now write the actual build output if keep_outputs: - patterns = ['u-boot', '*.bin', 'u-boot.dtb', '*.map', '*.img', - 'include/autoconf.mk', 'spl/u-boot-spl', - 'spl/u-boot-spl.bin'] - for pattern in patterns: - file_list = glob.glob(os.path.join(result.out_dir, pattern)) - for fname in file_list: - shutil.copy(fname, build_dir) + self.CopyFiles(result.out_dir, build_dir, '', ['u-boot', '*.bin', + 'u-boot.dtb', '*.map', '*.img', + 'spl/u-boot-spl', 'spl/u-boot-spl.bin', + 'tpl/u-boot-tpl', 'tpl/u-boot-tpl.bin']) + + def CopyFiles(self, out_dir, build_dir, dirname, patterns): + """Copy files from the build directory to the output.

+ Args: + out_dir: Path to output directory containing the files + build_dir: Place to copy the files + dirname: Source directory, '' for normal U-Boot, 'spl' for SPL + patterns: A list of filenames (strings) to copy, each relative + to the build directory + """ + for pattern in patterns: + file_list = glob.glob(os.path.join(out_dir, dirname, pattern)) + for fname in file_list: + target = os.path.basename(fname) + if dirname: + base, ext = os.path.splitext(target) + if ext: + target = '%s-%s%s' % (base, dirname, ext) + shutil.copy(fname, os.path.join(build_dir, target))

def RunJob(self, job): """Run a single job

It is useful to be able to see CONFIG changes made by commits. Add this feature to buildman using the -K flag so that all CONFIG changes are reported.

The CONFIG options exist in a number of files. Each is reported individually as well as a summary that covers all files. The output shows three parts: green for additions, red for removals and yellow for changes.

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

tools/buildman/builder.py | 181 ++++++++++++++++++++++++++++++++++++++++++---- tools/buildman/cmdline.py | 4 +- tools/buildman/control.py | 3 +- 3 files changed, 173 insertions(+), 15 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 72353b9..c26e2b6 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -96,6 +96,13 @@ 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 = [ + '.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 Builder: """Class for building U-Boot for a particular commit. @@ -166,12 +173,17 @@ class Builder: value is itself a dictionary: key: function name value: Size of function in bytes + config: Dictionary keyed by filename - e.g. '.config'. Each + value is itself a dictionary: + key: config name + value: config value """ - def __init__(self, rc, err_lines, sizes, func_sizes): + def __init__(self, rc, err_lines, sizes, func_sizes, config): self.rc = rc self.err_lines = err_lines self.sizes = sizes self.func_sizes = func_sizes + self.config = config

def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, @@ -254,7 +266,7 @@ class Builder:

def SetDisplayOptions(self, show_errors=False, show_sizes=False, show_detail=False, show_bloat=False, - list_error_boards=False): + list_error_boards=False, show_config=False): """Setup display options for the builder.

show_errors: True to show summarised error/warning info @@ -262,12 +274,14 @@ class Builder: show_detail: Show detail for each board show_bloat: Show detail for each function list_error_boards: Show the boards which caused each error/warning + show_config: Show config deltas """ self._show_errors = show_errors self._show_sizes = show_sizes self._show_detail = show_detail self._show_bloat = show_bloat self._list_error_boards = list_error_boards + self._show_config = show_config

def _AddTimestamp(self): """Add a new timestamp to the list and record the build period. @@ -519,13 +533,50 @@ class Builder: sym[name] = sym.get(name, 0) + int(size, 16) return sym

- def GetBuildOutcome(self, commit_upto, target, read_func_sizes): + def _ProcessConfig(self, fname): + """Read in a .config, autoconf.mk or autoconf.h file + + This function handles all config file types. It ignores comments and + any #defines which don't start with CONFIG_. + + Args: + fname: Filename to read + + Returns: + Dictionary: + key: Config name (e.g. CONFIG_DM) + value: Config value (e.g. 1) + """ + config = {} + if os.path.exists(fname): + with open(fname) as fd: + for line in fd: + line = line.strip() + if line.startswith('#define'): + values = line[8:].split(' ', 1) + if len(values) > 1: + key, value = values + else: + key = values[0] + value = '' + if not key.startswith('CONFIG_'): + continue + elif not line or line[0] in ['#', '*', '/']: + continue + else: + key, value = line.split('=', 1) + config[key] = value + return config + + def GetBuildOutcome(self, commit_upto, target, read_func_sizes, + read_config): """Work out the outcome of a build.

Args: commit_upto: Commit number to check (0..n-1) target: Target board to check read_func_sizes: True to read function size information + read_config: True to read .config and autoconf.h files

Returns: Outcome object @@ -534,6 +585,7 @@ class Builder: sizes_file = self.GetSizesFile(commit_upto, target) sizes = {} func_sizes = {} + config = {} if os.path.exists(done_file): with open(done_file, 'r') as fd: return_code = int(fd.readline()) @@ -577,17 +629,25 @@ class Builder: '') func_sizes[dict_name] = self.ReadFuncSizes(fname, fd)

- return Builder.Outcome(rc, err_lines, sizes, func_sizes) + if read_config: + output_dir = self.GetBuildDir(commit_upto, target) + for name in CONFIG_FILENAMES: + fname = os.path.join(output_dir, name) + config[name] = self._ProcessConfig(fname) + + return Builder.Outcome(rc, err_lines, sizes, func_sizes, config)

- return Builder.Outcome(OUTCOME_UNKNOWN, [], {}, {}) + return Builder.Outcome(OUTCOME_UNKNOWN, [], {}, {}, {})

- def GetResultSummary(self, boards_selected, commit_upto, read_func_sizes): + def GetResultSummary(self, boards_selected, commit_upto, read_func_sizes, + read_config): """Calculate a summary of the results of building a commit.

Args: board_selected: Dict containing boards to summarise commit_upto: Commit number to summarize (0..self.count-1) read_func_sizes: True to read function size information + read_config: True to read .config and autoconf.h files

Returns: Tuple: @@ -599,6 +659,10 @@ class Builder: List containing a summary of warning lines Dict keyed by error line, containing a list of the Board objects with that warning + Dictionary keyed by filename - e.g. '.config'. Each + value is itself a dictionary: + key: config name + value: config value """ def AddLine(lines_summary, lines_boards, line, board): line = line.rstrip() @@ -613,10 +677,13 @@ class Builder: err_lines_boards = {} warn_lines_summary = [] warn_lines_boards = {} + config = {} + for fname in CONFIG_FILENAMES: + config[fname] = {}

for board in boards_selected.itervalues(): outcome = self.GetBuildOutcome(commit_upto, board.target, - read_func_sizes) + read_func_sizes, read_config) board_dict[board.target] = outcome last_func = None last_was_warning = False @@ -642,8 +709,14 @@ class Builder: line, board) last_was_warning = is_warning last_func = None + for fname in CONFIG_FILENAMES: + config[fname] = {} + if outcome.config: + for key, value in outcome.config[fname].iteritems(): + config[fname][key] = value + return (board_dict, err_lines_summary, err_lines_boards, - warn_lines_summary, warn_lines_boards) + warn_lines_summary, warn_lines_boards, config)

def AddOutcome(self, board_dict, arch_list, changes, char, color): """Add an output to our list of outcomes for each architecture @@ -696,11 +769,14 @@ class Builder: """ self._base_board_dict = {} for board in board_selected: - self._base_board_dict[board] = Builder.Outcome(0, [], [], {}) + self._base_board_dict[board] = Builder.Outcome(0, [], [], {}, {}) self._base_err_lines = [] self._base_warn_lines = [] self._base_err_line_boards = {} self._base_warn_line_boards = {} + self._base_config = {} + for fname in CONFIG_FILENAMES: + self._base_config[fname] = {}

def PrintFuncSizeDetail(self, fname, old, new): grow, shrink, add, remove, up, down = 0, 0, 0, 0, 0, 0 @@ -895,7 +971,8 @@ class Builder:

def PrintResultSummary(self, board_selected, board_dict, err_lines, err_line_boards, warn_lines, warn_line_boards, - show_sizes, show_detail, show_bloat): + config, show_sizes, show_detail, show_bloat, + show_config): """Compare results with the base results and display delta.

Only boards mentioned in board_selected will be considered. This @@ -916,9 +993,14 @@ class Builder: none, or we don't want to print errors warn_line_boards: Dict keyed by warning line, containing a list of the Board objects with that warning + config: Dictionary keyed by filename - e.g. '.config'. Each + value is itself a dictionary: + key: config name + value: config value show_sizes: Show image size deltas show_detail: Show detail for each board show_bloat: Show detail for each function + show_config: Show config changes """ def _BoardList(line, line_boards): """Helper function to get a line of boards containing a line @@ -953,6 +1035,48 @@ class Builder: _BoardList(line, base_line_boards) + line) return better_lines, worse_lines

+ def _CalcConfig(delta, name, config): + """Calculate configuration changes + + Args: + delta: Type of the delta, e.g. '+' + name: name of the file which changed (e.g. .config) + config: configuration change dictionary + key: config name + value: config value + Returns: + String containing the configuration changes which can be + printed + """ + out = '' + for key in sorted(config.keys()): + out += '%s=%s ' % (key, config[key]) + return '%5s %s: %s' % (delta, name, out) + + def _ShowConfig(name, config_plus, config_minus, config_change): + """Show changes in configuration + + Args: + config_plus: configurations added, dictionary + key: config name + value: config value + config_minus: configurations removed, dictionary + key: config name + value: config value + config_change: configurations changed, dictionary + key: config name + value: config value + """ + if config_plus: + Print(_CalcConfig('+', name, config_plus), + colour=self.col.GREEN) + if config_minus: + Print(_CalcConfig('-', name, config_minus), + colour=self.col.RED) + if config_change: + Print(_CalcConfig('+/-', name, config_change), + colour=self.col.YELLOW) + better = [] # List of boards fixed since last commit worse = [] # List of new broken boards since last commit new = [] # List of boards that didn't exist last time @@ -1013,12 +1137,41 @@ class Builder: self.PrintSizeSummary(board_selected, board_dict, show_detail, show_bloat)

+ if show_config: + all_config_plus = {} + all_config_minus = {} + all_config_change = {} + for name in CONFIG_FILENAMES: + if not config[name]: + continue + config_plus = {} + config_minus = {} + config_change = {} + base = self._base_config[name] + for key, value in config[name].iteritems(): + if key not in base: + config_plus[key] = value + all_config_plus[key] = value + for key, value in base.iteritems(): + if key not in config[name]: + config_minus[key] = value + all_config_minus[key] = value + for key, value in base.iteritems(): + new_value = base[key] + if key in config[name] and value != new_value: + desc = '%s -> %s' % (value, new_value) + config_change[key] = desc + all_config_change[key] = desc + _ShowConfig(name, config_plus, config_minus, config_change) + _ShowConfig('all', config_plus, config_minus, config_change) + # Save our updated information for the next call to this function self._base_board_dict = board_dict self._base_err_lines = err_lines self._base_warn_lines = warn_lines self._base_err_line_boards = err_line_boards self._base_warn_line_boards = warn_line_boards + self._base_config = config

# Get a list of boards that did not get built, if needed not_built = [] @@ -1031,9 +1184,10 @@ class Builder:

def ProduceResultSummary(self, commit_upto, commits, board_selected): (board_dict, err_lines, err_line_boards, warn_lines, - warn_line_boards) = self.GetResultSummary( + warn_line_boards, config) = self.GetResultSummary( board_selected, commit_upto, - read_func_sizes=self._show_bloat) + read_func_sizes=self._show_bloat, + read_config=self._show_config) if commits: msg = '%02d: %s' % (commit_upto + 1, commits[commit_upto].subject) @@ -1041,7 +1195,8 @@ class Builder: self.PrintResultSummary(board_selected, board_dict, err_lines if self._show_errors else [], err_line_boards, warn_lines if self._show_errors else [], warn_line_boards, - self._show_sizes, self._show_detail, self._show_bloat) + config, self._show_sizes, self._show_detail, + self._show_bloat, self._show_config)

def ShowSummary(self, commits, board_selected): """Show a build summary for U-Boot for a given board list. diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index e8a6dad..916ea57 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -16,7 +16,7 @@ def ParseArgs(): """ parser = OptionParser() parser.add_option('-b', '--branch', type='string', - help='Branch name to build') + help='Branch name to build, or range of commits to build') parser.add_option('-B', '--bloat', dest='show_bloat', action='store_true', default=False, help='Show changes in function code size for each board') @@ -53,6 +53,8 @@ def ParseArgs(): default=None, help='Number of jobs to run at once (passed to make)') parser.add_option('-k', '--keep-outputs', action='store_true', 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('-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 720b978..8b3cd30 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -282,7 +282,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, options.show_detail = True builder.SetDisplayOptions(options.show_errors, options.show_sizes, options.show_detail, options.show_bloat, - options.list_error_boards) + options.list_error_boards, + options.show_config) if options.summary: builder.ShowSummary(commits, board_selected) else: