Skip to content

tests/riotboot: use FLASHFILE for the generated file#11089

Merged
kYc0o merged 5 commits intoRIOT-OS:masterfrom
cladmi:pr/riotboot/use_flashfile
Mar 5, 2019
Merged

tests/riotboot: use FLASHFILE for the generated file#11089
kYc0o merged 5 commits intoRIOT-OS:masterfrom
cladmi:pr/riotboot/use_flashfile

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Mar 1, 2019 •
edited
Loading

Contribution description

FLASHFILE is now a generated file when doing make all.
This prepares also for when flashers will use FLASHFILE as a file to
be flashed.

It currently still needs the hack below for openocd and edbg.

This also fixes the issue when building 'riotboot' in docker that was
being built with the host toolchain.

Description

This pull request is big and needs to modify many files to work. I could split if requested.

Testing procedure

Murdock change:

Murdock keeps running tests on samr21-xpro, and also runs the tests/riotboot test on samr21-xpro.

Riotboot global change

Review the code difference, the main thing is that now we can do FLASHFILE = $(RIOTBOOT_COMBINED_BIN) before parsing $(RIOTBASE)/Makefile.include

When compiling it should compile without ever saying Circular dependency.

My testing procedure is the following to ensure the firmware is indeed flashed and not relying on the previous version:

git clean -xdf tests/riotboot; BOARD=samr21-xpro make -C examples/hello-world/ flash; make -C tests/riotboot flash test

The test should succeed both with samr21-xpro and iotlab-m3.

Building riotboot in docker

This change also now has the consequence that when doing BUILD_IN_DOCKER=1 make all riotboot is correctly build in docker. If you have arm-gcc outside of your normal path you can try compiling with:

git clean -xdf tests/riotboot; DOCKER="sudo docker" BUILD_IN_DOCKER=1 PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin make -C tests/riotboot flash test

Issues/PRs references

This is part of #8838
This replaces part of #11083

Depends on #11097

@cladmi cladmi added Area: tests Area: tests and testing framework 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 Area: CI Area: Continuous Integration of RIOT components CI: run tests If set, CI server will run tests on hardware for the labeled PR Area: OTA Area: Over-the-air updates Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … labels Mar 1, 2019
@cladmi cladmi force-pushed the pr/riotboot/use_flashfile branch 2 times, most recently from 323735f to 1e33cb9 Compare March 1, 2019 16:59
@cladmi cladmi requested review from danpetry, jcarrano and kYc0o March 1, 2019 17:05
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Mar 1, 2019

riotboot test is still working https://ci.riot-os.org/RIOT-OS/RIOT/11089/1e33cb9dd6f18630b22786351d4c5e7ad8960933/output/run_test/tests/riotboot/samr21-xpro:gnu.txt
and the other one too. Cool :)

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Mar 1, 2019

