makefiles/buildtest: always execute 'buildtest' loop on host machine #11857
Conversation
MrKevinWeiss
added
Area: build system
|
Does build time increase in a significant way because docker is started per compilation? |
|
It does indeed increase I will do a measure on my machine. However, if not changing it because of execution time, I would see it as a "disable test as it takes more time". |
|
The time increase is indeed significant. It changes from 10 to 20 minutes for Should it be split in two different targets? So |
|
There is something I don't quite understand here, why should building in |
|
It MUST not use the host toolchain. That is why when testing building with However, the Lines 460 to 467 in 950b83e But some applications, are currently handled in a way that the build system still executes targets outside of this handling:
Consequence on
|
|
@cladmi Ok now I understand the issue correctly.
+1 for this approach, all tough I would rename it to |
|
Murdock is not using the It is only during release, or developers on their machine that would run Good for |
|
PR needs squashing. |
|
The testing prodecures from the first post are still valid and get the same output. The new
|
cladmi
force-pushed
the
pr/make/buildtest/docker
branch
from
cad1233 to
75ea03c
Compare
|
I rebased now that #11083 was merged and updated the testing procedure in the PR description to use |
|
Verified test procedure. master: BUILDTEST_MAKE_REDIRECT= BUILD_IN_DOCKER=1 BOARDS='native samr21-xpro' make --no-print-directory -C examples/default buildtestPR: BUILDTEST_MAKE_REDIRECT= BUILD_IN_DOCKER=1 BOARDS='native samr21-xpro' make --no-print-directory -C examples/default buildtestmaster:BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtestPR:BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtestPR:BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtest-indocker |
fjmolinas
added
CI: needs squashing
There was a problem hiding this comment.
I agree with the changes this PR introduces, it exposes issues when building in docker while allowing to keep the same behavior as before when using the buildtest-target. I was able to reproduce the test procedure correctly for the case that exposed the issue in #11842. ACK, please squash.
Add a 'buildtest-indocker' that forces executing 'buildtest' for loop completely inside the container. It prevents starting one container per compilation wich is slower but it could hide errors where the host toolchain would be used It is currently equivalent to `buildtest` but will change when the `buidtest` handling will be move outside of `BUILD_IN_DOCKER`. Display an error when executed without BUILD_IN_DOCKER=1.
This remove executing buildtest `for` loop in docker.
When building completely in docker, 'buildtest' would hide issues when
the host toolchain would be used when doing `make all` directly.
It has the consequence that it now starts a container for each
compilation which is slower.
The previous behavior can be reproduced by using
BUILD_IN_DOCKER=1 make buildtest-indocker
A side effect is also that now `BUILDTEST_MAKE_REDIRECT` would work when
doing `buildtest` with docker.
cladmi
force-pushed
the
pr/make/buildtest/docker
branch
from
75ea03c to
7d10da8
Compare
|
I squashed the commits and commit message. |
fjmolinas
added
CI: ready for build
|
All green, GO! |
|
Thank you for the review! :) |
Contribution description
This remove executing buildtest
forloop in docker.When building completely in docker, 'buildtest' would hide issues when
the host toolchain would be used when doing
make alldirectly.A side effect is also that now
BUILDTEST_MAKE_REDIRECTwould work whendoing
buildtestwith docker.This also now removes passing
BOARDSas it was added and needed onlyfor 'buildtest', and should work without the specific handling. #11385
The side effect is that now one
dockerinstance should be started per compilation but makes it more consistent with doingBUILD_IN_DOCKER=1 BOARD=board_name makefor each board.Testing procedure
I locally used
DOCKER="sudo docker"but did not include it in the tests commands displayed below.Normal buildtest uses
Running
buildtestwithBUILD_IN_DOCKER=1still works.BOARDScan still be limited:When setting
BUILDTEST_MAKE_REDIRECT=in the environment it will be taken into accountBUILDTEST_MAKE_REDIRECT= BUILD_IN_DOCKER=1 BOARDS='native samr21-xpro' make --no-print-directory -C examples/default buildtestIn master it was showing the
dockercommand first to runbuildtestnowallis run withdockerfor each board.Master:
BUILDTEST_MAKE_REDIRECT= BUILD_IN_DOCKER=1 BOARDS='native samr21-xpro' make --no-print-directory -C examples/hello-world/ buildtestRemoves errors hiding
Use a PATH where you do not have
arm-none-eabi-gccIt now correctly fails with
buildtestwithBUILD_IN_DOCKER=1as the host toolchain is used by that application.BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtestWhen it was hiding the error and correctly compiling in
masterDOCKER="sudo docker" BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtestand also hiding the error and correctly compiling in with
buildtest-indocker.
``` BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtest-indocker make: Entering directory '/home/harter/work/git/RIOT/tests/riotboot' Launching build container using image "riot/riotbuild:latest". mkdir -p /home/harter/work/git/RIOT/build sudo docker run --rm -t -u "$(id -u)" \ -v '/usr/share/zoneinfo/Europe/Berlin:/etc/localtime:ro' -v '/home/harter/work/git/RIOT:/data/riotbuild/riotbase' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles' -v /home/harter/.gitcache:/data/riotbuild/gitcache -e GIT_CACHE_DIR=/data/riotbuild/gitcache \ -e 'BOARDS=iotlab-m3 samr21-xpro' \ -w '/data/riotbuild/riotbase/tests/riotboot/' \ 'riot/riotbuild:latest' make buildtest-indocker Building for iotlab-m3 ... success. Building for samr21-xpro ... success. make: Leaving directory '/home/harter/work/git/RIOT/tests/riotboot' ```BOARDS="iotlab-m3 samr21-xpro" PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/riotboot buildtest-indockermcuboot is now working since https://github.com//pull/11083
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 BUILDTEST_MAKE_REDIRECT='>/dev/null' make -C tests/mcuboot/ buildtestWhen it was hiding the error and correctly compiling in
masterPATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin BUILD_IN_DOCKER=1 make -C tests/mcuboot/ buildtestIssues/PRs references
Found that
BUILD_IN_DOCKER=1 make buildtestwas hiding expected errors during 2019.07-RC1 testing RIOT-OS/Release-Specs#128 (comment)Fixes #11842