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

pkg/libfixmath: replace __FILE__ by RIOT_FILE_NOPATH #9643

Merged
merged 1 commit into from
Jul 31, 2018
Merged

pkg/libfixmath: replace __FILE__ by RIOT_FILE_NOPATH #9643

merged 1 commit into from
Jul 31, 2018

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Jul 26, 2018 •
edited
Loading

Contribution description

Update the TEST macro to use our RIOT_FILE_RELATIVE RIOT_FILE_NOPATH macro.

The binary file size changes when the RIOT directory is moved.
This caused the libfixmath_unittests to fail on my computer.

This saves 1872 6632 bytes of rom for samr21-xpro on my computer.

Issues/PRs references

Same reasons as why the macro was introduced #2969
Found during release testing

@cladmi cladmi added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 26, 2018
@cladmi cladmi mentioned this pull request Jul 27, 2018
Merged
@cladmi cladmi requested a review from ZetaR60 July 27, 2018 14:10
@cladmi cladmi added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 27, 2018
@cladmi cladmi changed the title pkg/libfixmath: replace __FILE__ by RIOT_FILE_RELATIVE pkg/libfixmath: replace __FILE__ by ~RIOT_FILE_RELATIVE~ RIOT_FILE_NOPATH Jul 27, 2018
@cladmi cladmi changed the title pkg/libfixmath: replace __FILE__ by ~RIOT_FILE_RELATIVE~ RIOT_FILE_NOPATH pkg/libfixmath: replace __FILE__ by RIOT_FILE_NOPATH Jul 27, 2018
@cladmi
Copy link
Contributor Author

cladmi commented Jul 27, 2018

I updated to use RIOT_FILE_NOPATH instead of RIOT_FILE_RELATIVE as there is this issue with docker: #9645 (comment) and that the full file path is not that much required. The more boards it compiles to, the better.

ZetaR60
ZetaR60 suggested changes Jul 27, 2018
View reviewed changes
Copy link
Contributor

@ZetaR60 ZetaR60 left a comment

Choose a reason for hiding this comment

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

This fixes the insufficient memory problems that arise with the Docker update to Bionic. However, it does not seem to fix the boards that are already listed in BOARD_INSUFFICIENT_MEMORY.

Test with the current Docker image:

user@user ~/Desktop/RIOT_libfixmath_debug/tests/libfixmath_unittests $ make BUILD_IN_DOCKER=1 buildtest
Launching build container using image "riot/riotbuild:latest".
docker run --rm -t -u "$(id -u)" \
    -v '/home/user/Desktop/RIOT_libfixmath_debug:/data/riotbuild/riotbase' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug/cpu:/data/riotbuild/riotcpu' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug/boards:/data/riotbuild/riotboard' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug/makefiles:/data/riotbuild/riotmake' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug:/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' \
     \
    -w '/data/riotbuild/riotproject/tests/libfixmath_unittests/' \
    'riot/riotbuild:latest' make buildtest 
