Skip to content

Makefile.include: Fix BUILDRELPATH when RIOTPROJECT is CURDIR.#10302

Merged
cladmi merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/make/docker/fix_riotproject_is_appdir
Nov 1, 2018
Merged

Makefile.include: Fix BUILDRELPATH when RIOTPROJECT is CURDIR.#10302
cladmi merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/make/docker/fix_riotproject_is_appdir

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Oct 30, 2018 •
edited
Loading

Contribution description

This fixes building an example that is outside of RIOT with docker and
also fixes building from a release archive.

This currently prevents building from the release archive with our docker image.

It is the first commit split from #9646 so that it could go in the release.

Testing procedure

It does not compile in master as the build path is wrongly set:

-w '/data/riotbuild/riotproject//home/harter/work/git/riot_master_test/examples/hello-world/' \

# Simulate a release archive
git archive --format=tar --prefix=riot_master_test/ riot/master  | tar -C .. -x

# compiling fails with docker
DOCKER="sudo docker" BUILD_IN_DOCKER=1 make -C ../riot_master_test/examples/hello-world/
make: Entering directory '/home/harter/work/git/riot_master_test/examples/hello-world'
Launching build container using image "riot/riotbuild:latest".
sudo docker run --rm -t -u "$(id -u)" \
    -v '/home/harter/work/git/riot_master_test:/data/riotbuild/riotbase' \
    -v '/home/harter/work/git/riot_master_test/cpu:/data/riotbuild/riotcpu' \
    -v '/home/harter/work/git/riot_master_test/boards:/data/riotbuild/riotboard' \
    -v '/home/harter/work/git/riot_master_test/makefiles:/data/riotbuild/riotmake' \
    -v '/home/harter/work/git/riot_master_test/examples/hello-world:/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/harter/.gitcache:/data/riotbuild/gitcache -e GIT_CACHE_DIR=/data/riotbuild/gitcache \
     \
    -w '/data/riotbuild/riotproject//home/harter/work/git/riot_master_test/examples/hello-world/' \
    'riot/riotbuild:latest' make
make: *** No targets specified and no makefile found.  Stop.
/home/harter/work/git/riot_master_test/makefiles/docker.inc.mk:100: recipe for target '..in-docker-container' failed
make: *** [..in-docker-container] Error 2
make: Leaving directory '/home/harter/work/git/riot_master_test/examples/hello-world'

With this PR it is set correctly -w '/data/riotbuild/riotproject/' \ and it compiles:

git archive --format=tar --prefix=riot_buildrelpath_fix/ HEAD  | tar -C .. -x

DOCKER="sudo docker" BUILD_IN_DOCKER=1 make -C ../riot_buildrelpath_fix/examples/hello-world/
make: Entering directory '/home/harter/work/git/riot_buildrelpath_fix/examples/hello-world'
Launching build container using image "riot/riotbuild:latest".
sudo docker run --rm -t -u "$(id -u)" \
    -v '/home/harter/work/git/riot_buildrelpath_fix:/data/riotbuild/riotbase' \
    -v '/home/harter/work/git/riot_buildrelpath_fix/cpu:/data/riotbuild/riotcpu' \
    -v '/home/harter/work/git/riot_buildrelpath_fix/boards:/data/riotbuild/riotboard' \
    -v '/home/harter/work/git/riot_buildrelpath_fix/makefiles:/data/riotbuild/riotmake' \
    -v '/home/harter/work/git/riot_buildrelpath_fix/examples/hello-world:/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/harter/.gitcache:/data/riotbuild/gitcache -e GIT_CACHE_DIR=/data/riotbuild/gitcache \
     \
    -w '/data/riotbuild/riotproject/' \
    'riot/riotbuild:latest' make
Building application "hello-world" for "native" with MCU "native".

"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/sys/auto_init
"make" -C /data/riotbuild/riotboard/native
"make" -C /data/riotbuild/riotboard/native/drivers
"make" -C /data/riotbuild/riotcpu/native
"make" -C /data/riotbuild/riotcpu/native/periph
"make" -C /data/riotbuild/riotcpu/native/vfs
   text    data     bss     dec     hex filename
  20641     372   47684   68697   10c59 /data/riotbuild/riotproject/bin/native/hello-world.elf
make: Leaving directory '/home/harter/work/git/riot_buildrelpath_fix/examples/hello-world'

Another testing procedure is to copy an application outside of a git repository.
The behavior is the same as with the archive.

cp -r examples/hello-world/ ..
RIOTBASE=${PWD}  DOCKER="sudo docker" BUILD_IN_DOCKER=1 make -C ../hello-world/ clean all

Issues/PRs references

The "external application" case was found in #9646 and the from an archive case was found during release tests RIOT-OS/Release-Specs#76

