[PATCH 00/20] gitlab: Simplify the test script

[PATCH 00/20] gitlab: Simplify the test script

[Simon Glass]

At present there are several things in the gitlab script which work around
limitations in buildman. With a few small feature additions these can be
removed.

This series adds some new features to buildman and simplifies the script:
- Option to run a single build in a specified output directory
- Allow ignoring warnings
- Removes a restriction on the build output directory

It also
- moves test.py over to use buildman for the --build option
- makes one change to azure since the same approach should be possible there
- fixes a few minor problems noticed in main and sandbox docs


Simon Glass (20):
  sandbox: Add documentation about required/useful packages
  main: Drop show_boot_progress() prototype
  buildman: Document the members of BuilderJob
  bulidman: Add support for a simple build
  gitlab: Use the -w option for sandbox_spl
  azure: Use the -w option for sandbox_spl
  gitlab: Use the --board buildman flag
  gitlab: Drop the BUILDMAN variable
  buildman: Update help for -d
  gitlab: Drop the buildman -d flag
  gitlab: Drop unnecessary if..fi
  gitlab: Use -w flag for all builds
  gitlab: Use bash to avoid needing a_test_which_does_not_exist
  buildman: Allow ignoring warnings in the return code
  gitlab: Use the buildman -W flag
  gitlab: Enable test_handoff
  buildman: Be more selective about which directories to remove
  buildman: Allow building within a subdir of the current dir
  test/py: Use buildman to build U-Boot
  gitlab: Simplify the exit code for test.py

 .azure-pipelines.yml            |  4 +-
 .gitlab-ci.yml                  | 81 +++++++++------------------------
 common/main.c                   |  5 --
 doc/arch/sandbox.rst            | 10 ++++
 test/py/conftest.py             | 16 +++----
 tools/buildman/README           | 13 +++++-
 tools/buildman/builder.py       | 51 ++++++++++++++++-----
 tools/buildman/builderthread.py | 34 ++++++++++----
 tools/buildman/cmdline.py       |  6 ++-
 tools/buildman/control.py       | 35 ++++----------
 tools/buildman/func_test.py     | 46 ++++++++++++++-----
 tools/buildman/test.py          | 20 ++++++++
 12 files changed, 186 insertions(+), 135 deletions(-)

-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 01/20] sandbox: Add documentation about required/useful packages

[Simon Glass]

Quite a few packages are used by sandbox or tools. Add a list of these to
help people setting up for the first time.

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

 doc/arch/sandbox.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst
index e577a95716..6a1c6fc552 100644
--- a/doc/arch/sandbox.rst
+++ b/doc/arch/sandbox.rst
@@ -34,6 +34,16 @@ integers can only be built on 64-bit hosts.
 Note that standalone/API support is not available at present.
 
 
+Prerequisites
+-------------
+
+Here are some packages that are worth installing if you are doing sandbox or
+tools development in U-Boot:
+
+   python3-pytest lzma lzma-alone lz4 python3 python3-virtualenv
+   libssl1.0-dev
+
+
 Basic Operation
 ---------------
 
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 02/20] main: Drop show_boot_progress() prototype

[Simon Glass]

This is defined in bootstage.h and is not called in this file anyway. Drop
it.

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

 common/main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/common/main.c b/common/main.c
index ec8994ad45..06d7ff56d6 100644
--- a/common/main.c
+++ b/common/main.c
@@ -15,11 +15,6 @@
 #include <init.h>
 #include <version.h>
 
-/*
- * Board-specific Platform code can reimplement show_boot_progress () if needed
- */
-__weak void show_boot_progress(int val) {}
-
 static void run_preboot_environment_command(void)
 {
 	char *p;
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 03/20] buildman: Document the members of BuilderJob

[Simon Glass]

This class has a few more members now. Add documentation for them and fix
a nit in the 'commits' comment.

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

 tools/buildman/builderthread.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 570c1f6595..1e52ef8295 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -39,11 +39,15 @@ class BuilderJob:
 
     Members:
         board: Board object to build
-        commits: List of commit options to build.
+        commits: List of Commit objects to build
+        keep_outputs: True to save build output files
+        step: 1 to process every commit, n to process every nth commit
     """
     def __init__(self):
         self.board = None
         self.commits = []
+        self.keep_outputs = False
+        self.step = 1
 
 
 class ResultThread(threading.Thread):
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 04/20] bulidman: Add support for a simple build

[Simon Glass]

It is useful to run a simple build and put all the output in a single
directory. Add a -w option to support this.

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

 tools/buildman/README           | 11 ++++++++++
 tools/buildman/builder.py       | 15 +++++++++++--
 tools/buildman/builderthread.py | 28 ++++++++++++++++-------
 tools/buildman/cmdline.py       |  2 ++
 tools/buildman/control.py       | 10 ++++++++-
 tools/buildman/func_test.py     | 39 +++++++++++++++++++++++++++++----
 6 files changed, 90 insertions(+), 15 deletions(-)

diff --git a/tools/buildman/README b/tools/buildman/README
index c1ac0d0f58..abbbbea9f2 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -1056,6 +1056,17 @@ toolchain. For example:
    buildman -O clang-7 --board sandbox
 
 
+Doing a simple build
+====================
+
+In some cases you just want to build a single board and get the full output, use
+the -w option, for example:
+
+   buildman -o /tmp/build --board sandbox -w
+
+This will write the full build into /tmp/build including object files.
+
+
 Other options
 =============
 
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 3fd4fac695..081c1d0901 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -174,6 +174,8 @@ class Builder:
         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.
+        work_in_output: Use the output directory as the work directory and
+            don't write to a separate output directory.
 
     Private members:
         _base_board_dict: Last-summarised Dict of boards
@@ -224,7 +226,7 @@ class Builder:
                  no_subdirs=False, full_path=False, verbose_build=False,
                  incremental=False, per_board_out_dir=False,
                  config_only=False, squash_config_y=False,
-                 warnings_as_errors=False):
+                 warnings_as_errors=False, work_in_output=False):
         """Create a new Builder object
 
         Args:
@@ -250,10 +252,15 @@ class Builder:
             config_only: Only configure each build, don't build it
             squash_config_y: Convert CONFIG options with the value 'y' to '1'
             warnings_as_errors: Treat all compiler warnings as errors
+            work_in_output: Use the output directory as the work directory and
+                don't write to a separate output directory.
         """
         self.toolchains = toolchains
         self.base_dir = base_dir
-        self._working_dir = os.path.join(base_dir, '.bm-work')
+        if work_in_output:
+            self._working_dir = base_dir
+        else:
+            self._working_dir = os.path.join(base_dir, '.bm-work')
         self.threads = []
         self.do_make = self.Make
         self.gnu_make = gnu_make
@@ -280,6 +287,7 @@ class Builder:
         self.config_only = config_only
         self.squash_config_y = squash_config_y
         self.config_filenames = BASE_CONFIG_FILENAMES
+        self.work_in_output = work_in_output
         if not self.squash_config_y:
             self.config_filenames += EXTRA_CONFIG_FILENAMES
 
@@ -1474,6 +1482,8 @@ class Builder:
         Args:
             thread_num: Number of thread to check.
         """
+        if self.work_in_output:
+            return self._working_dir
         return os.path.join(self._working_dir, '%02d' % thread_num)
 
     def _PrepareThread(self, thread_num, setup_git):
@@ -1571,6 +1581,7 @@ class Builder:
             job.board = brd
             job.commits = commits
             job.keep_outputs = keep_outputs
+            job.work_in_output = self.work_in_output
             job.step = self._step
             self.queue.put(job)
 
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 1e52ef8295..7561f39942 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -42,12 +42,15 @@ class BuilderJob:
         commits: List of Commit objects to build
         keep_outputs: True to save build output files
         step: 1 to process every commit, n to process every nth commit
+        work_in_output: Use the output directory as the work directory and
+            don't write to a separate output directory.
     """
     def __init__(self):
         self.board = None
         self.commits = []
         self.keep_outputs = False
         self.step = 1
+        self.work_in_output = False
 
 
 class ResultThread(threading.Thread):
@@ -118,7 +121,7 @@ class BuilderThread(threading.Thread):
                 **kwargs)
 
     def RunCommit(self, commit_upto, brd, work_dir, do_config, config_only,
-                  force_build, force_build_failures):
+                  force_build, force_build_failures, work_in_output):
         """Build a particular commit.
 
         If the build is already done, and we are not forcing a build, we skip
@@ -133,6 +136,8 @@ class BuilderThread(threading.Thread):
             force_build: Force a build even if one was previously done
             force_build_failures: Force a bulid if the previous result showed
                 failure
+            work_in_output: Use the output directory as the work directory and
+                don't write to a separate output directory.
 
         Returns:
             tuple containing:
@@ -143,7 +148,7 @@ class BuilderThread(threading.Thread):
         # self.Make() below, in the event that we do a build.
         result = command.CommandResult()
         result.return_code = 0
-        if self.builder.in_tree:
+        if work_in_output or self.builder.in_tree:
             out_dir = work_dir
         else:
             if self.per_board_out_dir:
@@ -265,14 +270,18 @@ class BuilderThread(threading.Thread):
         result.out_dir = out_dir
         return result, do_config
 
-    def _WriteResult(self, result, keep_outputs):
+    def _WriteResult(self, result, keep_outputs, work_in_output):
         """Write a built result to the output directory.
 
         Args:
             result: CommandResult object containing result to write
             keep_outputs: True to store the output binaries, False
                 to delete them
+            work_in_output: Use the output directory as the work directory and
+                don't write to a separate output directory.
         """
+        if work_in_output:
+            return
         # Fatal error
         if result.return_code < 0:
             return
@@ -434,7 +443,8 @@ class BuilderThread(threading.Thread):
                 result, request_config = self.RunCommit(commit_upto, brd,
                         work_dir, do_config, self.builder.config_only,
                         force_build or self.builder.force_build,
-                        self.builder.force_build_failures)
+                        self.builder.force_build_failures,
+                        work_in_output=job.work_in_output)
                 failed = result.return_code or result.stderr
                 did_config = do_config
                 if failed and not do_config:
@@ -442,7 +452,8 @@ class BuilderThread(threading.Thread):
                     # with a reconfig.
                     if self.builder.force_config_on_failure:
                         result, request_config = self.RunCommit(commit_upto,
-                            brd, work_dir, True, False, True, False)
+                            brd, work_dir, True, False, True, False,
+                            work_in_output=job.work_in_output)
                         did_config = True
                 if not self.builder.force_reconfig:
                     do_config = request_config
@@ -481,15 +492,16 @@ class BuilderThread(threading.Thread):
                         raise ValueError('Interrupt')
 
                 # We have the build results, so output the result
