All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits
@ 2015-02-06  5:06 Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build Simon Glass
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

At present it is hard to see the changes in CONFIG between different commits
in a branch. The CONFIG options exist in multiple files and the interaction
between Kconfig and the board config files makes it difficult to see
exactly what is going on.

This series is an attempt to improve the situation. It is likely to be
useful mostly to those who are converting things to Kconfig.

A new .cfg file is generated during a build which includes a list of all
preprocessor symbols visible to the source code. This is created for U-Boot,
SPL and TPL. Its value is that it shows all CONFIGs that affect the source
code, regardless of whether they come from Kconfig or board config headers.

Buildman stores these files as well as the .config and autoconf files when
doing a build.

A new -K option allows you to see changes in the contents of these files, for
example:

14: dm: Move Raspberry Pi driver model CONFIGs to Kconfig
    + .config: CONFIG_CMD_DM=y CONFIG_DM=y CONFIG_DM_DEVICE_REMOVE=y
	CONFIG_DM_GPIO=y CONFIG_DM_SERIAL=y CONFIG_DM_STDIO=y CONFIG_DM_WARN=y
    - autoconf.mk: CONFIG_CMD_DM=y CONFIG_DM=y CONFIG_DM_DEVICE_REMOVE=y
	CONFIG_DM_GPIO=y CONFIG_DM_SERIAL=y CONFIG_DM_STDIO=y CONFIG_DM_WARN=y
    + autoconf.h: CONFIG_CMD_DM=1 CONFIG_DM=1 CONFIG_DM_DEVICE_REMOVE=1
	CONFIG_DM_GPIO=1 CONFIG_DM_SERIAL=1 CONFIG_DM_STDIO=1 CONFIG_DM_WARN=1
15: dm: exynos: Move driver model CONFIGs to Kconfig

The above fragment shows that commit 14 added some options to Kconfig and
removed them from autoconf. Everything looks fine. If there were any net
change in the config, then it would show up in u-boot.cfg, as the following
example shows:

23: dm: Kconfig: Move CONFIG_SYS_MALLOC_F_LEN to Kconfig
    + .config: CONFIG_SYS_MALLOC_F=y CONFIG_SYS_MALLOC_F_LEN=0x400
    - autoconf.mk: CONFIG_SYS_MALLOC_F_LEN="(1 << 10)"
    + autoconf.h: CONFIG_SYS_MALLOC_F=1 CONFIG_SYS_MALLOC_F_LEN=0x400
    + u-boot.cfg: CONFIG_SYS_MALLOC_F=1

Here we see (from u-boot.cfg) that a new CONFIG_SYS_MALLOC_F option was
added that did not exist before.

The -K option is normally only useful when used with a single board, or a
group of related boards.


Simon Glass (6):
  Create a .cfg file containing the CONFIG options used to build
  buildman: Add a space before the list of boards
  buildman: Show 'make' command line when -V is used
  buildman: Adjust the 'aborted' heuristic for writing output
  buildman: Store build config files
  buildman: Allow comparison of build configuration

 Makefile                        |  10 ++-
 scripts/Makefile.spl            |   9 +-
 tools/buildman/builder.py       | 186 +++++++++++++++++++++++++++++++++++++---
 tools/buildman/builderthread.py |  62 +++++++++++---
 tools/buildman/cmdline.py       |   4 +-
 tools/buildman/control.py       |   3 +-
 tools/buildman/test.py          |   2 +-
 7 files changed, 244 insertions(+), 32 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build
  2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
@ 2015-02-06  5:06 ` Simon Glass
  2015-04-18 22:19   ` Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 2/6] buildman: Add a space before the list of boards Simon Glass
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

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
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 2/6] buildman: Add a space before the list of boards
  2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build Simon Glass
@ 2015-02-06  5:06 ` Simon Glass
  2015-03-05 23:18   ` Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 3/6] buildman: Show 'make' command line when -V is used Simon Glass
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

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 at 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)
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 3/6] buildman: Show 'make' command line when -V is used
  2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 2/6] buildman: Add a space before the list of boards Simon Glass
@ 2015-02-06  5:06 ` Simon Glass
  2015-04-18 22:19   ` Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 4/6] buildman: Adjust the 'aborted' heuristic for writing output Simon Glass
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

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
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 4/6] buildman: Adjust the 'aborted' heuristic for writing output
  2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
                   ` (2 preceding siblings ...)
  2015-02-06  5:06 ` [U-Boot] [PATCH 3/6] buildman: Show 'make' command line when -V is used Simon Glass
@ 2015-02-06  5:06 ` Simon Glass
  2015-04-18 22:19   ` Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 5/6] buildman: Store build config files Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 6/6] buildman: Allow comparison of build configuration Simon Glass
  5 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

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
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 5/6] buildman: Store build config files
  2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
                   ` (3 preceding siblings ...)
  2015-02-06  5:06 ` [U-Boot] [PATCH 4/6] buildman: Adjust the 'aborted' heuristic for writing output Simon Glass