@cladmi cladmi added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … labels Oct 30, 2018
@cladmi cladmi added this to the Release 2018.10 milestone Oct 30, 2018
@cladmi cladmi requested review from jia200x, jnohlgard and smlng October 30, 2018 15:42
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Oct 30, 2018

I will give it a look now. Thanks

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Oct 30, 2018

Without PR:

make: Entering directory '/tmp/hello-world'
Launching build container using image "riot/riotbuild:latest".
sudo docker run --rm -t -u "$(id -u)" \
    -v '/tmp/RIOT:/data/riotbuild/riotbase' \
    -v '/tmp/RIOT/cpu:/data/riotbuild/riotcpu' \
    -v '/tmp/RIOT/boards:/data/riotbuild/riotboard' \
    -v '/tmp/RIOT/makefiles:/data/riotbuild/riotmake' \
    -v '/tmp/hello-world:/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/jialamos/.gitcache:/data/riotbuild/gitcache -e GIT_CACHE_DIR=/data/riotbuild/gitcache \
     \
    -w '/data/riotbuild/riotproject//tmp/hello-world/' \
    'riot/riotbuild:latest' make all 
[sudo] password for jialamos: 
make: *** No rule to make target 'all'.  Stop.
make: *** [/tmp/RIOT/makefiles/docker.inc.mk:101: ..in-docker-container] Error 2
make: Leaving directory '/tmp/hello-world'

with PR:

make: Entering directory '/tmp/hello-world'
Launching build container using image "riot/riotbuild:latest".
sudo docker run --rm -t -u "$(id -u)" \
    -v '/tmp/RIOT:/data/riotbuild/riotbase' \
    -v '/tmp/RIOT/cpu:/data/riotbuild/riotcpu' \
    -v '/tmp/RIOT/boards:/data/riotbuild/riotboard' \
    -v '/tmp/RIOT/makefiles:/data/riotbuild/riotmake' \
    -v '/tmp/hello-world:/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/jialamos/.gitcache:/data/riotbuild/gitcache -e GIT_CACHE_DIR=/data/riotbuild/gitcache \
     \
    -w '/data/riotbuild/riotproject/' \
    'riot/riotbuild:latest' make all 
Building application "hello-world" for "native" with MCU "native".

"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/sys/auto_init
"make" -C /data/riotbuild/riotboard/native
"make" -C /data/riotbuild/riotboard/native/drivers
"make" -C /data/riotbuild/riotcpu/native
"make" -C /data/riotbuild/riotcpu/native/periph
"make" -C /data/riotbuild/riotcpu/native/vfs
   text    data     bss     dec     hex filename
  20625     372   47684   68681   10c49 /data/riotbuild/riotproject/bin/native/hello-world.elf

I confirm this fixes the issue.

@jia200x jia200x added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Oct 30, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Oct 30, 2018

I will add the backport label. This is required for testing the Release from the archives.

@jia200x jia200x added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 30, 2018
@cladmi
Makefile.include: Fix BUILDRELPATH when RIOTPROJECT is CURDIR.
a5b2af1 
This fixes building an example that is outside of RIOT with docker and
also fixes building from a release archive.
@cladmi cladmi force-pushed the pr/make/docker/fix_riotproject_is_appdir branch from 809fd5f to a5b2af1 Compare October 30, 2018 16:16
@cladmi cladmi changed the title Makefile.include: Fix BUILDRELPATH when RIOTPROJECT is APPDIR. Makefile.include: Fix BUILDRELPATH when RIOTPROJECT is CURDIR. Oct 30, 2018
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Oct 30, 2018

I inlined changed the commit title, as the change uses CURDIR variable and PR title too.

@jia200x jia200x added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines labels Oct 30, 2018
jia200x
jia200x approved these changes Oct 30, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

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

ACK. Thanks @cladmi

@jia200x jia200x added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 30, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Oct 30, 2018

I set the CI label.

@cladmi cladmi mentioned this pull request Oct 30, 2018
Closed
6 tasks
@cladmi cladmi requested a review from jcarrano November 1, 2018 08:46
@cladmi cladmi merged commit 2fc5ad6 into RIOT-OS:master Nov 1, 2018
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 1, 2018

CI is green, merge.

@jia200x jia200x modified the milestone: Release 2018.10 Nov 1, 2018
@jia200x jia200x mentioned this pull request Nov 1, 2018
Merged
@cladmi cladmi deleted the pr/make/docker/fix_riotproject_is_appdir branch November 2, 2018 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jia200x jia200x jia200x approved these changes

@smlng smlng Awaiting requested review from smlng

@jnohlgard jnohlgard Awaiting requested review from jnohlgard

@jcarrano jcarrano Awaiting requested review from jcarrano

Assignees

No one assigned

Labels

Area: build system Area: Build system Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2018.10

Development

Successfully merging this pull request may close these issues.

2 participants

@cladmi @jia200x