-                self._WriteResult(result, job.keep_outputs)
+                self._WriteResult(result, job.keep_outputs, job.work_in_output)
                 self.builder.out_queue.put(result)
         else:
             # Just build the currently checked-out build
             result, request_config = self.RunCommit(None, brd, work_dir, True,
                         self.builder.config_only, True,
-                        self.builder.force_build_failures)
+                        self.builder.force_build_failures,
+                        work_in_output=job.work_in_output)
             result.commit_upto = 0
-            self._WriteResult(result, job.keep_outputs)
+            self._WriteResult(result, job.keep_outputs, job.work_in_output)
             self.builder.out_queue.put(result)
 
     def run(self):
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index b41209373d..c7b4bf6b4a 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -106,6 +106,8 @@ def ParseArgs():
           default=False, help='Show build results while the build progresses')
     parser.add_option('-V', '--verbose-build', action='store_true',
           default=False, help='Run make with V=1, logging all output')
+    parser.add_option('-w', '--work-in-output', action='store_true',
+          default=False, help='Use the output directory as the work directory')
     parser.add_option('-x', '--exclude', dest='exclude',
           type='string', action='append',
           help='Specify a list of boards to exclude, separated by comma')
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 969d866547..5d80400f7a 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -263,6 +263,13 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
         str = ("No commits found to process in branch '%s': "
                "set branch's upstream or use -c flag" % options.branch)
         sys.exit(col.Color(col.RED, str))
+    if options.work_in_output:
+        if len(selected) != 1:
+            sys.exit(col.Color(col.RED,
+                               '-w can only be used with a single board'))
+        if count != 1:
+            sys.exit(col.Color(col.RED,
+                               '-w can only be used with a single commit'))
 
     # Read the metadata from the commits. First look@the upstream commit,
     # then the ones in the branch. We would like to do something like
@@ -334,7 +341,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
             per_board_out_dir=options.per_board_out_dir,
             config_only=options.config_only,
             squash_config_y=not options.preserve_config_y,
-            warnings_as_errors=options.warnings_as_errors)
+            warnings_as_errors=options.warnings_as_errors,
+            work_in_output=options.work_in_output)
     builder.force_config_on_failure = not options.quick
     if make_func:
         builder.do_make = make_func
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 4c3d497294..f9f8f80593 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -16,6 +16,7 @@ import control
 import gitutil
 import terminal
 import toolchain
