-
Notifications
You must be signed in to change notification settings - Fork 2k
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
makefiles: allow to override suit manifest payloads #16771
makefiles: allow to override suit manifest payloads #16771
Conversation
0caaeab
to
0297840
Compare
I think @benpicco uses suit quite a lot as well, might have an input on this or might be breaking something for him with this. |
Murdock does not like it |
Canceled build, I'll take a look, probably some make target ordering issue |
Murdock should be happy now! |
d74c718
to
d37eb3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still looks like a welcome cleanup.
Was there anything missing here?
|
||
# | ||
export SLOT0_OFFSET SLOT0_LEN SLOT1_OFFSET SLOT1_LEN | ||
|
||
# Mandatory APP_VER, set to epoch by default | ||
EPOCH := $(shell date +%s) | ||
EPOCH = $(call memoized,EPOCH,$(shell date +%s)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reasoning behind that change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoiding a shell call if not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can split
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine, I was just not familiar with call memoized
d37eb3e
to
d820296
Compare
I'm going to rebase because this currently needs the fixes in #17950 to properly test. |
1 similar comment
I'm going to rebase because this currently needs the fixes in #17950 to properly test. |
With #17950 the test passes:
|
1 similar comment
With #17950 the test passes:
|
I also updated the path to show it working on native:
|
1 similar comment
I also updated the path to show it working on native:
|
@benpicco IMO nothing is missing, would you mind if I squashed? |
1 similar comment
@benpicco IMO nothing is missing, would you mind if I squashed? |
d820296
to
f01df2c
Compare
Sure, please squash! |
This adds: * SUIT_MANIFEST_BASENAME: allow for non slotfiles payloads to have different names that slotfiles payloads. * SUIT_MANIFEST_PAYLOADS: firmware payloads to be published with the manifest. * SUIT_MANIFEST_SLOTFILES: firmware payloads referenced by the manifest in the form 'filename:[offset]:[comp_name]' as expected by gen_manifest.py. With this the same recipes suit/publish suit/notify can be used with non slotfiles payloads.
- rename riotboot files so that they are of the form: slot<n>.<version>.bin - move all generated files under $(BINDIR)/riotboot_files (this can be overwritten.
- move all generated manifests under $(BINDIR)/suit_files (this can be overwritten. - rename signed manifests so that they are of the form: <somename>.<version>.bin, where <somename> is by default riot.suit. This avoids cluterring BINDIR while as well having a naming scheme that allows custom names for manifests addresssing different types of payloads.
APP_VER must also be defined for suit.inc.mk in case non-fw payloads are used (e.g. no riotboot) Use memoized so the shell call happens only if needed
f01df2c
to
76ee54e
Compare
Thanks for the review! |
Contribution description
This PR re-work how
suit
manifests are named and stored as well as the naming of suit manifests payloads (e.g. riotboot). FIles that need to hold version (or sequence number) are of the form:<somename>.<version>.bin
manifests follow the same formatwith but with a
<somename> == riot.suit
by default. Since both suit files and riotboot files can easily clutter the bin directory they are moth moved to their own directories withinBINDIR
, but having distinct directories for both.This adds:
manifest.
in the form 'filename:[offset]:[comp_name]' as expected by
gen_manifest.py.
With this the same recipes suit/publish suit/notify can be used with
non-slotfiles payloads.
PS: bikeshedding the names is very welcome.
Testing procedure
examples/suit_update
still passes.suit/publish suit/notify
can work out of the box with for example a patch like this:examples/suit_update
still passestests/riotboot
still passestests/riotboot_flashwrite
still passses.