| Message ID | 20200307030734.237401-15-sjg@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | gitlab: Simplify the test script | expand |
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.
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.
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
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
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!
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')
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(-)