Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

makefiles/docker: prevent recursive docker invocation #20638

Merged
merged 1 commit into from
Apr 29, 2024
Merged

makefiles/docker: prevent recursive docker invocation #20638

merged 1 commit into from
Apr 29, 2024

Conversation

mguetschow
Copy link
Contributor

Contribution description

When BUILD_IN_DOCKER ?= 1 is set as default in an application Makefile, the build system currently tries to invoke docker recursively from within docker.

This PR adds BUILD_IN_DOCKER=0 as environment variable to the docker invocation to explicitly disable recursive docker.

Note that this approach still fails if the application Makefile contains an unconditional BUILD_IN_DOCKER = 1.

Testing procedure

Apply the following patch and run make -C examples/hello-world all:

diff --git a/examples/hello-world/Makefile b/examples/hello-world/Makefile
index 258d8e9baf..a05360cfd5 100644
--- a/examples/hello-world/Makefile
+++ b/examples/hello-world/Makefile
@@ -7,6 +7,8 @@ BOARD ?= native
 # This has to be the absolute path to the RIOT base directory:
 RIOTBASE ?= $(CURDIR)/../..
 
+BUILD_IN_DOCKER ?= 1
+
 # Comment this out to disable code in RIOT that does safety checking
 # which is not needed in a production environment but helps in the
 # development process:

On master, this fails with a recursive invocation of docker

make: Entering directory '/home/mikolai/TUD/Code/RIOT/examples/hello-world'
Launching build container using image "docker.io/riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Berlin:/etc/localtime:ro' -v '/home/mikolai/TUD/Code/RIOT:/data/riotbuild/riotbase:delegated' -v '/home/mikolai/.cargo/registry:/data/riotbuild/.cargo/registry:delegated' -v '/home/mikolai/.cargo/git:/data/riotbuild/.cargo/git:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -v '/home/mikolai/.riot:/data/riotbuild/build:delegated' -e 'BUILD_DIR=/data/riotbuild/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'       -e 'DISABLE_MODULE=' -e 'DEFAULT_MODULE=' -e 'FEATURES_REQUIRED=' -e 'FEATURES_BLACKLIST=' -e 'FEATURES_OPTIONAL=' -e 'USEMODULE=' -e 'USEPKG='  -w '/data/riotbuild/riotbase/examples/hello-world/' 'docker.io/riot/riotbuild:latest' make     all
/bin/sh: 1: docker: not found
Launching build container using image "docker.io/riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Etc/UTC:/etc/localtime:ro' -v '/data/riotbuild/riotbase:/data/riotbuild/riotbase:delegated' -v '/data/riotbuild/.cargo/registry:/data/riotbuild/.cargo/registry:delegated' -v '/data/riotbuild/.cargo/git:/data/riotbuild/.cargo/git:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -v '/data/riotbuild/build:/data/riotbuild/build:delegated' -e 'BUILD_DIR=/data/riotbuild/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'       -e 'DISABLE_MODULE=' -e 'DEFAULT_MODULE=' -e 'FEATURES_REQUIRED=' -e 'FEATURES_BLACKLIST=' -e 'FEATURES_OPTIONAL=' -e 'USEMODULE=' -e 'USEPKG='  -w '/data/riotbuild/riotbase/examples/hello-world/' 'docker.io/riot/riotbuild:latest' make     all
/bin/sh: 1: docker: not found
make: *** [/data/riotbuild/riotbase/makefiles/docker.inc.mk:350: ..in-docker-container] Error 127
make: *** [/home/mikolai/TUD/Code/RIOT/makefiles/docker.inc.mk:350: ..in-docker-container] Error 2
make: Leaving directory '/home/mikolai/TUD/Code/RIOT/examples/hello-world'

With this change, it builds successfully.

Issues/PRs references

Fixes #20636

@github-actions github-actions bot added the Area: build system Area: Build system label Apr 29, 2024
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 29, 2024
@mguetschow mguetschow requested a review from maribu April 29, 2024 14:55
@maribu maribu enabled auto-merge April 29, 2024 14:57
@maribu
Copy link
Member

maribu commented Apr 29, 2024

Good catch!

@riot-ci
Copy link

riot-ci commented Apr 29, 2024

Murdock results

✔️ PASSED

2863149 makefiles/docker: prevent recursive docker invocation

Success Failures Total Runtime
10065 0 10066 12m:59s

Artifacts

@maribu maribu added this pull request to the merge queue Apr 29, 2024
Merged via the queue into RIOT-OS:master with commit ff0d00c Apr 29, 2024
27 checks passed
@mguetschow mguetschow deleted the docker-makefile-prevent-recursion branch April 30, 2024 07:39
@mguetschow
Copy link
Contributor Author

Thanks :)

@mguetschow mguetschow mentioned this pull request Apr 30, 2024
Merged
@mguetschow mguetschow added this to the Release 2024.07 milestone Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maribu maribu maribu approved these changes

Assignees
No one assigned
Labels
Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: BUILD_IN_DOCKER in Makefile on WSL not working as intended / breaking the build
3 participants
@mguetschow @maribu @riot-ci