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

Message ID 20200307030734.237401-15-sjg@chromium.org
State New
Headers show
Series
  • gitlab: Simplify the test script
Related show

Commit Message

Simon Glass March 7, 2020, 3:07 a.m.
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 at chromium.org>
---

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

Comments

Stephen Warren March 9, 2020, 5:10 p.m. | #1
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.
Tom Rini March 9, 2020, 5:41 p.m. | #2
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.
Simon Glass March 10, 2020, 11:22 p.m. | #3
Hi Stephen,

On Mon, 9 Mar 2020 at 11:10, Stephen Warren <swarren at 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
Simon Glass March 11, 2020, 2:27 a.m. | #4
Hi Tom,

On Mon, 9 Mar 2020 at 11:42, Tom Rini <trini at 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
Tom Rini March 12, 2020, 2:03 p.m. | #5
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 at 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!

Patch

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')