Skip to content

makefiles/docker: prevent recursive docker invocation#20638

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
mguetschow:docker-makefile-prevent-recursion
Apr 29, 2024
Merged

makefiles/docker: prevent recursive docker invocation#20638
maribu merged 1 commit intoRIOT-OS:masterfrom
mguetschow:docker-makefile-prevent-recursion

Conversation

@mguetschow
Copy link
Copy Markdown
Contributor

@mguetschow mguetschow commented Apr 29, 2024

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
Copy Markdown
Member

maribu commented Apr 29, 2024

Good catch!

@riot-ci
Copy link
Copy Markdown

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
@mguetschow mguetschow deleted the docker-makefile-prevent-recursion branch April 30, 2024 07:39
@mguetschow
Copy link
Copy Markdown
Contributor Author

mguetschow commented Apr 30, 2024

Thanks :)

@mguetschow mguetschow mentioned this pull request Apr 30, 2024
Merged
@mguetschow mguetschow added this to the Release 2024.07 milestone Jul 5, 2024
@crasbe crasbe mentioned this pull request Nov 20, 2025
Merged
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