@ 2015-02-06  5:06 ` Simon Glass
  2015-04-18 22:19   ` Simon Glass
  2015-02-06  5:06 ` [U-Boot] [PATCH 6/6] buildman: Allow comparison of build configuration Simon Glass
  5 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

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
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 6/6] buildman: Allow comparison of build configuration
  2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
                   ` (4 preceding siblings ...)
  2015-02-06  5:06 ` [U-Boot] [PATCH 5/6] buildman: Store build config files Simon Glass
@ 2015-02-06  5:06 ` Simon Glass
  2015-04-18 22:19   ` Simon Glass
  5 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-02-06  5:06 UTC (permalink / raw)
  To: u-boot

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:
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 2/6] buildman: Add a space before the list of boards
  2015-02-06  5:06 ` [U-Boot] [PATCH 2/6] buildman: Add a space before the list of boards Simon Glass
@ 2015-03-05 23:18   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2015-03-05 23:18 UTC (permalink / raw)
  To: u-boot

On 5 February 2015 at 22:06, Simon Glass <sjg@chromium.org> wrote:
> 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 at 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)
> --
> 2.2.0.rc0.207.ga3a616c
>

Applied u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build
  2015-02-06  5:06 ` [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build Simon Glass
@ 2015-04-18 22:19   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2015-04-18 22:19 UTC (permalink / raw)
  To: u-boot

Hi,

On 5 February 2015 at 22:06, Simon Glass <sjg@chromium.org> wrote:
> 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(-)

There have been no comments on this. I'm going to apply it, but if
there are any concerns please let me know.

Applied to u-boot-x86/buildman.

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 3/6] buildman: Show 'make' command line when -V is used
  2015-02-06  5:06 ` [U-Boot] [PATCH 3/6] buildman: Show 'make' command line when -V is used Simon Glass
@ 2015-04-18 22:19   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2015-04-18 22:19 UTC (permalink / raw)
  To: u-boot

On 5 February 2015 at 22:06, Simon Glass <sjg@chromium.org> wrote:
> 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(-)

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 4/6] buildman: Adjust the 'aborted' heuristic for writing output
  2015-02-06  5:06 ` [U-Boot] [PATCH 4/6] buildman: Adjust the 'aborted' heuristic for writing output Simon Glass
@ 2015-04-18 22:19   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2015-04-18 22:19 UTC (permalink / raw)
  To: u-boot

On 5 February 2015 at 22:06, Simon Glass <sjg@chromium.org> wrote:
> 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(-)

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 5/6] buildman: Store build config files
  2015-02-06  5:06 ` [U-Boot] [PATCH 5/6] buildman: Store build config files Simon Glass
@ 2015-04-18 22:19   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2015-04-18 22:19 UTC (permalink / raw)
  To: u-boot

On 5 February 2015 at 22:06, Simon Glass <sjg@chromium.org> wrote:
> 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(-)

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 6/6] buildman: Allow comparison of build configuration
  2015-02-06  5:06 ` [U-Boot] [PATCH 6/6] buildman: Allow comparison of build configuration Simon Glass
@ 2015-04-18 22:19   ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2015-04-18 22:19 UTC (permalink / raw)
  To: u-boot

On 5 February 2015 at 22:06, Simon Glass <sjg@chromium.org> wrote:
> 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(-)

This feature is still a bit raw, but the best way to get feedback is
to get people using it.

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-04-18 22:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-06  5:06 [U-Boot] [PATCH 0/6] buildman: Support comparison of CONFIG options between commits Simon Glass
2015-02-06  5:06 ` [U-Boot] [PATCH 1/6] Create a .cfg file containing the CONFIG options used to build Simon Glass
2015-04-18 22:19   ` Simon Glass
2015-02-06  5:06 ` [U-Boot] [PATCH 2/6] buildman: Add a space before the list of boards Simon Glass
2015-03-05 23:18   ` Simon Glass
2015-02-06  5:06 ` [U-Boot] [PATCH 3/6] buildman: Show 'make' command line when -V is used Simon Glass
2015-04-18 22:19   ` Simon Glass
2015-02-06  5:06 ` [U-Boot] [PATCH 4/6] buildman: Adjust the 'aborted' heuristic for writing output Simon Glass
2015-04-18 22:19   ` Simon Glass
2015-02-06  5:06 ` [U-Boot] [PATCH 5/6] buildman: Store build config files Simon Glass
2015-04-18 22:19   ` Simon Glass
2015-02-06  5:06 ` [U-Boot] [PATCH 6/6] buildman: Allow comparison of build configuration Simon Glass
2015-04-18 22:19   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.