I can put the murdock change in a separate PR if wanted. It could also help preparing other pull requests that depend on `FLASHFILE.

@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Mar 4, 2019

I agree with the changes, though I'd like to test it. I'll have time only this afternoon.

This was referenced Mar 4, 2019
Merged
Merged
@cladmi cladmi added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 4, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Mar 4, 2019

I split the murdock change as it is a prerequisite.

cladmi added 4 commits March 5, 2019 11:22
@cladmi
tests/riotboot: use 'RIOTBOOT_COMBINED_BIN' variable
60f71a1 
Use the new variable instead of the composed name.
@cladmi
Makefile.include: include 'riotboot.mk' before using FLASHFILE
ecfdece 
Include the 'riotboot.mk' file before using FLASHFILE/ELFFILE/HEXFILE
variables. This will allow setting variables to values from riotboot.mk
like `FLASHFILE = $(RIOTBOOT_COMBINED_BIN)` before it is evaluated in
Makefile.include.

It should be included after defining 'BINFILE' for 'riotboot.bin'
handling.
@cladmi
makefiles/riotboot.mk: slots only need to depend on BASELIBS
240e4a3 
Using 'link' was working too but will introduce a circular dependency
when FLASHFILE is one of the slot files.

This trims down to the minimal required dependency to work. It is now
the same as `ELFFILE` dependencies.
@cladmi
tests/riotboot: use FLASHFILE for the generated file
13e852c 
FLASHFILE is now a generated file when doing `make all`.
This prepares also for when flashers will use `FLASHFILE` as a file to
be flashed.

It currently still needs the hack below for openocd and edbg.

This also fixes the issue when building 'riotboot' in docker that was
being built with the host toolchain.
@cladmi cladmi force-pushed the pr/riotboot/use_flashfile branch from 1e33cb9 to 13e852c Compare March 5, 2019 10:23
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Mar 5, 2019

Rebased now that #11097 got merged.

@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 5, 2019
@cladmi
riotboot: prepare for when FLASHFILE is used
7a88bb4 
This currently does nothing but setting FLASHFILE when flashing.
This will allow passing the variable when flasher will use the FLASHFILE
variable.
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Mar 5, 2019 •
edited
Loading

I added a commit in this one that will handle flashers using FLASHFILE. I thought about putting it in another PR but it makes more sense to have it already here I think.

It can be tested by replacing the flasher with echo $(FLASHFILE) and seeing the file being printed:

RIOT_CI_BUILD=1 BOARD=samr21-xpro make --no-print-directory -C tests/riotboot riotboot/flash-slot0 riotboot/flash-slot1 riotboot/flash-extended-slot0 riotboot/flash-combined-slot0 FLASHER=echo FFLAGS='FLASHFILE: $(FLASHFILE)'
...
compiling /home/harter/work/git/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr...
make: Nothing to be done for 'all'.
creating /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0.riot.bin...
echo FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0.riot.bin
FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0.riot.bin
creating /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot1.riot.bin...
echo FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot1.riot.bin
FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot1.riot.bin
"make" -C /home/harter/work/git/RIOT/boards/samr21-xpro
"make" -C /home/harter/work/git/RIOT/core
"make" -C /home/harter/work/git/RIOT/cpu/samd21
"make" -C /home/harter/work/git/RIOT/cpu/cortexm_common
"make" -C /home/harter/work/git/RIOT/cpu/cortexm_common/periph
"make" -C /home/harter/work/git/RIOT/cpu/sam0_common
"make" -C /home/harter/work/git/RIOT/cpu/sam0_common/periph
"make" -C /home/harter/work/git/RIOT/cpu/samd21/periph
"make" -C /home/harter/work/git/RIOT/drivers
"make" -C /home/harter/work/git/RIOT/drivers/periph_common
"make" -C /home/harter/work/git/RIOT/sys
"make" -C /home/harter/work/git/RIOT/sys/checksum
"make" -C /home/harter/work/git/RIOT/sys/isrpipe
"make" -C /home/harter/work/git/RIOT/sys/newlib_syscalls_default
"make" -C /home/harter/work/git/RIOT/sys/pm_layered
"make" -C /home/harter/work/git/RIOT/sys/riotboot
"make" -C /home/harter/work/git/RIOT/sys/stdio_uart
"make" -C /home/harter/work/git/RIOT/sys/tsrb
echo FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0-extended.bin
FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0-extended.bin
echo FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0-combined.bin
FLASHFILE: /home/harter/work/git/RIOT/tests/riotboot/bin/samr21-xpro/tests_riotboot-slot0-combined.bin

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Mar 5, 2019

tested on OS X, works with the below command:
BUILD_IN_DOCKER=1 BOARD=samr21-xpro make -C tests/riotboot flash test
(after cleaning with git clean -xdf dist/tools/riotboot_gen_hdr/ to get rid of OS X residue build parts... typical trouble ;)

kYc0o
kYc0o approved these changes Mar 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@kYc0o kYc0o left a comment

Choose a reason for hiding this comment

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

ACK

@kYc0o kYc0o merged commit 52d2851 into RIOT-OS:master Mar 5, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Mar 5, 2019

Thank you for the testing and the review.

@cladmi cladmi deleted the pr/riotboot/use_flashfile branch March 5, 2019 16:53
@danpetry danpetry added this to the Release 2019.04 milestone Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kYc0o kYc0o kYc0o approved these changes

@danpetry danpetry Awaiting requested review from danpetry

@jcarrano jcarrano Awaiting requested review from jcarrano

Assignees

No one assigned

Labels

Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components Area: OTA Area: Over-the-air updates Area: tests Area: tests and testing framework 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 CI: run tests If set, CI server will run tests on hardware for the labeled PR

Projects

None yet

Milestone

Release 2019.04

Development

Successfully merging this pull request may close these issues.

4 participants

@cladmi @kYc0o @emmanuelsearch @danpetry