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.

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

bootloaders|tests/riotboot: broken with BUILD_IN_DOCKER and wrong flashfile #12003

Closed
cladmi opened this issue Aug 13, 2019 · 9 comments · Fixed by #12302
Closed

bootloaders|tests/riotboot: broken with BUILD_IN_DOCKER and wrong flashfile #12003

cladmi opened this issue Aug 13, 2019 · 9 comments · Fixed by #12302
Assignees
Labels
Area: build system Area: Build system Area: OTA Area: Over-the-air updates Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@cladmi
Copy link
Contributor

cladmi commented Aug 13, 2019

Description

The application in bootloaders/riotboot has the following issues, from a first look I think I also found the culprits:

The application in tests/riotboot also has the same BUILD_IN_DOCKER issues.

The tests/riotboot test does not work with BUILD_IN_DOCKER.

Steps to reproduce the issue

FLASHFILE is extended file
BOARD=iotlab-m3 make --no-print-directory -C bootloaders/riotboot/ info-debug-variable-FLASHFILE
/home/harter/work/git/RIOT/bootloaders/riotboot/bin/iotlab-m3/riotboot-slot0-extended.bin
tests/riotboot test does not work with `BUILD_IN_DOCKER=1`
WARNING:nrf52dk.tests/riotboot:make RIOT_CI_BUILD=1 CC_NOCOLOR=1 --no-print-directory -C ./tests/riotboot test
expecting slot number 0, app_ver 0
curslotnr
/srv/ilab-builds/workspace/jobs/experimental-pull-request-tests/dist/tools/pyterm/pyterm -p "/dev/riot/tty-nrf52dk" -b "115200" --noprefix --no-repeat-command-on-empty-line
Twisted not available, please install it if you want to use pyterm's JSON capabilities
Connect to serial port /dev/riot/tty-nrf52dk
Welcome to pyterm!
Type '/exit' to exit.
main(): This is RIOT! (Version: buildtest)
Hello riotboot!
You are running RIOT on a(n) nrf52dk board.
This board features a(n) nrf52 MCU.
riotboot_test: running from slot 0
Image magic_number: 0x544f4952
Image Version: 0x00000000
Image start address: 0x00002400
Header chksum: 0xa57ac1a1

>  curslotnr
Current slot=0
> curslothdr
 curslothdr
Image magic_number: 0x544f4952
Image Version: 0x00000000
Image start address: 0x00002400
Header chksum: 0xa57ac1a1

> getslotaddr 0
 getslotaddr 0
Slot 0 address=0x00002400
> dumpaddrs
 dumpaddrs
slot 0: metadata: 0x2000 image: 0x00002400
slot 1: metadata: 0x41000 image: 0x00000000
> 
compiling /srv/ilab-builds/workspace/jobs/experimental-pull-request-tests/dist/tools/riotboot_gen_hdr/bin/genhdr...
make: Nothing to be done for 'all'.
make[1]: *** No rule to make target '/srv/ilab-builds/workspace/jobs/experimental-pull-request-tests/tests/riotboot/bin/nrf52dk/tests_riotboot-slot1.bin', needed by '/srv/ilab-builds/workspace/jobs/experimental-pull-request-tests/tests/riotboot/bin/nrf52dk/tests_riotboot-slot1.hdr'.  Stop.
/srv/ilab-builds/workspace/jobs/experimental-pull-request-tests/tests/riotboot/../../Makefile.include:669: recipe for target 'test' failed
make: *** [test] Error 1
Fixed
It does not build with `docker`
BUILD_IN_DOCKER=1 DOCKER="sudo docker" BOARD=frdm-k64f make --no-print-directory -C bootloaders/riotboot
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/cpu/fcfield.o: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/cpu/vectors.o: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/newlib_syscalls_default/syscalls.o: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/application_riotboot.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/board.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/checksum.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/core.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/cortexm_common.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/cortexm_common_periph.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/cpu.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/newlib_syscalls_default.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/periph.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/riotboot.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/stdio_uart.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/sys.a: No such file or directory
arm-none-eabi-gcc: error: /home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/periph_common.a: No such file or directory
/home/harter/work/git/RIOT/makefiles/boot/riotboot.mk:35: recipe for target '/home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/riotboot-slot0.elf' failed
Building uses the local toolchain
PATH=${DEFAULT_PATH} BUILD_IN_DOCKER=1 DOCKER="sudo docker" BOARD=frdm-k64f make --no-print-directory -C bootloaders/riotboot/  
/bin/sh: 1: arm-none-eabi-gcc: not found
/home/harter/work/git/RIOT/makefiles/boot/riotboot.mk:35: recipe for target '/home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/riotboot-slot0.elf' failed
make: *** [/home/harter/work/git/RIOT/bootloaders/riotboot/bin/frdm-k64f/riotboot-slot0.elf] Error 127

Expected results

  • FLASHFILE is ${BINDIR}/riotboot.elf
  • It compiles correctly in docker without local toolchain

Versions

Found on e5fe868

Commit introducing the issues bb71e97 and aaa187e

@cladmi cladmi changed the title bootloaders/riotboot: broken with BUILD_IN_DOCKER and wrong flashfile bootloaders|tests/riotboot: broken with BUILD_IN_DOCKER and wrong flashfile Aug 13, 2019
@cladmi
Copy link
Contributor Author