Building for acd52832 ... success.
Building for airfy-beacon ... success.
Building for arduino-due ... success.
Building for arduino-mkr1000 ... success.
Building for arduino-mkrfox1200 ... success.
Building for arduino-mkrzero ... success.
Building for arduino-zero ... success.
Building for avsextrem ... success.
Building for b-l072z-lrwan1 ... success.
Building for b-l475e-iot01a ... success.
Building for bluepill ... success.
Building for calliope-mini ... success.
Building for cc2538dk ... success.
Building for cc2650-launchpad ... success.
Building for cc2650stk ... success.
Building for ek-lm4f120xl ... success.
Building for f4vi1 ... success.
Building for feather-m0 ... success.
Building for fox ... success.
Building for frdm-k22f ... success.
Building for frdm-k64f ... success.
Building for frdm-kw41z ... success.
Building for hifive1 ... success.
Building for ikea-tradfri ... success.
Building for iotlab-a8-m3 ... success.
Building for iotlab-m3 ... success.
Building for limifrog-v1 ... success.
Building for maple-mini ... success.
Building for mbed_lpc1768 ... success.
Building for microbit ... success.
Building for mips-malta ... success.
Building for msba2 ... success.
Building for msbiot ... success.
Building for mulle ... success.
Building for native ... success.
Building for nrf51dongle ... success.
Building for nrf52840dk ... success.
Building for nrf52dk ... success.
Building for nrf6310 ... success.
Building for nucleo-f030r8 ... success.
Building for nucleo-f031k6 ... failed!
Building for nucleo-f042k6 ... failed!
Building for nucleo-f070rb ... success.
Building for nucleo-f072rb ... success.
Building for nucleo-f091rc ... success.
Building for nucleo-f103rb ... success.
Building for nucleo-f207zg ... success.
Building for nucleo-f302r8 ... success.
Building for nucleo-f303k8 ... success.
Building for nucleo-f303re ... success.
Building for nucleo-f303ze ... success.
Building for nucleo-f334r8 ... success.
Building for nucleo-f401re ... success.
Building for nucleo-f410rb ... success.
Building for nucleo-f411re ... success.
Building for nucleo-f412zg ... success.
Building for nucleo-f413zh ... success.
Building for nucleo-f429zi ... success.
Building for nucleo-f446re ... success.
Building for nucleo-f446ze ... success.
Building for nucleo-f722ze ... success.
Building for nucleo-f746zg ... success.
Building for nucleo-f767zi ... success.
Building for nucleo-l031k6 ... failed!
Building for nucleo-l053r8 ... success.
Building for nucleo-l073rz ... success.
Building for nucleo-l152re ... success.
Building for nucleo-l432kc ... success.
Building for nucleo-l433rc ... success.
Building for nucleo-l452re ... success.
Building for nucleo-l476rg ... success.
Building for nucleo-l496zg ... success.
Building for nz32-sc151 ... success.
Building for opencm904 ... success.
Building for openmote-cc2538 ... success.
Building for pba-d-01-kw2x ... success.
Building for pic32-clicker ... success.
Building for pic32-wifire ... success.
Building for remote-pa ... success.
Building for remote-reva ... success.
Building for remote-revb ... success.
Building for ruuvitag ... success.
Building for samd21-xpro ... success.
Building for saml21-xpro ... success.
Building for samr21-xpro ... success.
Building for seeeduino_arch-pro ... success.
Building for slstk3401a ... success.
Building for slstk3402a ... success.
Building for sltb001a ... success.
Building for slwstk6000b ... success.
Building for slwstk6220a ... success.
Building for sodaq-autonomo ... success.
Building for sodaq-explorer ... success.
Building for spark-core ... success.
Building for stk3600 ... success.
Building for stk3700 ... success.
Building for stm32f0discovery ... success.
Building for stm32f3discovery ... success.
Building for stm32f429i-disc1 ... success.
Building for stm32f4discovery ... success.
Building for stm32f769i-disco ... success.
Building for stm32l476g-disco ... success.
Building for stm32mindev ... success.
Building for teensy31 ... success.
Building for thingy52 ... success.
Building for ublox-c030-u201 ... success.
Building for udoo ... success.
Building for yunjia-nrf51822 ... success.
/data/riotbuild/riotmake/buildtests.inc.mk:7: recipe for target 'buildtest' failed
make: *** [buildtest] Error 1
/home/user/Desktop/RIOT_libfixmath_debug/makefiles/docker.inc.mk:90: recipe for target '..in-docker-container' failed
make: *** [..in-docker-container] Error 2

Test with the new Bionic image:

user@user ~/Desktop/RIOT_libfixmath_debug/tests/libfixmath_unittests $ make BUILD_IN_DOCKER=1 buildtest
Launching build container using image "riotdocker_bionic:latest".
docker run --rm -t -u "$(id -u)" \
    -v '/home/user/Desktop/RIOT_libfixmath_debug:/data/riotbuild/riotbase' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug/cpu:/data/riotbuild/riotcpu' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug/boards:/data/riotbuild/riotboard' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug/makefiles:/data/riotbuild/riotmake' \
    -v '/home/user/Desktop/RIOT_libfixmath_debug:/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' \
     \
    -w '/data/riotbuild/riotproject/tests/libfixmath_unittests/' \
    'riotdocker_bionic:latest' make buildtest 
Building for acd52832 ... success.
Building for airfy-beacon ... success.
Building for arduino-due ... success.
Building for arduino-mkr1000 ... success.
Building for arduino-mkrfox1200 ... success.
Building for arduino-mkrzero ... success.
Building for arduino-zero ... success.
Building for avsextrem ... success.
Building for b-l072z-lrwan1 ... success.
Building for b-l475e-iot01a ... success.
Building for bluepill ... success.
Building for calliope-mini ... success.
Building for cc2538dk ... success.
Building for cc2650-launchpad ... success.
Building for cc2650stk ... success.
Building for ek-lm4f120xl ... success.
Building for f4vi1 ... success.
Building for feather-m0 ... success.
Building for fox ... success.
Building for frdm-k22f ... success.
Building for frdm-k64f ... success.
Building for frdm-kw41z ... success.
Building for hifive1 ... success.
Building for ikea-tradfri ... success.
Building for iotlab-a8-m3 ... success.
Building for iotlab-m3 ... success.
Building for limifrog-v1 ... success.
Building for maple-mini ... success.
Building for mbed_lpc1768 ... success.
Building for microbit ... success.
Building for mips-malta ... success.
Building for msba2 ... success.
Building for msbiot ... success.
Building for mulle ... success.
Building for native ... success.
Building for nrf51dongle ... success.
Building for nrf52840dk ... success.
Building for nrf52dk ... success.
Building for nrf6310 ... success.
Building for nucleo-f030r8 ... success.
Building for nucleo-f031k6 ... failed!
Building for nucleo-f042k6 ... failed!
Building for nucleo-f070rb ... success.
Building for nucleo-f072rb ... success.
Building for nucleo-f091rc ... success.
Building for nucleo-f103rb ... success.
Building for nucleo-f207zg ... success.
Building for nucleo-f302r8 ... success.
Building for nucleo-f303k8 ... success.
Building for nucleo-f303re ... success.
Building for nucleo-f303ze ... success.
Building for nucleo-f334r8 ... success.
Building for nucleo-f401re ... success.
Building for nucleo-f410rb ... success.
Building for nucleo-f411re ... success.
Building for nucleo-f412zg ... success.
Building for nucleo-f413zh ... success.
Building for nucleo-f429zi ... success.
Building for nucleo-f446re ... success.
Building for nucleo-f446ze ... success.
Building for nucleo-f722ze ... success.
Building for nucleo-f746zg ... success.
Building for nucleo-f767zi ... success.
Building for nucleo-l031k6 ... failed!
Building for nucleo-l053r8 ... success.
Building for nucleo-l073rz ... success.
Building for nucleo-l152re ... success.
Building for nucleo-l432kc ... success.
Building for nucleo-l433rc ... success.
Building for nucleo-l452re ... success.
Building for nucleo-l476rg ... success.
Building for nucleo-l496zg ... success.
Building for nz32-sc151 ... success.
Building for opencm904 ... success.
Building for openmote-cc2538 ... success.
Building for pba-d-01-kw2x ... success.
Building for pic32-clicker ... success.
Building for pic32-wifire ... success.
Building for remote-pa ... success.
Building for remote-reva ... success.
Building for remote-revb ... success.
Building for ruuvitag ... success.
Building for samd21-xpro ... success.
Building for saml21-xpro ... success.
Building for samr21-xpro ... success.
Building for seeeduino_arch-pro ... success.
Building for slstk3401a ... success.
Building for slstk3402a ... success.
Building for sltb001a ... success.
Building for slwstk6000b ... success.
Building for slwstk6220a ... success.
Building for sodaq-autonomo ... success.
Building for sodaq-explorer ... success.
Building for spark-core ... success.
Building for stk3600 ... success.
Building for stk3700 ... success.
Building for stm32f0discovery ... success.
Building for stm32f3discovery ... success.
Building for stm32f429i-disc1 ... success.
Building for stm32f4discovery ... success.
Building for stm32f769i-disco ... success.
Building for stm32l476g-disco ... success.
Building for stm32mindev ... success.
Building for teensy31 ... success.
Building for thingy52 ... success.
Building for ublox-c030-u201 ... success.
Building for udoo ... success.
Building for yunjia-nrf51822 ... success.
/data/riotbuild/riotmake/buildtests.inc.mk:7: recipe for target 'buildtest' failed
make: *** [buildtest] Error 1
/home/user/Desktop/RIOT_libfixmath_debug/makefiles/docker.inc.mk:90: recipe for target '..in-docker-container' failed
make: *** [..in-docker-container] Error 2

tests/libfixmath_unittests/Makefile Outdated
@@ -7,8 +7,6 @@ BOARD_BLACKLIST := arduino-mega2560 waspmote-pro arduino-uno arduino-duemilanove
# The MSP boards don't feature round(), exp(), and log(), which are used in the unittests
BOARD_BLACKLIST += chronos msb-430 msb-430h telosb wsn430-v1_3b wsn430-v1_4 z1

BOARD_INSUFFICIENT_MEMORY := nucleo-f031k6 nucleo-f042k6 nucleo-l031k6
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is safe to remove this, according to my test above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review. It is cool that it fixes the upgrade.

I removed this line to get the report from murdock, I will re-re-re-revert when it fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It indeed failed also on murdock I reverted the BOARD_INSUFFICIENT_MEMORY commit.

@miri64 miri64 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 Jul 30, 2018
@cladmi cladmi force-pushed the pr/libfixmath/debug branch from c740f7e to bb0c94e Compare July 30, 2018 12:37
@cladmi
Copy link
Contributor Author

cladmi commented Jul 30, 2018

Please tell me if I can squash to provide the final commits.

@cladmi cladmi mentioned this pull request Jul 30, 2018
Closed
46 tasks
@ZetaR60
Copy link
Contributor

ZetaR60 commented Jul 30, 2018

You can squash.

I think we need another opinion on the changes to makefiles/docker.inc.mk as I am not familiar enough with the makefiles to properly review them.

@cladmi
Copy link
Contributor Author

cladmi commented Jul 31, 2018

My bad for the docker thing, I rebased it on #9643 to be able to test and forgot to remove it…

@cladmi
pkg/libfixmath: Replace __FILE__ by RIOT_FILE_NOPATH
7913dd9 
The binary file size changes when the RIOT directory is moved.
This caused the `libfixmath_unittests` to fail on my computer.

I used RIOT_FILE_NOPATH instead of RIOT_FILE_RELATIVE as 'TEST' is used a lot
and it would allow more boards to be tested, full path is not that important.
@cladmi cladmi force-pushed the pr/libfixmath/debug branch from bb0c94e to 7913dd9 Compare July 31, 2018 09:56
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 31, 2018
@ZetaR60
Copy link
Contributor

ZetaR60 commented Jul 31, 2018

ACK. Everything looks good.

@ZetaR60 ZetaR60 merged commit 3ab2029 into RIOT-OS:master Jul 31, 2018
@cladmi cladmi added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jul 31, 2018
@cladmi cladmi mentioned this pull request Jul 31, 2018
Merged
@cladmi
Copy link
Contributor Author

cladmi commented Jul 31, 2018

Backport provided in #9657

@cladmi cladmi deleted the pr/libfixmath/debug branch July 31, 2018 18:23
@ZetaR60 ZetaR60 mentioned this pull request Aug 2, 2018
Merged
@cladmi cladmi added this to the Release 2018.07 milestone Aug 11, 2018
@cladmi cladmi mentioned this pull request Oct 30, 2018
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZetaR60 ZetaR60 ZetaR60 approved these changes

Assignees
No one assigned
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch
Projects
None yet
Milestone
Release 2018.07
Development

Successfully merging this pull request may close these issues.
3 participants
@cladmi @ZetaR60 @miri64