+import tools
 
 settings_data = '''
 # Buildman settings file
@@ -208,7 +209,7 @@ class TestFunctional(unittest.TestCase):
 
     def tearDown(self):
         shutil.rmtree(self._base_dir)
-        shutil.rmtree(self._output_dir)
+        #shutil.rmtree(self._output_dir)
 
     def setupToolchains(self):
         self._toolchains = toolchain.Toolchains()
@@ -218,12 +219,12 @@ class TestFunctional(unittest.TestCase):
         return command.RunPipe([[self._buildman_pathname] + list(args)],
                 capture=True, capture_stderr=True)
 
-    def _RunControl(self, *args, **kwargs):
+    def _RunControl(self, *args, clean_dir=False, boards=None):
         sys.argv = [sys.argv[0]] + list(args)
         options, args = cmdline.ParseArgs()
         result = control.DoBuildman(options, args, toolchains=self._toolchains,
-                make_func=self._HandleMake, boards=self._boards,
-                clean_dir=kwargs.get('clean_dir', True))
+                make_func=self._HandleMake, boards=boards or self._boards,
+                clean_dir=clean_dir)
         self._builder = control.builder
         return result
 
@@ -397,6 +398,12 @@ class TestFunctional(unittest.TestCase):
                     combined='Test configuration complete')
         elif stage == 'build':
             stderr = ''
+            out_dir = ''
+            for arg in args:
+                if arg.startswith('O='):
+                    out_dir = arg[2:]
+            fname = os.path.join(cwd or '', out_dir, 'u-boot')
+            tools.WriteFile(fname, b'U-Boot')
             if type(commit) is not str:
                 stderr = self._error.get((brd.target, commit.sequence))
             if stderr:
@@ -535,3 +542,27 @@ class TestFunctional(unittest.TestCase):
         with self.assertRaises(SystemExit):
             self._RunControl('-b', self._test_branch, '-o',
                              os.path.join(os.getcwd(), 'test'))
+
+    def testWorkInOutput(self):
+        """Test the -w option which should write directly to the output dir"""
+        board_list = board.Boards()
+        board_list.AddBoard(board.Board(*boards[0]))
+        self._RunControl('-o', self._output_dir, '-w', clean_dir=False,
+                         boards=board_list)
+        self.assertTrue(
+            os.path.exists(os.path.join(self._output_dir, 'u-boot')))
+
+    def testWorkInOutputFail(self):
+        """Test the -w option failures"""
+        with self.assertRaises(SystemExit) as e:
+            self._RunControl('-o', self._output_dir, '-w', clean_dir=False)
+        self.assertIn("single board", str(e.exception))
+        self.assertFalse(
+            os.path.exists(os.path.join(self._output_dir, 'u-boot')))
+
+        board_list = board.Boards()
+        board_list.AddBoard(board.Board(*boards[0]))
+        with self.assertRaises(SystemExit) as e:
+            self._RunControl('-b', self._test_branch, '-o', self._output_dir,
+                             '-w', clean_dir=False, boards=board_list)
+        self.assertIn("single commit", str(e.exception))
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 05/20] gitlab: Use the -w option for sandbox_spl

[Simon Glass]

Avoid needing to know about the internal .bm-work directory, by passing
the -w flag to buildman.

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

 .gitlab-ci.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 55943bb3a2..248e0530d2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -167,10 +167,10 @@ Run binman, buildman, dtoc and patman testsuites:
       virtualenv -p /usr/bin/python3 /tmp/venv;
       . /tmp/venv/bin/activate;
       pip install pyelftools;
-      export UBOOT_TRAVIS_BUILD_DIR=/tmp/.bm-work/sandbox_spl;
+      export UBOOT_TRAVIS_BUILD_DIR=/tmp/sandbox_spl;
       export PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt";
       export PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}";
-      ./tools/buildman/buildman -o /tmp -P sandbox_spl;
+      ./tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w sandbox_spl;
       ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test;
       ./tools/buildman/buildman -t;
       ./tools/dtoc/dtoc -t;
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 06/20] azure: Use the -w option for sandbox_spl

[Simon Glass]

Avoid needing to know about the internal .bm-work directory, by passing
the -w flag to buildman.

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

 .azure-pipelines.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 86480581a2..f44b196a15 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -136,10 +136,10 @@ jobs:
           virtualenv -p /usr/bin/python3 /tmp/venv
           . /tmp/venv/bin/activate
           pip install pyelftools
-          export UBOOT_TRAVIS_BUILD_DIR=/tmp/.bm-work/sandbox_spl
+          export UBOOT_TRAVIS_BUILD_DIR=/tmp/sandbox_spl
           export PYTHONPATH=${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt
           export PATH=${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}
-          ./tools/buildman/buildman -o /tmp -P sandbox_spl
+          ./tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w sandbox_spl
           ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test
           ./tools/buildman/buildman -t
           ./tools/dtoc/dtoc -t
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 07/20] gitlab: Use the --board buildman flag

[Simon Glass]

The current method of selecting the board to build is a bit error-prone,
e.g. with "^sandbox$" it actually builds 5 boards (all of those in the
sandbox architecture).

Use the (newish) --board flag instead, to get the same result.

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

 .gitlab-ci.yml | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 248e0530d2..3f48cad752 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -31,9 +31,10 @@ stages:
     # use clang only do one configuration.
     - if [[ "${BUILDMAN}" != "" ]]; then
         ret=0;
-        tools/buildman/buildman -o /tmp -P -E ${BUILDMAN} ${OVERRIDE}|| ret=$?;
+        tools/buildman/buildman -o /tmp -P -E --board ${BUILDMAN} ${OVERRIDE}
+          || ret=$?;
         if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-          tools/buildman/buildman -o /tmp -sdeP ${BUILDMAN};
+          tools/buildman/buildman -o /tmp -sdeP --board ${BUILDMAN};
           exit $ret;
         fi;
       fi
@@ -181,14 +182,14 @@ sandbox test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox"
-    BUILDMAN: "^sandbox$"
+    BUILDMAN: "sandbox"
   <<: *buildman_and_testpy_dfn
 
 sandbox with clang test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox"
-    BUILDMAN: "^sandbox$"
+    BUILDMAN: "sandbox"
     OVERRIDE: "-O clang-7"
   <<: *buildman_and_testpy_dfn
 
@@ -196,7 +197,7 @@ sandbox_spl test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox_spl"
-    BUILDMAN: "^sandbox_spl$"
+    BUILDMAN: "sandbox_spl"
     TEST_PY_TEST_SPEC: "test_ofplatdata"
   <<: *buildman_and_testpy_dfn
 
@@ -205,14 +206,14 @@ evb-ast2500 test.py:
   variables:
     TEST_PY_BD: "evb-ast2500"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^evb-ast2500$"
+    BUILDMAN: "evb-ast2500"
   <<: *buildman_and_testpy_dfn
 
 sandbox_flattree test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox_flattree"
-    BUILDMAN: "^sandbox_flattree$"
+    BUILDMAN: "sandbox_flattree"
   <<: *buildman_and_testpy_dfn
 
 vexpress_ca15_tc2 test.py:
@@ -220,7 +221,7 @@ vexpress_ca15_tc2 test.py:
   variables:
     TEST_PY_BD: "vexpress_ca15_tc2"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^vexpress_ca15_tc2$"
+    BUILDMAN: "vexpress_ca15_tc2"
   <<: *buildman_and_testpy_dfn
 
 vexpress_ca9x4 test.py:
@@ -228,7 +229,7 @@ vexpress_ca9x4 test.py:
   variables:
     TEST_PY_BD: "vexpress_ca9x4"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^vexpress_ca9x4$"
+    BUILDMAN: "vexpress_ca9x4"
   <<: *buildman_and_testpy_dfn
 
 integratorcp_cm926ejs test.py:
@@ -237,7 +238,7 @@ integratorcp_cm926ejs test.py:
     TEST_PY_BD: "integratorcp_cm926ejs"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^integratorcp_cm926ejs$"
+    BUILDMAN: "integratorcp_cm926ejs"
   <<: *buildman_and_testpy_dfn
 
 qemu_arm test.py:
@@ -245,7 +246,7 @@ qemu_arm test.py:
   variables:
     TEST_PY_BD: "qemu_arm"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu_arm$"
+    BUILDMAN: "qemu_arm"
   <<: *buildman_and_testpy_dfn
 
 qemu_arm64 test.py:
@@ -253,7 +254,7 @@ qemu_arm64 test.py:
   variables:
     TEST_PY_BD: "qemu_arm64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu_arm64$"
+    BUILDMAN: "qemu_arm64"
   <<: *buildman_and_testpy_dfn
 
 qemu_mips test.py:
@@ -261,7 +262,7 @@ qemu_mips test.py:
   variables:
     TEST_PY_BD: "qemu_mips"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu_mips$"
+    BUILDMAN: "qemu_mips"
   <<: *buildman_and_testpy_dfn
 
 qemu_mipsel test.py:
@@ -269,7 +270,7 @@ qemu_mipsel test.py:
   variables:
     TEST_PY_BD: "qemu_mipsel"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu_mipsel$"
+    BUILDMAN: "qemu_mipsel"
   <<: *buildman_and_testpy_dfn
 
 qemu_mips64 test.py:
@@ -277,7 +278,7 @@ qemu_mips64 test.py:
   variables:
     TEST_PY_BD: "qemu_mips64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu_mips64$"
+    BUILDMAN: "qemu_mips64"
   <<: *buildman_and_testpy_dfn
 
 qemu_mips64el test.py:
@@ -285,7 +286,7 @@ qemu_mips64el test.py:
   variables:
     TEST_PY_BD: "qemu_mips64el"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu_mips64el$"
+    BUILDMAN: "qemu_mips64el"
   <<: *buildman_and_testpy_dfn
 
 qemu-ppce500 test.py:
@@ -293,7 +294,7 @@ qemu-ppce500 test.py:
   variables:
     TEST_PY_BD: "qemu-ppce500"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu-ppce500$"
+    BUILDMAN: "qemu-ppce500"
   <<: *buildman_and_testpy_dfn
 
 qemu-riscv64 test.py:
@@ -301,7 +302,7 @@ qemu-riscv64 test.py:
   variables:
     TEST_PY_BD: "qemu-riscv64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu-riscv64$"
+    BUILDMAN: "qemu-riscv64"
   <<: *buildman_and_testpy_dfn
 
 qemu-x86 test.py:
@@ -309,7 +310,7 @@ qemu-x86 test.py:
   variables:
     TEST_PY_BD: "qemu-x86"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu-x86$"
+    BUILDMAN: "qemu-x86"
   <<: *buildman_and_testpy_dfn
 
 qemu-x86_64 test.py:
@@ -317,7 +318,7 @@ qemu-x86_64 test.py:
   variables:
     TEST_PY_BD: "qemu-x86_64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "^qemu-x86_64$"
+    BUILDMAN: "qemu-x86_64"
   <<: *buildman_and_testpy_dfn
 
 zynq_zc702 test.py:
@@ -326,7 +327,7 @@ zynq_zc702 test.py:
     TEST_PY_BD: "zynq_zc702"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^zynq_zc702$"
+    BUILDMAN: "zynq_zc702"
   <<: *buildman_and_testpy_dfn
 
 xilinx_versal_virt test.py:
@@ -335,7 +336,7 @@ xilinx_versal_virt test.py:
     TEST_PY_BD: "xilinx_versal_virt"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^xilinx_versal_virt$"
+    BUILDMAN: "xilinx_versal_virt"
   <<: *buildman_and_testpy_dfn
 
 xtfpga test.py:
@@ -344,5 +345,5 @@ xtfpga test.py:
     TEST_PY_BD: "xtfpga"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "^xtfpga$"
+    BUILDMAN: "xtfpga"
   <<: *buildman_and_testpy_dfn
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 08/20] gitlab: Drop the BUILDMAN variable

[Simon Glass]

This is not needed now since we use the same name as the pytests.

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

 .gitlab-ci.yml | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3f48cad752..2cd6209222 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -29,12 +29,12 @@ stages:
   script:
     # From buildman, exit code 129 means warnings only.  If we've been asked to
     # use clang only do one configuration.
-    - if [[ "${BUILDMAN}" != "" ]]; then
+    - if [[ "${TEST_PY_BD}" != "" ]]; then
         ret=0;
-        tools/buildman/buildman -o /tmp -P -E --board ${BUILDMAN} ${OVERRIDE}
+        tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
           || ret=$?;
         if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-          tools/buildman/buildman -o /tmp -sdeP --board ${BUILDMAN};
+          tools/buildman/buildman -o /tmp -sdeP --board ${TEST_PY_BD};
           exit $ret;
         fi;
       fi
@@ -182,14 +182,12 @@ sandbox test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox"
-    BUILDMAN: "sandbox"
   <<: *buildman_and_testpy_dfn
 
 sandbox with clang test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox"
-    BUILDMAN: "sandbox"
     OVERRIDE: "-O clang-7"
   <<: *buildman_and_testpy_dfn
 
@@ -197,7 +195,6 @@ sandbox_spl test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox_spl"
-    BUILDMAN: "sandbox_spl"
     TEST_PY_TEST_SPEC: "test_ofplatdata"
   <<: *buildman_and_testpy_dfn
 
@@ -206,14 +203,12 @@ evb-ast2500 test.py:
   variables:
     TEST_PY_BD: "evb-ast2500"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "evb-ast2500"
   <<: *buildman_and_testpy_dfn
 
 sandbox_flattree test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox_flattree"
-    BUILDMAN: "sandbox_flattree"
   <<: *buildman_and_testpy_dfn
 
 vexpress_ca15_tc2 test.py:
@@ -221,7 +216,6 @@ vexpress_ca15_tc2 test.py:
   variables:
     TEST_PY_BD: "vexpress_ca15_tc2"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "vexpress_ca15_tc2"
   <<: *buildman_and_testpy_dfn
 
 vexpress_ca9x4 test.py:
@@ -229,7 +223,6 @@ vexpress_ca9x4 test.py:
   variables:
     TEST_PY_BD: "vexpress_ca9x4"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "vexpress_ca9x4"
   <<: *buildman_and_testpy_dfn
 
 integratorcp_cm926ejs test.py:
@@ -238,7 +231,6 @@ integratorcp_cm926ejs test.py:
     TEST_PY_BD: "integratorcp_cm926ejs"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "integratorcp_cm926ejs"
   <<: *buildman_and_testpy_dfn
 
 qemu_arm test.py:
@@ -246,7 +238,6 @@ qemu_arm test.py:
   variables:
     TEST_PY_BD: "qemu_arm"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu_arm"
   <<: *buildman_and_testpy_dfn
 
 qemu_arm64 test.py:
@@ -254,7 +245,6 @@ qemu_arm64 test.py:
   variables:
     TEST_PY_BD: "qemu_arm64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu_arm64"
   <<: *buildman_and_testpy_dfn
 
 qemu_mips test.py:
@@ -262,7 +252,6 @@ qemu_mips test.py:
   variables:
     TEST_PY_BD: "qemu_mips"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu_mips"
   <<: *buildman_and_testpy_dfn
 
 qemu_mipsel test.py:
@@ -270,7 +259,6 @@ qemu_mipsel test.py:
   variables:
     TEST_PY_BD: "qemu_mipsel"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu_mipsel"
   <<: *buildman_and_testpy_dfn
 
 qemu_mips64 test.py:
@@ -278,7 +266,6 @@ qemu_mips64 test.py:
   variables:
     TEST_PY_BD: "qemu_mips64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu_mips64"
   <<: *buildman_and_testpy_dfn
 
 qemu_mips64el test.py:
@@ -286,7 +273,6 @@ qemu_mips64el test.py:
   variables:
     TEST_PY_BD: "qemu_mips64el"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu_mips64el"
   <<: *buildman_and_testpy_dfn
 
 qemu-ppce500 test.py:
@@ -294,7 +280,6 @@ qemu-ppce500 test.py:
   variables:
     TEST_PY_BD: "qemu-ppce500"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu-ppce500"
   <<: *buildman_and_testpy_dfn
 
 qemu-riscv64 test.py:
@@ -302,7 +287,6 @@ qemu-riscv64 test.py:
   variables:
     TEST_PY_BD: "qemu-riscv64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu-riscv64"
   <<: *buildman_and_testpy_dfn
 
 qemu-x86 test.py:
@@ -310,7 +294,6 @@ qemu-x86 test.py:
   variables:
     TEST_PY_BD: "qemu-x86"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu-x86"
   <<: *buildman_and_testpy_dfn
 
 qemu-x86_64 test.py:
@@ -318,7 +301,6 @@ qemu-x86_64 test.py:
   variables:
     TEST_PY_BD: "qemu-x86_64"
     TEST_PY_TEST_SPEC: "not sleep"
-    BUILDMAN: "qemu-x86_64"
   <<: *buildman_and_testpy_dfn
 
 zynq_zc702 test.py:
@@ -327,7 +309,6 @@ zynq_zc702 test.py:
     TEST_PY_BD: "zynq_zc702"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "zynq_zc702"
   <<: *buildman_and_testpy_dfn
 
 xilinx_versal_virt test.py:
@@ -336,7 +317,6 @@ xilinx_versal_virt test.py:
     TEST_PY_BD: "xilinx_versal_virt"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "xilinx_versal_virt"
   <<: *buildman_and_testpy_dfn
 
 xtfpga test.py:
@@ -345,5 +325,4 @@ xtfpga test.py:
     TEST_PY_BD: "xtfpga"
     TEST_PY_TEST_SPEC: "not sleep"
     TEST_PY_ID: "--id qemu"
-    BUILDMAN: "xtfpga"
   <<: *buildman_and_testpy_dfn
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 09/20] buildman: Update help for -d

[Simon Glass]

This help is a bit ambiguous. It only does anything if showing size
changes with -S. Update the help and the function comments.

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

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

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 081c1d0901..a41d0b316e 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -337,7 +337,7 @@ class Builder:
 
         show_errors: True to show summarised error/warning info
         show_sizes: Show size deltas
-        show_detail: Show detail for each board
+        show_detail: Show size delta detail for each board if show_sizes
         show_bloat: Show detail for each function
         list_error_boards: Show the boards which caused each error/warning
         show_config: Show config deltas
@@ -1000,7 +1000,7 @@ class Builder:
                 board.target
             board_dict: Dict containing boards for which we built this
                 commit, keyed by board.target. The value is an Outcome object.
-            show_detail: Show detail for each board
+            show_detail: Show size delta detail for each board
             show_bloat: Show detail for each function
         """
         arch_list = {}
@@ -1117,7 +1117,7 @@ class Builder:
             environment: Dictionary keyed by environment variable, Each
                      value is the value of environment variable.
             show_sizes: Show image size deltas
-            show_detail: Show detail for each board
+            show_detail: Show size delta detail for each board if show_sizes
             show_bloat: Show detail for each function
             show_config: Show config changes
             show_environment: Show environment changes
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index c7b4bf6b4a..74b410010d 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -31,7 +31,7 @@ def ParseArgs():
           help='Reconfigure for every commit (disable incremental build)')
     parser.add_option('-d', '--detail', dest='show_detail',
           action='store_true', default=False,
-          help='Show detailed information for each board in summary')
+          help='Show detailed size delta for each board in the -S summary')
     parser.add_option('-D', '--config-only', action='store_true', default=False,
           help="Don't build, just configure each commit")
     parser.add_option('-e', '--show_errors', action='store_true',
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 10/20] gitlab: Drop the buildman -d flag

[Simon Glass]

This has no effect since -S is not given also. Drop it.

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

 .gitlab-ci.yml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2cd6209222..36c2ecfa43 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -34,7 +34,7 @@ stages:
         tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
           || ret=$?;
         if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-          tools/buildman/buildman -o /tmp -sdeP --board ${TEST_PY_BD};
+          tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
           exit $ret;
         fi;
       fi
@@ -65,7 +65,7 @@ build all 32bit ARM platforms:
     - ret=0;
       ./tools/buildman/buildman -o /tmp -P -E arm -x aarch64 || ret=$?;
       if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-        ./tools/buildman/buildman -o /tmp -sdeP;
+        ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
 
@@ -79,7 +79,7 @@ build all 64bit ARM platforms:
     - ret=0;
       ./tools/buildman/buildman -o /tmp -P -E aarch64 || ret=$?;
       if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-        ./tools/buildman/buildman -o /tmp -sdeP;
+        ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
 
@@ -90,7 +90,7 @@ build all PowerPC platforms:
     - ret=0;
       ./tools/buildman/buildman -o /tmp -P -E powerpc || ret=$?;
       if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-        ./tools/buildman/buildman -o /tmp -sdeP;
+        ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
 
@@ -101,7 +101,7 @@ build all other platforms:
     - ret=0;
       ./tools/buildman/buildman -o /tmp -P -E -x arm,powerpc || ret=$?;
       if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-        ./tools/buildman/buildman -o /tmp -sdeP;
+        ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
 
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 11/20] gitlab: Drop unnecessary if..fi

[Simon Glass]

Since TEST_PY_BD is always defined we can drop this check.

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

 .gitlab-ci.yml | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 36c2ecfa43..b29d59d942 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -29,14 +29,12 @@ stages:
   script:
     # From buildman, exit code 129 means warnings only.  If we've been asked to
     # use clang only do one configuration.
-    - if [[ "${TEST_PY_BD}" != "" ]]; then
-        ret=0;
-        tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
-          || ret=$?;
-        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-          tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
-          exit $ret;
-        fi;
+    - ret=0;
+      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
+        || ret=$?;
+      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
+        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
+        exit $ret;
       fi
     # "not a_test_which_does_not_exist" is a dummy -k parameter which will
     # never prevent any test from running. That way, we can always pass
@@ -48,15 +46,13 @@ stages:
     - export UBOOT_TRAVIS_BUILD_DIR=/tmp/.bm-work/${TEST_PY_BD};
       export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH};
       export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci;
-      if [[ "${TEST_PY_BD}" != "" ]]; then
-        ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID}
-          -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}"
-          --build-dir "$UBOOT_TRAVIS_BUILD_DIR";
-        ret=$?;
-        if [[ $ret -ne 0 ]]; then
-          exit $ret;
-        fi;
-      fi;
+      ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID}
+        -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}"
+        --build-dir "$UBOOT_TRAVIS_BUILD_DIR";
+      ret=$?;
+      if [[ $ret -ne 0 ]]; then
+        exit $ret;
+      fi
 
 build all 32bit ARM platforms:
   tags: [ 'all' ]
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 12/20] gitlab: Use -w flag for all builds

[Simon Glass]

Avoid needing to know about the internal .bm-work directory, by passing
the -w flag to buildman.

Also drop the repeated call to buildman since the first one should show
all the expected output. We only need to use -s if we are building
multiple boards and want the errors to be coalesced. In this case we are
only building a single board.

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

 .gitlab-ci.yml | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b29d59d942..bbd05aa872 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -29,11 +29,11 @@ stages:
   script:
     # From buildman, exit code 129 means warnings only.  If we've been asked to
     # use clang only do one configuration.
+    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
     - ret=0;
-      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
-        || ret=$?;
+      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
+        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
       if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
         exit $ret;
       fi
     # "not a_test_which_does_not_exist" is a dummy -k parameter which will
@@ -43,8 +43,7 @@ stages:
     - virtualenv -p /usr/bin/python3 /tmp/venv
     - . /tmp/venv/bin/activate
     - pip install -r test/py/requirements.txt
-    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/.bm-work/${TEST_PY_BD};
-      export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH};
+    - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH};
       export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci;
       ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID}
         -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}"
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 13/20] gitlab: Use bash to avoid needing a_test_which_does_not_exist

[Simon Glass]

Bash allows for variables to expand only if non-empty:

	$ var=test
	$ echo ${var:+"$var"}
	test
	$ echo ${var:+"-k $var"}
	-k test
	$ var=
	$ echo ${var:+"-k $var"}

Use this feature to avoid the workaround.

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

 .gitlab-ci.yml | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index bbd05aa872..05f56c6d19 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -36,17 +36,13 @@ stages:
       if [[ $ret -ne 0 && $ret -ne 129 ]]; then
         exit $ret;
       fi
-    # "not a_test_which_does_not_exist" is a dummy -k parameter which will
-    # never prevent any test from running. That way, we can always pass
-    # "-k something" even when $TEST_PY_TEST_SPEC doesnt need a custom
-    # value.
     - virtualenv -p /usr/bin/python3 /tmp/venv
     - . /tmp/venv/bin/activate
     - pip install -r test/py/requirements.txt
     - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH};
       export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci;
       ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID}
-        -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}"
+        ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"}
         --build-dir "$UBOOT_TRAVIS_BUILD_DIR";
       ret=$?;
       if [[ $ret -ne 0 ]]; then
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 14/20] buildman: Allow ignoring warnings in the return code

[Simon Glass]

Sometimes we don't want to get an error with warnings. Add a -W option to
support this. If buildman detects warnings (and no error) it will return
an exit code of 0 (success).

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

 tools/buildman/README     | 2 +-
 tools/buildman/cmdline.py | 2 ++
 tools/buildman/control.py | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/buildman/README b/tools/buildman/README
index abbbbea9f2..87bd8ee9d7 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -1079,7 +1079,7 @@ When doing builds, Buildman's return code will reflect the overall result:
 
     0 (success)     No errors or warnings found
     128             Errors found
-    129             Warnings found
+    129             Warnings found (use -W to return 0 in this case)
 
 
 How to change from MAKEALL
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 74b410010d..f387aeb1cf 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -108,6 +108,8 @@ def ParseArgs():
           default=False, help='Run make with V=1, logging all output')
     parser.add_option('-w', '--work-in-output', action='store_true',
           default=False, help='Use the output directory as the work directory')
+    parser.add_option('-W', '--ignore-warnings', action='store_true',
+          default=False, help='Return success even if there are warnings')
     parser.add_option('-x', '--exclude', dest='exclude',
           type='string', action='append',
           help='Specify a list of boards to exclude, separated by comma')
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 5d80400f7a..ded4360250 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -386,6 +386,6 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
                                 options.keep_outputs, options.verbose)
             if fail:
                 return 128
-            elif warned:
+            elif warned and not options.ignore_warnings:
                 return 129
     return 0
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 15/20] gitlab: Use the buildman -W flag

[Simon Glass]

It doesn't seem to make sense to tell buildman to report warning as errors
(thus ensuring there are no warnings) and then ignore the warnings.

We can simplify the logic now that we can tell buildman to ignore
warnings.

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

 .gitlab-ci.yml | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 05f56c6d19..9de6592d1a 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -30,12 +30,8 @@ stages:
     # From buildman, exit code 129 means warnings only.  If we've been asked to
     # use clang only do one configuration.
     - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
-    - ret=0;
-      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
-        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
-      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
-        exit $ret;
-      fi
+    - tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -W
+        --board ${TEST_PY_BD} ${OVERRIDE}
     - virtualenv -p /usr/bin/python3 /tmp/venv
     - . /tmp/venv/bin/activate
     - pip install -r test/py/requirements.txt
@@ -54,8 +50,8 @@ build all 32bit ARM platforms:
   stage: world build
   script:
     - ret=0;
-      ./tools/buildman/buildman -o /tmp -P -E arm -x aarch64 || ret=$?;
-      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
+      ./tools/buildman/buildman -o /tmp -PW arm -x aarch64 || ret=$?;
+      if [[ $ret -ne 0 ]]; then
         ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
@@ -68,8 +64,8 @@ build all 64bit ARM platforms:
     - . /tmp/venv/bin/activate
     - pip install pyelftools
     - ret=0;
-      ./tools/buildman/buildman -o /tmp -P -E aarch64 || ret=$?;
-      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
+      ./tools/buildman/buildman -o /tmp -PW aarch64 || ret=$?;
+      if [[ $ret -ne 0 ]]; then
         ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
@@ -79,8 +75,8 @@ build all PowerPC platforms:
   stage: world build
   script:
     - ret=0;
-      ./tools/buildman/buildman -o /tmp -P -E powerpc || ret=$?;
-      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
+      ./tools/buildman/buildman -o /tmp -PW powerpc || ret=$?;
+      if [[ $ret -ne 0 ]]; then
         ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
@@ -90,8 +86,8 @@ build all other platforms:
   stage: world build
   script:
     - ret=0;
-      ./tools/buildman/buildman -o /tmp -P -E -x arm,powerpc || ret=$?;
-      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
+      ./tools/buildman/buildman -o /tmp -PW -x arm,powerpc || ret=$?;
+      if [[ $ret -ne 0 ]]; then
         ./tools/buildman/buildman -o /tmp -seP;
         exit $ret;
       fi;
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 16/20] gitlab: Enable test_handoff

[Simon Glass]

Ensure that this SPL test runs on gitlab.

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

 .gitlab-ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9de6592d1a..26c8fb3199 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -182,7 +182,7 @@ sandbox_spl test.py:
   tags: [ 'all' ]
   variables:
     TEST_PY_BD: "sandbox_spl"
-    TEST_PY_TEST_SPEC: "test_ofplatdata"
+    TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff"
   <<: *buildman_and_testpy_dfn
 
 evb-ast2500 test.py:
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 17/20] buildman: Be more selective about which directories to remove

[Simon Glass]

At present buildman removes any directory it doesn't intend to write
output into. This is overly expansive since if the output directory
happens to be somewhere with existing files, they may be removed. Using
an existing directory for buildman is not a good practice, but since the
result might be catastrophic, it is best to guard against it.

A previous commit[1] fixed this by refusing to write to a subdirectory
of the current directory, assumed to have U-Boot source code. But we can
do better by only removing directories that look like the ones buildman
creates.

Update the code to do this and add a test.

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

[1] 409fc029c40 tools: buildman: Don't use the working dir as build dir

---

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

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index a41d0b316e..30ec4254f8 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -485,6 +485,7 @@ class Builder:
         if self.commits:
             commit = self.commits[commit_upto]
             subject = commit.subject.translate(trans_valid_chars)
+            # See _GetOutputSpaceRemovals() which parses this name
             commit_dir = ('%02d_of_%02d_g%s_%s' % (commit_upto + 1,
                     self.commit_count, commit.hash, subject[:20]))
         elif not self.no_subdirs:
@@ -1525,12 +1526,15 @@ class Builder:
         for thread in range(max_threads):
             self._PrepareThread(thread, setup_git)
 
-    def _PrepareOutputSpace(self):
+    def _GetOutputSpaceRemovals(self):
         """Get the output directories ready to receive files.
 
-        We delete any output directories which look like ones we need to
-        create. Having left over directories is confusing when the user wants
-        to check the output manually.
+        Figure out what needs to be deleted in the output directory before it
+        can be used. We only delete old buildman directories which have the
+        expected name pattern. See _GetOutputDir().
+
+        Returns:
+            List of full paths of directories to remove
         """
         if not self.commits:
             return
@@ -1541,7 +1545,20 @@ class Builder:
         to_remove = []
         for dirname in glob.glob(os.path.join(self.base_dir, '*')):
             if dirname not in dir_list:
-                to_remove.append(dirname)
+                leaf = dirname[len(self.base_dir) + 1:]
+                m =  re.match('[0-9]+_of_[0-9]+_g[0-9a-f]+_.*', leaf)
+                if m:
+                    to_remove.append(dirname)
+        return to_remove
+
+    def _PrepareOutputSpace(self):
+        """Get the output directories ready to receive files.
+
+        We delete any output directories which look like ones we need to
+        create. Having left over directories is confusing when the user wants
+        to check the output manually.
+        """
+        to_remove = self._GetOutputSpaceRemovals()
         if to_remove:
             Print('Removing %d old build directories' % len(to_remove),
                   newline=False)
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index acd862b3b0..2aaedf44ac 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -22,6 +22,7 @@ import commit
 import terminal
 import test_util
 import toolchain
+import tools
 
 use_network = True
 
@@ -469,6 +470,25 @@ class TestBuild(unittest.TestCase):
         self.assertEqual('HOSTCC=clang CC=clang',
                          tc.GetEnvArgs(toolchain.VAR_MAKE_ARGS))
 
+    def testPrepareOutputSpace(self):
+        def _Touch(fname):
+            tools.WriteFile(os.path.join(base_dir, fname), b'')
+
+        base_dir = tempfile.mkdtemp()
+
+        # Add various files that we want removed and left alone
+        to_remove = ['01_of_22_g0982734987_title', '102_of_222_g92bf_title',
+                     '01_of_22_g2938abd8_title']
+        to_leave = ['something_else', '01-something.patch', '01_of_22_another']
+        for name in to_remove + to_leave:
+            _Touch(name)
+
+        build = builder.Builder(self.toolchains, base_dir, None, 1, 2)
+        build.commits = self.commits
+        build.commit_count = len(commits)
+        result = set(build._GetOutputSpaceRemovals())
+        expected = set([os.path.join(base_dir, f) for f in to_remove])
+        self.assertEqual(expected, result)
 
 if __name__ == "__main__":
     unittest.main()
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 18/20] buildman: Allow building within a subdir of the current dir

[Simon Glass]

This is useful in some situations, in particular with -w and when building
in-tree. Now that we are more careful about what we remove in
_PrepareOutputSpace(), it should be save to relax this restriction.

Update the progress information also so it is clear what buildman is
doing. Remove files can take a long time.

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

 tools/buildman/builder.py   |  3 ++-
 tools/buildman/control.py   | 23 -----------------------
 tools/buildman/func_test.py |  9 ---------
 3 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 30ec4254f8..70c55c588a 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1560,10 +1560,11 @@ class Builder:
         """
         to_remove = self._GetOutputSpaceRemovals()
         if to_remove:
-            Print('Removing %d old build directories' % len(to_remove),
+            Print('Removing %d old build directories...' % len(to_remove),
                   newline=False)
             for dirname in to_remove:
                 shutil.rmtree(dirname)
+            Print('done')
 
     def BuildBoards(self, commits, board_selected, keep_outputs, verbose):
         """Build all commits for a list of boards
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index ded4360250..7d31863c63 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -85,28 +85,6 @@ def ShowActions(series, why_selected, boards_selected, builder, options,
         for warning in board_warnings:
             print(col.Color(col.YELLOW, warning))
 
-def CheckOutputDir(output_dir):
-    """Make sure that the output directory is not within the current directory
-
-    If we try to use an output directory which is within the current directory
-    (which is assumed to hold the U-Boot source) we may end up deleting the
-    U-Boot source code. Detect this and print an error in this case.
-
-    Args:
-        output_dir: Output directory path to check
-    """
-    path = os.path.realpath(output_dir)
-    cwd_path = os.path.realpath('.')
-    while True:
-        if os.path.realpath(path) == cwd_path:
-            Print("Cannot use output directory '%s' since it is within the current directory '%s'" %
-                  (path, cwd_path))
-            sys.exit(1)
-        parent = os.path.dirname(path)
-        if parent == path:
-            break
-        path = parent
-
 def ShowToolchainInfo(boards, toolchains, print_arch, print_prefix):
     """Show information about a the tool chain used by one or more boards
 
@@ -331,7 +309,6 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
             output_dir = os.path.join(options.output_dir, dirname)
         if clean_dir and os.path.exists(output_dir):
             shutil.rmtree(output_dir)
-    CheckOutputDir(output_dir)
     builder = Builder(toolchains, output_dir, options.git_dir,
             options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
             show_unknown=options.show_unknown, step=options.step,
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index f9f8f80593..2a256a9263 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -534,15 +534,6 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(self._builder.count, self._total_builds)
         self.assertEqual(self._builder.fail, 0)
 
-    def testBadOutputDir(self):
-        """Test building with an output dir the same as out current dir"""
-        self._test_branch = '/__dev/__testbranch'
-        with self.assertRaises(SystemExit):
-            self._RunControl('-b', self._test_branch, '-o', os.getcwd())
-        with self.assertRaises(SystemExit):
-            self._RunControl('-b', self._test_branch, '-o',
-                             os.path.join(os.getcwd(), 'test'))
-
     def testWorkInOutput(self):
         """Test the -w option which should write directly to the output dir"""
         board_list = board.Boards()
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 19/20] test/py: Use buildman to build U-Boot

[Simon Glass]

It is a pain to have to set the ARCH and CROSS_COMPILE environment
variables when using test.py's --build option. It is possible to get these
using the -A and -a options from buildman. But it seems better to just use
buildman to do the build.

Remove the manual 'make' logic in test/py and use buildman instead.

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

 test/py/conftest.py | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/test/py/conftest.py b/test/py/conftest.py
index 34ac4fb062..8079cd7305 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -141,17 +141,13 @@ def pytest_configure(config):
 
     if config.getoption('build'):
         if build_dir != source_dir:
-            o_opt = 'O=%s' % build_dir
+            dest_args = ['-o', build_dir, '-w']
         else:
-            o_opt = ''
-        cmds = (
-            ['make', o_opt, '-s', board_type + '_defconfig'],
-            ['make', o_opt, '-s', '-j8'],
-        )
-        with log.section('make'):
-            runner = log.get_runner('make', sys.stdout)
-            for cmd in cmds:
-                runner.run(cmd, cwd=source_dir)
+            dest_args = ['-i']
+        cmd = ['buildman', '--board', board_type] + dest_args
+        with log.section('buildman'):
+            runner = log.get_runner('buildman', sys.stdout)
+            runner.run(cmd, cwd=source_dir)
             runner.close()
             log.status_pass('OK')
 
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 20/20] gitlab: Simplify the exit code for test.py

[Simon Glass]

It seems unnecessary to read the exit code and then check it again. Drop
this and just let the test.py provide the exit code directly.

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

 .gitlab-ci.yml | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 26c8fb3199..98adfa7d80 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -39,11 +39,7 @@ stages:
       export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci;
       ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID}
         ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"}
-        --build-dir "$UBOOT_TRAVIS_BUILD_DIR";
-      ret=$?;
-      if [[ $ret -ne 0 ]]; then
-        exit $ret;
-      fi
+        --build-dir "$UBOOT_TRAVIS_BUILD_DIR"
 
 build all 32bit ARM platforms:
   tags: [ 'all' ]
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 19/20] test/py: Use buildman to build U-Boot

[Stephen Warren]

On 3/6/20 8:07 PM, Simon Glass wrote:
> It is a pain to have to set the ARCH and CROSS_COMPILE environment
> variables when using test.py's --build option. It is possible to get these
> using the -A and -a options from buildman. But it seems better to just use
> buildman to do the build.
> 
> Remove the manual 'make' logic in test/py and use buildman instead.

I far prefer using make here; this requires zero setup of buildman (e.g. 
the config file and specific toolchains), and so it much *less* of a pain.

[PATCH 19/20] test/py: Use buildman to build U-Boot

[Tom Rini]

On Mon, Mar 09, 2020 at 11:10:55AM -0600, Stephen Warren wrote:
> On 3/6/20 8:07 PM, Simon Glass wrote:
> > It is a pain to have to set the ARCH and CROSS_COMPILE environment
> > variables when using test.py's --build option. It is possible to get these
> > using the -A and -a options from buildman. But it seems better to just use
> > buildman to do the build.
> > 
> > Remove the manual 'make' logic in test/py and use buildman instead.
> 
> I far prefer using make here; this requires zero setup of buildman (e.g. the
> config file and specific toolchains), and so it much *less* of a pain.

I have to agree here.  Keeping our test suite as dependency-free as
possible is important.  But... that's also not what's going on in the
code.  We don't set ARCH from what I can see, and of course don't use
it.  We don't set the CROSS_COMPILER from the snippet in question, only
the output directory.  Today, looking at the Travis/GitLab CI scripts we
don't even build via test.py but rather buildman prior to calling
test.py.  And I don't think I saw that changing in this series either.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/e2ab1525/attachment.sig>

[PATCH 07/20] gitlab: Use the --board buildman flag

[Tom Rini]

On Fri, Mar 06, 2020 at 08:07:21PM -0700, Simon Glass wrote:

> The current method of selecting the board to build is a bit error-prone,
> e.g. with "^sandbox$" it actually builds 5 boards (all of those in the
> sandbox architecture).

OK, hold up.  You've reminded me of something that's been broken for a
while and I forgot to follow up on.  Which is that at some point we
stopped applying regex to either only the defconfig name, or stopped
checking that as the first one.  The example in question used to only
build sandbox_defconfig, but now (and for some time) has built all of
arch or soc sandbox.

That said, I'm not sure that at the end of the day I'm opposed to the
--board flag instead.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/cfc534d3/attachment.sig>

[PATCH 05/20] gitlab: Use the -w option for sandbox_spl

[Tom Rini]

On Fri, Mar 06, 2020 at 08:07:19PM -0700, Simon Glass wrote:

> Avoid needing to know about the internal .bm-work directory, by passing
> the -w flag to buildman.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  .gitlab-ci.yml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

What I'd like to see is GitLab / Azure / Travis changing in-sync with
these steps, rather than one commit for each (and Travis getting out of
sync).  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/c40d1dd6/attachment.sig>

[PATCH 00/20] gitlab: Simplify the test script

[Tom Rini]

On Fri, Mar 06, 2020 at 08:07:14PM -0700, Simon Glass wrote:

> At present there are several things in the gitlab script which work around
> limitations in buildman. With a few small feature additions these can be
> removed.
> 
> This series adds some new features to buildman and simplifies the script:
> - Option to run a single build in a specified output directory
> - Allow ignoring warnings
> - Removes a restriction on the build output directory
> 
> It also
> - moves test.py over to use buildman for the --build option
> - makes one change to azure since the same approach should be possible there
> - fixes a few minor problems noticed in main and sandbox docs

One general comment is that while it's clear from this series that
you're focusing on test.py invocation most of the time, a lot of the
commit messages aren't clear that you're changing the
buildman_and_testpy_template stanza.

Also, while you're keeping Azure in sync (as it's largely based on the
GitLab logic), Travis is being left out.  But the GitLab logic is based
on the Travis logic and as much as I dislike Travis for being slow and
having network induced failures, it's still a valid and supported CI
flow that we can't drop, so please keep it in sync with these
improvements, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/50e99e93/attachment.sig>

[PATCH 13/20] gitlab: Use bash to avoid needing a_test_which_does_not_exist

[Tom Rini]

On Fri, Mar 06, 2020 at 08:07:27PM -0700, Simon Glass wrote:
> Bash allows for variables to expand only if non-empty:
> 
> 	$ var=test
> 	$ echo ${var:+"$var"}
> 	test
> 	$ echo ${var:+"-k $var"}
> 	-k test
> 	$ var=
> 	$ echo ${var:+"-k $var"}
> 
> Use this feature to avoid the workaround.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  .gitlab-ci.yml | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index bbd05aa872..05f56c6d19 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -36,17 +36,13 @@ stages:
>        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
>          exit $ret;
>        fi
> -    # "not a_test_which_does_not_exist" is a dummy -k parameter which will
> -    # never prevent any test from running. That way, we can always pass
> -    # "-k something" even when $TEST_PY_TEST_SPEC doesnt need a custom
> -    # value.
>      - virtualenv -p /usr/bin/python3 /tmp/venv
>      - . /tmp/venv/bin/activate
>      - pip install -r test/py/requirements.txt
>      - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH};
>        export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci;
>        ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID}
> -        -k "${TEST_PY_TEST_SPEC:-not a_test_which_does_not_exist}"
> +        ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"}
>          --build-dir "$UBOOT_TRAVIS_BUILD_DIR";
>        ret=$?;
>        if [[ $ret -ne 0 ]]; then

Please change the comment to note that other less-than-obvious bash
feature being used here, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/c59dd63c/attachment.sig>

[PATCH 12/20] gitlab: Use -w flag for all builds

[Tom Rini]

On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> Avoid needing to know about the internal .bm-work directory, by passing
> the -w flag to buildman.
> 
> Also drop the repeated call to buildman since the first one should show
> all the expected output. We only need to use -s if we are building
> multiple boards and want the errors to be coalesced. In this case we are
> only building a single board.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  .gitlab-ci.yml | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index b29d59d942..bbd05aa872 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -29,11 +29,11 @@ stages:
>    script:
>      # From buildman, exit code 129 means warnings only.  If we've been asked to
>      # use clang only do one configuration.
> +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
>      - ret=0;
> -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> -        || ret=$?;
> +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
>        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
>          exit $ret;
>        fi

The repeated call is so that when we have a CI error from buildman the
error is at the bottom of the output and we don't have to hunt for it,
so I'm not sure this is a developer-friendly change.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/0668d69c/attachment.sig>

[PATCH 15/20] gitlab: Use the buildman -W flag

[Tom Rini]

On Fri, Mar 06, 2020 at 08:07:29PM -0700, Simon Glass wrote:

> It doesn't seem to make sense to tell buildman to report warning as errors
> (thus ensuring there are no warnings) and then ignore the warnings.
> 
> We can simplify the logic now that we can tell buildman to ignore
> warnings.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  .gitlab-ci.yml | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 05f56c6d19..9de6592d1a 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -30,12 +30,8 @@ stages:
>      # From buildman, exit code 129 means warnings only.  If we've been asked to
>      # use clang only do one configuration.
>      - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> -    - ret=0;
> -      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> -        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> -      if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> -        exit $ret;
> -      fi
> +    - tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -W
> +        --board ${TEST_PY_BD} ${OVERRIDE}
>      - virtualenv -p /usr/bin/python3 /tmp/venv
>      - . /tmp/venv/bin/activate
>      - pip install -r test/py/requirements.txt

Maybe this whole bit of logic needs some re-thinking.  We pass -E to get
warnings-as-errors so I think it's better to drop this new flag, fix the
test to just check for non-zero and a Fixes tag for
329f5ef51d2e8de79fe2e846f2c2905da9530a71 as that should have done this
in the first place I think.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200309/a43aa0df/attachment.sig>

[PATCH 19/20] test/py: Use buildman to build U-Boot

[Simon Glass]

Hi Stephen,

On Mon, 9 Mar 2020 at 11:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 3/6/20 8:07 PM, Simon Glass wrote:
> > It is a pain to have to set the ARCH and CROSS_COMPILE environment
> > variables when using test.py's --build option. It is possible to get these
> > using the -A and -a options from buildman. But it seems better to just use
> > buildman to do the build.
> >
> > Remove the manual 'make' logic in test/py and use buildman instead.
>
> I far prefer using make here; this requires zero setup of buildman (e.g.
> the config file and specific toolchains), and so it much *less* of a pain.

For people who don't have an existing setup though, it is tricky. And
having to set those environment variables is a hassle.

'buildman --fetch-arch all' will download and set up toolchains.

Perhaps we could make this optional?

Regards,
Simon

[PATCH 07/20] gitlab: Use the --board buildman flag

[Simon Glass]

Hi Tom,

On Mon, 9 Mar 2020 at 11:46, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Mar 06, 2020 at 08:07:21PM -0700, Simon Glass wrote:
>
> > The current method of selecting the board to build is a bit error-prone,
> > e.g. with "^sandbox$" it actually builds 5 boards (all of those in the
> > sandbox architecture).
>
> OK, hold up.  You've reminded me of something that's been broken for a
> while and I forgot to follow up on.  Which is that at some point we
> stopped applying regex to either only the defconfig name, or stopped
> checking that as the first one.  The example in question used to only
> build sandbox_defconfig, but now (and for some time) has built all of
> arch or soc sandbox.

Are you sure we even used just the defconfig name? I don't recall
that. Do you know when this was?

Since 'sandbox' is an arch name this will build anything sandbox
board, and I think that is intended.

>
> That said, I'm not sure that at the end of the day I'm opposed to the
> --board flag instead.

OK.

Regards,
Simon

[PATCH 19/20] test/py: Use buildman to build U-Boot

[Simon Glass]

Hi Tom,

On Mon, 9 Mar 2020 at 11:42, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Mar 09, 2020 at 11:10:55AM -0600, Stephen Warren wrote:
> > On 3/6/20 8:07 PM, Simon Glass wrote:
> > > It is a pain to have to set the ARCH and CROSS_COMPILE environment
> > > variables when using test.py's --build option. It is possible to get these
> > > using the -A and -a options from buildman. But it seems better to just use
> > > buildman to do the build.
> > >
> > > Remove the manual 'make' logic in test/py and use buildman instead.
> >
> > I far prefer using make here; this requires zero setup of buildman (e.g. the
> > config file and specific toolchains), and so it much *less* of a pain.
>
> I have to agree here.  Keeping our test suite as dependency-free as
> possible is important.  But... that's also not what's going on in the
> code.  We don't set ARCH from what I can see, and of course don't use
> it.  We don't set the CROSS_COMPILER from the snippet in question, only
> the output directory.  Today, looking at the Travis/GitLab CI scripts we
> don't even build via test.py but rather buildman prior to calling
> test.py.  And I don't think I saw that changing in this series either.

I mean that to run pytest I have to do:

PATH=$PATH:tools/buildman ARCH=`buildman -a zynq_zybo`
CROSS_COMPILE=`buildman -A zynq_zybo` \
test/py/test.py -B zynq_zybo --id sjg-zynq_zybo --build-dir
../current/zynq_zybo --build

which is a bit of a pain.

With this change I can do:

test/py/test.py -B zynq_zybo --id sjg-zynq_zybo --build-dir
../current/zynq_zybo --build

Regards,
Simon

[PATCH 19/20] test/py: Use buildman to build U-Boot

[Tom Rini]

On Tue, Mar 10, 2020 at 08:27:47PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 9 Mar 2020 at 11:42, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Mar 09, 2020 at 11:10:55AM -0600, Stephen Warren wrote:
> > > On 3/6/20 8:07 PM, Simon Glass wrote:
> > > > It is a pain to have to set the ARCH and CROSS_COMPILE environment
> > > > variables when using test.py's --build option. It is possible to get these
> > > > using the -A and -a options from buildman. But it seems better to just use
> > > > buildman to do the build.
> > > >
> > > > Remove the manual 'make' logic in test/py and use buildman instead.
> > >
> > > I far prefer using make here; this requires zero setup of buildman (e.g. the
> > > config file and specific toolchains), and so it much *less* of a pain.
> >
> > I have to agree here.  Keeping our test suite as dependency-free as
> > possible is important.  But... that's also not what's going on in the
> > code.  We don't set ARCH from what I can see, and of course don't use
> > it.  We don't set the CROSS_COMPILER from the snippet in question, only
> > the output directory.  Today, looking at the Travis/GitLab CI scripts we
> > don't even build via test.py but rather buildman prior to calling
> > test.py.  And I don't think I saw that changing in this series either.
> 
> I mean that to run pytest I have to do:
> 
> PATH=$PATH:tools/buildman ARCH=`buildman -a zynq_zybo`
> CROSS_COMPILE=`buildman -A zynq_zybo` \
> test/py/test.py -B zynq_zybo --id sjg-zynq_zybo --build-dir
> ../current/zynq_zybo --build
> 
> which is a bit of a pain.
> 
> With this change I can do:
> 
> test/py/test.py -B zynq_zybo --id sjg-zynq_zybo --build-dir
> ../current/zynq_zybo --build

Right.  The commit message isn't clear as the CI loops build the board
with buildman first.  Second, we don't use ARCH= when building U-Boot,
so we could just drop that from buildman I suspect.  Third, no, I think
it's important to NOT require buildman to be builder here and setting
CROSS_COMPILE in the environment is fine and makes integration with
other systems easier.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200312/650cf31a/attachment.sig>

[PATCH 12/20] gitlab: Use -w flag for all builds

[Simon Glass]

Hi Tom,

On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > Avoid needing to know about the internal .bm-work directory, by passing
> > the -w flag to buildman.
> >
> > Also drop the repeated call to buildman since the first one should show
> > all the expected output. We only need to use -s if we are building
> > multiple boards and want the errors to be coalesced. In this case we are
> > only building a single board.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  .gitlab-ci.yml | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index b29d59d942..bbd05aa872 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -29,11 +29,11 @@ stages:
> >    script:
> >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> >      # use clang only do one configuration.
> > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> >      - ret=0;
> > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > -        || ret=$?;
> > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> >          exit $ret;
> >        fi
>
> The repeated call is so that when we have a CI error from buildman the
> error is at the bottom of the output and we don't have to hunt for it,
> so I'm not sure this is a developer-friendly change.

I don't quite get this, since the two buildman calls are one after the
other. What difference do you see in the output?

Regards,
Simon

[PATCH 00/20] gitlab: Simplify the test script

[Simon Glass]

Hi Tom,

On Mon, 9 Mar 2020 at 11:55, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Mar 06, 2020 at 08:07:14PM -0700, Simon Glass wrote:
>
> > At present there are several things in the gitlab script which work around
> > limitations in buildman. With a few small feature additions these can be
> > removed.
> >
> > This series adds some new features to buildman and simplifies the script:
> > - Option to run a single build in a specified output directory
> > - Allow ignoring warnings
> > - Removes a restriction on the build output directory
> >
> > It also
> > - moves test.py over to use buildman for the --build option
> > - makes one change to azure since the same approach should be possible there
> > - fixes a few minor problems noticed in main and sandbox docs
>
> One general comment is that while it's clear from this series that
> you're focusing on test.py invocation most of the time, a lot of the
> commit messages aren't clear that you're changing the
> buildman_and_testpy_template stanza.

I'm not sure what you are looking for there. Do you want the commit
message to mention which part of the gitlab file is being changed?

>
> Also, while you're keeping Azure in sync (as it's largely based on the
> GitLab logic), Travis is being left out.  But the GitLab logic is based
> on the Travis logic and as much as I dislike Travis for being slow and
> having network induced failures, it's still a valid and supported CI
> flow that we can't drop, so please keep it in sync with these
> improvements, thanks!

OK.

Regards,
Simon

[PATCH 00/20] gitlab: Simplify the test script

[Tom Rini]

On Sat, Mar 14, 2020 at 09:10:09PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 9 Mar 2020 at 11:55, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Mar 06, 2020 at 08:07:14PM -0700, Simon Glass wrote:
> >
> > > At present there are several things in the gitlab script which work around
> > > limitations in buildman. With a few small feature additions these can be
> > > removed.
> > >
> > > This series adds some new features to buildman and simplifies the script:
> > > - Option to run a single build in a specified output directory
> > > - Allow ignoring warnings
> > > - Removes a restriction on the build output directory
> > >
> > > It also
> > > - moves test.py over to use buildman for the --build option
> > > - makes one change to azure since the same approach should be possible there
> > > - fixes a few minor problems noticed in main and sandbox docs
> >
> > One general comment is that while it's clear from this series that
> > you're focusing on test.py invocation most of the time, a lot of the
> > commit messages aren't clear that you're changing the
> > buildman_and_testpy_template stanza.
> 
> I'm not sure what you are looking for there. Do you want the commit
> message to mention which part of the gitlab file is being changed?

I mean a lot of places say something like "Change gitlab ..." but aren't
changing any of the world builds, only test.py stuff, so I would prefer
"Change gitlab when using test.py ..."
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200315/0b3dd1b1/attachment.sig>

[PATCH 12/20] gitlab: Use -w flag for all builds

[Tom Rini]

On Sat, Mar 14, 2020 at 09:10:07PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > > Avoid needing to know about the internal .bm-work directory, by passing
> > > the -w flag to buildman.
> > >
> > > Also drop the repeated call to buildman since the first one should show
> > > all the expected output. We only need to use -s if we are building
> > > multiple boards and want the errors to be coalesced. In this case we are
> > > only building a single board.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  .gitlab-ci.yml | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > index b29d59d942..bbd05aa872 100644
> > > --- a/.gitlab-ci.yml
> > > +++ b/.gitlab-ci.yml
> > > @@ -29,11 +29,11 @@ stages:
> > >    script:
> > >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> > >      # use clang only do one configuration.
> > > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > >      - ret=0;
> > > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > > -        || ret=$?;
> > > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> > >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> > >          exit $ret;
> > >        fi
> >
> > The repeated call is so that when we have a CI error from buildman the
> > error is at the bottom of the output and we don't have to hunt for it,
> > so I'm not sure this is a developer-friendly change.
> 
> I don't quite get this, since the two buildman calls are one after the
> other. What difference do you see in the output?

Instead of having the errors be throughout the page we see something
like:
arch: +BOARD1 BOARD2
+(BOARD1,BOARD2) error..

At the bottom of the page.  So you open the failed build link, hit "End"
and there's where to find what to fix.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200315/67f4f870/attachment.sig>

[PATCH 00/20] gitlab: Simplify the test script

[Simon Glass]

Hi Tom,

On Sun, 15 Mar 2020 at 07:02, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Mar 14, 2020 at 09:10:09PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 9 Mar 2020 at 11:55, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Mar 06, 2020 at 08:07:14PM -0700, Simon Glass wrote:
> > >
> > > > At present there are several things in the gitlab script which work around
> > > > limitations in buildman. With a few small feature additions these can be
> > > > removed.
> > > >
> > > > This series adds some new features to buildman and simplifies the script:
> > > > - Option to run a single build in a specified output directory
> > > > - Allow ignoring warnings
> > > > - Removes a restriction on the build output directory
> > > >
> > > > It also
> > > > - moves test.py over to use buildman for the --build option
> > > > - makes one change to azure since the same approach should be possible there
> > > > - fixes a few minor problems noticed in main and sandbox docs
> > >
> > > One general comment is that while it's clear from this series that
> > > you're focusing on test.py invocation most of the time, a lot of the
> > > commit messages aren't clear that you're changing the
> > > buildman_and_testpy_template stanza.
> >
> > I'm not sure what you are looking for there. Do you want the commit
> > message to mention which part of the gitlab file is being changed?
>
> I mean a lot of places say something like "Change gitlab ..." but aren't
> changing any of the world builds, only test.py stuff, so I would prefer
> "Change gitlab when using test.py ..."

OK I will try. I've got the changes to all three environments in a
single commit at present, so we don't end up with double the commis.
Or would you prefer it split out?

One more thing...I notice that gitlab and azure use 'set -ex' to avoid
needing to check errors in the script. Is is possible for travis to do
that do, or is there some restriction?

> Thanks!
>
> --
> Tom

Regards,
Simon

[PATCH 12/20] gitlab: Use -w flag for all builds

[Simon Glass]

Hi Tom,

On Sun, 15 Mar 2020 at 07:03, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Mar 14, 2020 at 09:10:07PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > > > Avoid needing to know about the internal .bm-work directory, by passing
> > > > the -w flag to buildman.
> > > >
> > > > Also drop the repeated call to buildman since the first one should show
> > > > all the expected output. We only need to use -s if we are building
> > > > multiple boards and want the errors to be coalesced. In this case we are
> > > > only building a single board.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > >  .gitlab-ci.yml | 9 ++++-----
> > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > > index b29d59d942..bbd05aa872 100644
> > > > --- a/.gitlab-ci.yml
> > > > +++ b/.gitlab-ci.yml
> > > > @@ -29,11 +29,11 @@ stages:
> > > >    script:
> > > >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> > > >      # use clang only do one configuration.
> > > > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > > >      - ret=0;
> > > > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > > > -        || ret=$?;
> > > > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > > > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> > > >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > > > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> > > >          exit $ret;
> > > >        fi
> > >
> > > The repeated call is so that when we have a CI error from buildman the
> > > error is at the bottom of the output and we don't have to hunt for it,
> > > so I'm not sure this is a developer-friendly change.
> >
> > I don't quite get this, since the two buildman calls are one after the
> > other. What difference do you see in the output?
>
> Instead of having the errors be throughout the page we see something
> like:
> arch: +BOARD1 BOARD2
> +(BOARD1,BOARD2) error..
>
> At the bottom of the page.  So you open the failed build link, hit "End"
> and there's where to find what to fix.

Yes I see. But in this case we are only building a single board so
there should be no difference.

BTW it looks like we are not using the -l flag for the 'multiple
build' case, so we shouldn't see the (BOARD1,BOARD2) thing.

Regards,
Simon

[PATCH 00/20] gitlab: Simplify the test script

[Tom Rini]

On Sun, Mar 15, 2020 at 09:07:48AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 15 Mar 2020 at 07:02, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Mar 14, 2020 at 09:10:09PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 9 Mar 2020 at 11:55, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Mar 06, 2020 at 08:07:14PM -0700, Simon Glass wrote:
> > > >
> > > > > At present there are several things in the gitlab script which work around
> > > > > limitations in buildman. With a few small feature additions these can be
> > > > > removed.
> > > > >
> > > > > This series adds some new features to buildman and simplifies the script:
> > > > > - Option to run a single build in a specified output directory
> > > > > - Allow ignoring warnings
> > > > > - Removes a restriction on the build output directory
> > > > >
> > > > > It also
> > > > > - moves test.py over to use buildman for the --build option
> > > > > - makes one change to azure since the same approach should be possible there
> > > > > - fixes a few minor problems noticed in main and sandbox docs
> > > >
> > > > One general comment is that while it's clear from this series that
> > > > you're focusing on test.py invocation most of the time, a lot of the
> > > > commit messages aren't clear that you're changing the
> > > > buildman_and_testpy_template stanza.
> > >
> > > I'm not sure what you are looking for there. Do you want the commit
> > > message to mention which part of the gitlab file is being changed?
> >
> > I mean a lot of places say something like "Change gitlab ..." but aren't
> > changing any of the world builds, only test.py stuff, so I would prefer
> > "Change gitlab when using test.py ..."
> 
> OK I will try. I've got the changes to all three environments in a
> single commit at present, so we don't end up with double the commis.
> Or would you prefer it split out?

Doing all CIs in step makes sense.  So something like...:
Azure/GitLab/Travis: Change test.py and buildman ...

> One more thing...I notice that gitlab and azure use 'set -ex' to avoid
> needing to check errors in the script. Is is possible for travis to do
> that do, or is there some restriction?

Not sure.  When in doubt I always hack the heck out of .travis.yml to
just be a testcase of what I want to figure out, push and see.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200315/991df2ba/attachment.sig>

[PATCH 12/20] gitlab: Use -w flag for all builds

[Tom Rini]

On Sun, Mar 15, 2020 at 09:07:54AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 15 Mar 2020 at 07:03, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Mar 14, 2020 at 09:10:07PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > > > > Avoid needing to know about the internal .bm-work directory, by passing
> > > > > the -w flag to buildman.
> > > > >
> > > > > Also drop the repeated call to buildman since the first one should show
> > > > > all the expected output. We only need to use -s if we are building
> > > > > multiple boards and want the errors to be coalesced. In this case we are
> > > > > only building a single board.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > >  .gitlab-ci.yml | 9 ++++-----
> > > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > > > index b29d59d942..bbd05aa872 100644
> > > > > --- a/.gitlab-ci.yml
> > > > > +++ b/.gitlab-ci.yml
> > > > > @@ -29,11 +29,11 @@ stages:
> > > > >    script:
> > > > >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> > > > >      # use clang only do one configuration.
> > > > > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > > > >      - ret=0;
> > > > > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > > > > -        || ret=$?;
> > > > > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > > > > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> > > > >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > > > > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> > > > >          exit $ret;
> > > > >        fi
> > > >
> > > > The repeated call is so that when we have a CI error from buildman the
> > > > error is at the bottom of the output and we don't have to hunt for it,
> > > > so I'm not sure this is a developer-friendly change.
> > >
> > > I don't quite get this, since the two buildman calls are one after the
> > > other. What difference do you see in the output?
> >
> > Instead of having the errors be throughout the page we see something
> > like:
> > arch: +BOARD1 BOARD2
> > +(BOARD1,BOARD2) error..
> >
> > At the bottom of the page.  So you open the failed build link, hit "End"
> > and there's where to find what to fix.
> 
> Yes I see. But in this case we are only building a single board so
> there should be no difference.
> 
> BTW it looks like we are not using the -l flag for the 'multiple
> build' case, so we shouldn't see the (BOARD1,BOARD2) thing.

Perhaps this is a good example of why I'm asking for the commit message
to be clearer about the tests being changed :)  But isn't this also the
same build area as the general builds?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200315/c03855e0/attachment.sig>

[PATCH 12/20] gitlab: Use -w flag for all builds

[Simon Glass]

Hi Tom,

On Sun, 15 Mar 2020 at 09:23, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Mar 15, 2020 at 09:07:54AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sun, 15 Mar 2020 at 07:03, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Mar 14, 2020 at 09:10:07PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > > > > > Avoid needing to know about the internal .bm-work directory, by passing
> > > > > > the -w flag to buildman.
> > > > > >
> > > > > > Also drop the repeated call to buildman since the first one should show
> > > > > > all the expected output. We only need to use -s if we are building
> > > > > > multiple boards and want the errors to be coalesced. In this case we are
> > > > > > only building a single board.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > >  .gitlab-ci.yml | 9 ++++-----
> > > > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > > > > index b29d59d942..bbd05aa872 100644
> > > > > > --- a/.gitlab-ci.yml
> > > > > > +++ b/.gitlab-ci.yml
> > > > > > @@ -29,11 +29,11 @@ stages:
> > > > > >    script:
> > > > > >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> > > > > >      # use clang only do one configuration.
> > > > > > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > > > > >      - ret=0;
> > > > > > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > > > > > -        || ret=$?;
> > > > > > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > > > > > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> > > > > >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > > > > > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> > > > > >          exit $ret;
> > > > > >        fi
> > > > >
> > > > > The repeated call is so that when we have a CI error from buildman the
> > > > > error is at the bottom of the output and we don't have to hunt for it,
> > > > > so I'm not sure this is a developer-friendly change.
> > > >
> > > > I don't quite get this, since the two buildman calls are one after the
> > > > other. What difference do you see in the output?
> > >
> > > Instead of having the errors be throughout the page we see something
> > > like:
> > > arch: +BOARD1 BOARD2
> > > +(BOARD1,BOARD2) error..
> > >
> > > At the bottom of the page.  So you open the failed build link, hit "End"
> > > and there's where to find what to fix.
> >
> > Yes I see. But in this case we are only building a single board so
> > there should be no difference.
> >
> > BTW it looks like we are not using the -l flag for the 'multiple
> > build' case, so we shouldn't see the (BOARD1,BOARD2) thing.
>
> Perhaps this is a good example of why I'm asking for the commit message
> to be clearer about the tests being changed :)  But isn't this also the
> same build area as the general builds?

Yes I'll update the commit subjects.

This is the buildman_and_testpy_template section, which is only used
to build a single board, so far as I can tell.

Regards,
Simon

[PATCH 12/20] gitlab: Use -w flag for all builds

[Tom Rini]

On Sun, Mar 15, 2020 at 09:50:30AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 15 Mar 2020 at 09:23, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Mar 15, 2020 at 09:07:54AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sun, 15 Mar 2020 at 07:03, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sat, Mar 14, 2020 at 09:10:07PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > > > > > > Avoid needing to know about the internal .bm-work directory, by passing
> > > > > > > the -w flag to buildman.
> > > > > > >
> > > > > > > Also drop the repeated call to buildman since the first one should show
> > > > > > > all the expected output. We only need to use -s if we are building
> > > > > > > multiple boards and want the errors to be coalesced. In this case we are
> > > > > > > only building a single board.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > ---
> > > > > > >
> > > > > > >  .gitlab-ci.yml | 9 ++++-----
> > > > > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > > > > >
> > > > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > > > > > index b29d59d942..bbd05aa872 100644
> > > > > > > --- a/.gitlab-ci.yml
> > > > > > > +++ b/.gitlab-ci.yml
> > > > > > > @@ -29,11 +29,11 @@ stages:
> > > > > > >    script:
> > > > > > >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> > > > > > >      # use clang only do one configuration.
> > > > > > > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > > > > > >      - ret=0;
> > > > > > > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > > > > > > -        || ret=$?;
> > > > > > > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > > > > > > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> > > > > > >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > > > > > > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> > > > > > >          exit $ret;
> > > > > > >        fi
> > > > > >
> > > > > > The repeated call is so that when we have a CI error from buildman the
> > > > > > error is at the bottom of the output and we don't have to hunt for it,
> > > > > > so I'm not sure this is a developer-friendly change.
> > > > >
> > > > > I don't quite get this, since the two buildman calls are one after the
> > > > > other. What difference do you see in the output?
> > > >
> > > > Instead of having the errors be throughout the page we see something
> > > > like:
> > > > arch: +BOARD1 BOARD2
> > > > +(BOARD1,BOARD2) error..
> > > >
> > > > At the bottom of the page.  So you open the failed build link, hit "End"
> > > > and there's where to find what to fix.
> > >
> > > Yes I see. But in this case we are only building a single board so
> > > there should be no difference.
> > >
> > > BTW it looks like we are not using the -l flag for the 'multiple
> > > build' case, so we shouldn't see the (BOARD1,BOARD2) thing.
> >
> > Perhaps this is a good example of why I'm asking for the commit message
> > to be clearer about the tests being changed :)  But isn't this also the
> > same build area as the general builds?
> 
> Yes I'll update the commit subjects.
> 
> This is the buildman_and_testpy_template section, which is only used
> to build a single board, so far as I can tell.

OK then yes, the cases where we are building a single board it would
make sense to not do the second build.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200315/b6d05483/attachment.sig>

[PATCH 12/20] gitlab: Use -w flag for all builds

[Simon Glass]

Hi Tom,

On Sun, 15 Mar 2020 at 10:18, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Mar 15, 2020 at 09:50:30AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sun, 15 Mar 2020 at 09:23, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Mar 15, 2020 at 09:07:54AM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sun, 15 Mar 2020 at 07:03, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sat, Mar 14, 2020 at 09:10:07PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 9 Mar 2020 at 11:58, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Fri, Mar 06, 2020 at 08:07:26PM -0700, Simon Glass wrote:
> > > > > > > > Avoid needing to know about the internal .bm-work directory, by passing
> > > > > > > > the -w flag to buildman.
> > > > > > > >
> > > > > > > > Also drop the repeated call to buildman since the first one should show
> > > > > > > > all the expected output. We only need to use -s if we are building
> > > > > > > > multiple boards and want the errors to be coalesced. In this case we are
> > > > > > > > only building a single board.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > ---
> > > > > > > >
> > > > > > > >  .gitlab-ci.yml | 9 ++++-----
> > > > > > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > > > > > > index b29d59d942..bbd05aa872 100644
> > > > > > > > --- a/.gitlab-ci.yml
> > > > > > > > +++ b/.gitlab-ci.yml
> > > > > > > > @@ -29,11 +29,11 @@ stages:
> > > > > > > >    script:
> > > > > > > >      # From buildman, exit code 129 means warnings only.  If we've been asked to
> > > > > > > >      # use clang only do one configuration.
> > > > > > > > +    - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD}
> > > > > > > >      - ret=0;
> > > > > > > > -      tools/buildman/buildman -o /tmp -P -E --board ${TEST_PY_BD} ${OVERRIDE}
> > > > > > > > -        || ret=$?;
> > > > > > > > +      tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E
> > > > > > > > +        --board ${TEST_PY_BD} ${OVERRIDE} || ret=$?;
> > > > > > > >        if [[ $ret -ne 0 && $ret -ne 129 ]]; then
> > > > > > > > -        tools/buildman/buildman -o /tmp -seP --board ${TEST_PY_BD};
> > > > > > > >          exit $ret;
> > > > > > > >        fi
> > > > > > >
> > > > > > > The repeated call is so that when we have a CI error from buildman the
> > > > > > > error is at the bottom of the output and we don't have to hunt for it,
> > > > > > > so I'm not sure this is a developer-friendly change.
> > > > > >
> > > > > > I don't quite get this, since the two buildman calls are one after the
> > > > > > other. What difference do you see in the output?
> > > > >
> > > > > Instead of having the errors be throughout the page we see something
> > > > > like:
> > > > > arch: +BOARD1 BOARD2
> > > > > +(BOARD1,BOARD2) error..
> > > > >
> > > > > At the bottom of the page.  So you open the failed build link, hit "End"
> > > > > and there's where to find what to fix.
> > > >
> > > > Yes I see. But in this case we are only building a single board so
> > > > there should be no difference.
> > > >
> > > > BTW it looks like we are not using the -l flag for the 'multiple
> > > > build' case, so we shouldn't see the (BOARD1,BOARD2) thing.
> > >
> > > Perhaps this is a good example of why I'm asking for the commit message
> > > to be clearer about the tests being changed :)  But isn't this also the
> > > same build area as the general builds?
> >
> > Yes I'll update the commit subjects.
> >
> > This is the buildman_and_testpy_template section, which is only used
> > to build a single board, so far as I can tell.
>
> OK then yes, the cases where we are building a single board it would
> make sense to not do the second build.  Thanks!

OK.

Also I think I got my 'kea' gitlab runner working. Do you want to try
to add it to the group runners?

Regards,
Simon