cladmi commented Aug 13, 2019

Currently running buildtest with BUILD_IN_DOCKER=1 would work as #11842 is hiding the issue.

@fjmolinas
Copy link
Contributor

I opened #12303 to address the second point.

I can think of a couple solutions for the first issue:

ifeq (,$(filter riotboot_bootloader,$(FEATURES_USED)))
# Applications that use 'riotboot' feature default to actually using it,
# except for the bootloader itself which will use 'riotboot.elf'
# when beeing flashed stand-alone.
# It also makes 'flash' and 'flash-only' work without specific command.
FLASHFILE = $(RIOTBOOT_EXTENDED_BIN)
endif
  • use APPLICATION = riotboot to achieve the same behavior, this means less code addition but means checking a variable we don't check in RIOT so far. IMO this is actually a good solution but no application can be named riotboot other than the bootloader. Does this seem wrong regarding the bootloader? @cladmi
ifeq (,$(filter riotboot,$(APPLICATION)))
# Applications that use 'riotboot' feature default to actually using it,
# except for the bootloader itself which will use 'riotboot.elf'
# when beeing flashed stand-alone.
# It also makes 'flash' and 'flash-only' work without specific command.
FLASHFILE = $(RIOTBOOT_EXTENDED_BIN)
endif

Maybe @kaspar030 has thoughts on this one as well..

fjmolinas added a commit to fjmolinas/RIOT that referenced this issue Sep 26, 2019
- riotboot targets should not be needed for riotboot application
  so dont include it.
- also fixes RIOT-OS#12003 by not setting FLASHFILE = $(RIOTBOOT_EXTENDED_BIN)
  when compiling riotboot application
fjmolinas added a commit to fjmolinas/RIOT that referenced this issue Sep 26, 2019
- riotboot targets should not be needed for riotboot application
  so dont include it.
- also fixes RIOT-OS#12003 by not setting FLASHFILE = $(RIOTBOOT_EXTENDED_BIN)
  when compiling riotboot application
@cladmi
Copy link
Contributor Author

cladmi commented Sep 30, 2019

This one is not closed yet #12302 took care of one part only for the moment.

@fjmolinas
Copy link
Contributor

Yes, I though prefixing with something different than fix would not close the issue, re-opened!

@fjmolinas fjmolinas reopened this Sep 30, 2019
@cladmi
Copy link
Contributor Author

cladmi commented Sep 30, 2019

It was the top one in the pr comment that closed it, I only paid attention to the last one.

@cladmi
Copy link
Contributor Author

cladmi commented Sep 30, 2019

I updated the issue description to reflect the fixed compilation and failing test.

@cladmi
Copy link
Contributor Author

cladmi commented Oct 17, 2019

While reviewing #12446 I found out another issue with compiling the tool on the raspberry PI.

compiling /tmp/dwq.0.20579331210052065/5c38a93b2e9b62fea1be9c23e3b3971c/dist/tools/riotboot_gen_hdr/bin/genhdr...
genhdr.c: In function 'genhdr':
genhdr.c:71:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (errno != 0 || *p != '\0' || app_ver_arg > UINT32_MAX) {
                                                 ^
genhdr.c:79:52: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (errno != 0 || *p != '\0' || start_addr_arg > UINT32_MAX) {
                                                    ^
genhdr.c:87:76: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (errno != 0 || *p != '\0' || hdr_len_arg % HDR_ALIGN || hdr_len_arg > UINT32_MAX) {
                                                                            ^

When testing a change on this, the test caching must also be disabled as of course these files are ignored for it.

EDIT: a nightly output for reference https://ci.riot-os.org/RIOT-OS/RIOT/master/02ae803acc06b662b73a37f70b8b2e3580d2c8d3/output/run_test/tests/riotboot/samr21-xpro:llvm.txt

@fjmolinas
Copy link
Contributor

While reviewing #12446 I found out another issue with compiling the tool on the raspberry PI.

compiling /tmp/dwq.0.20579331210052065/5c38a93b2e9b62fea1be9c23e3b3971c/dist/tools/riotboot_gen_hdr/bin/genhdr...
genhdr.c: In function 'genhdr':
genhdr.c:71:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (errno != 0 || *p != '\0' || app_ver_arg > UINT32_MAX) {
                                                 ^
genhdr.c:79:52: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (errno != 0 || *p != '\0' || start_addr_arg > UINT32_MAX) {
                                                    ^
genhdr.c:87:76: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     if (errno != 0 || *p != '\0' || hdr_len_arg % HDR_ALIGN || hdr_len_arg > UINT32_MAX) {
                                                                            ^
When testing a change on this, the test caching must also be disabled as of course these files are ignored for it.

I'm guessing those are pre-compiled on murdock right? But you are right, this is also an issue.

@cladmi
Copy link
Contributor Author

cladmi commented Oct 17, 2019

It is an issue only on 32bit hosts, so not on our machines or murdock that are 64bit.

fjmolinas added a commit to fjmolinas/RIOT that referenced this issue Jan 10, 2020
- riotboot targets should not be needed for riotboot application
  so dont include it.
- also fixes RIOT-OS#12003 by not setting FLASHFILE = $(RIOTBOOT_EXTENDED_BIN)
  when compiling riotboot application
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Jan 10, 2020
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 Area: OTA Area: Over-the-air updates Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants