Skip to content

make: fix BUILD_IN_DOCKER when RIOT is submodule#10550

Merged
jcarrano merged 1 commit intoRIOT-OS:masterfrom
smlng:pr/docker/riot_submodule
Dec 6, 2018
Merged

make: fix BUILD_IN_DOCKER when RIOT is submodule#10550
jcarrano merged 1 commit intoRIOT-OS:masterfrom
smlng:pr/docker/riot_submodule

Conversation

@smlng
Copy link
Member

@smlng smlng commented Dec 5, 2018

Contribution description

This removes a non-working mount point when using BUILD_IN_DOCKER for an
external application where RIOTBASE is a submodule within the apps repo.

Actually I'm not sure why the removed lines are in place anyway, because they are only used when RIOT is a submodule, however they likely never worked.

Testing procedure

Try to build an application with BUILD_IN_DOCKER=1 which has RIOT as a submodule within its repo and uses that as RIOTBASE. Take for instance this https://github.com/inetrg/vslab-riot and you'll get the following error:

cd /tmp
git clone https://github.com/inetrg/vslab-riot
cd vslab-riot
BUILD_IN_DOCKER=1 DOCKER="sudo docker" make -C src/
Launching build container using image "riot/riotbuild:latest".
sudo docker run --rm -t -u "$(id -u)" \
    -v '/tmp/vslab-riot/RIOT:/data/riotbuild/riotbase' \
    -v '/tmp/vslab-riot/RIOT/cpu:/data/riotbuild/riotcpu' \
    -v '/tmp/vslab-riot/RIOT/boards:/data/riotbuild/riotboard' \
    -v '/tmp/vslab-riot/RIOT/makefiles:/data/riotbuild/riotmake' \
    -v '/tmp/vslab-riot:/data/riotbuild/riotproject' \
    -v /etc/localtime:/etc/localtime:ro \
    -e 'RIOTBASE=/data/riotbuild/riotbase' \
    -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' \
    -e 'RIOTCPU=/data/riotbuild/riotcpu' \
    -e 'RIOTBOARD=/data/riotbuild/riotboard' \
    -e 'RIOTMAKE=/data/riotbuild/riotmake' \
    -e 'RIOTPROJECT=/data/riotbuild/riotproject' \
      -v src/.git:src/.git \
     \
    -w '/data/riotbuild/riotproject/src/' \
    'riot/riotbuild:latest' make
docker: Error response from daemon: invalid volume specification: 'src/.git:src/.git': invalid mount config for type "volume": invalid mount path: 'src/.git' mount path must be absolute.
See 'docker run --help'.
make: *** [..in-docker-container] Error 125

It works when you clone RIOT separately and specify RIOTBASE accordingly, e.g.

cd /tmp
git clone https://github.com/RIOT-OS/RIOT
cd /tmp/vslab-riot
RIOTBASE=/tmp/RIOT BUILD_IN_DOCKER=1 make -C src

Issues/PRs references

@smlng smlng added 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 labels Dec 5, 2018
Copy link
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could reproduce the problem, but this fix will break worktrees.

DOCKER_VOLUMES_AND_ENV += $(if $(wildcard $(GIT_CACHE_DIR)),-e GIT_CACHE_DIR=$(DOCKER_BUILD_ROOT)/gitcache)

# Handle worktree by mounting the git common dir in the same location
_is_git_worktree = $(shell grep '^gitdir: ' $(RIOTBASE)/.git 2>/dev/null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this will break worktrees, which was fixed in #10303.

@jcarrano
Copy link
Contributor

jcarrano commented Dec 5, 2018

Here are the commands I used to reproduce the issue:

$ git clone git@github.com:inetrg/vslab-riot.git
$ cd vslab-riot
$ git submodule init
$ git submodule update
$ BUILD_IN_DOCKER=1 DOCKER="sudo docker" make -C src/
make: Entering directory '/home/jcarrano/source/vslab-riot/src'
Launching build container using image "riot/riotbuild:latest".
sudo docker run --rm -t -u "$(id -u)" \
    -v '/home/jcarrano/source/vslab-riot/RIOT:/data/riotbuild/riotbase' \
    -v '/home/jcarrano/source/vslab-riot/RIOT/cpu:/data/riotbuild/riotcpu' \
    -v '/home/jcarrano/source/vslab-riot/RIOT/boards:/data/riotbuild/riotboard' \
    -v '/home/jcarrano/source/vslab-riot/RIOT/makefiles:/data/riotbuild/riotmake' \
    -v '/home/jcarrano/source/vslab-riot:/data/riotbuild/riotproject' \
    -v /etc/localtime:/etc/localtime:ro \
    -e 'RIOTBASE=/data/riotbuild/riotbase' \
    -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' \
    -e 'RIOTCPU=/data/riotbuild/riotcpu' \
    -e 'RIOTBOARD=/data/riotbuild/riotboard' \
    -e 'RIOTMAKE=/data/riotbuild/riotmake' \
    -e 'RIOTPROJECT=/data/riotbuild/riotproject' \
    -v /home/jcarrano/.gitcache:/data/riotbuild/gitcache -e GIT_CACHE_DIR=/data/riotbuild/gitcache -v ../.git:../.git \
     \
    -w '/data/riotbuild/riotproject/src/' \
    'riot/riotbuild:latest' make  
docker: Error response from daemon: invalid volume specification: '../.git:../.git': invalid mount config for type "volume": invalid mount path: '../.git' mount path must be absolute.
See 'docker run --help'.
make: *** [/home/jcarrano/source/vslab-riot/RIOT/makefiles/docker.inc.mk:106: ..in-docker-container] Error 125
make: Leaving directory '/home/jcarrano/source/vslab-riot/src'

@cladmi
Copy link
Contributor

cladmi commented Dec 5, 2018

Actually I'm not sure why the removed lines are in place anyway, because they are only used when RIOT is a submodule, however they likely never worked.

The description says worktree and not submodules.

You have more information in the commit introducing it 61a3e5d#diff-4417068c1754de9c0490bd66a2ea783e and also the detailed procedure in the PR introducing it #10303

@cladmi cladmi added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Dec 5, 2018
@smlng smlng force-pushed the pr/docker/riot_submodule branch from 74c22d3 to ae5608a Compare December 5, 2018 19:28
@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 5, 2018
@smlng
Copy link
Member Author

smlng commented Dec 5, 2018

changed as suggested by @jcarrano and it solves the problem for me, too!

@smlng
Copy link
Member Author

smlng commented Dec 6, 2018

@cladmi and @jcarrano can you have another look. I kind of need this (or at least its annoying) to use my PR branch for the RIOT submodule in a projects instead of the RIOT repo ...

Copy link
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested by using docker in with a submodule as in the description, and also verified that worktrees are still working.

@jcarrano jcarrano merged commit 25e165e into RIOT-OS:master Dec 6, 2018
@smlng smlng deleted the pr/docker/riot_submodule branch December 6, 2018 11:53
@smlng
Copy link
Member Author

smlng commented Dec 6, 2018

🎉

@cladmi
Copy link
Contributor

cladmi commented Dec 6, 2018

I prefered the old commit message though at least make: fix BUILD_IN_DOCKER when RIOT is submodule was explaining why it was changed. Now the title just says exactly what the code line does, so you need to dig into this PR to understand why it was modified.

@aabadie aabadie added this to the Release 2019.01 milestone Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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 Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants