-
Notifications
You must be signed in to change notification settings - Fork 2.1k
makefiles/docker: add support for tinybuild and smallbuild #21844
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
base: master
Are you sure you want to change the base?
Conversation
|
Okay, since Makefiles aren't supposed to check the It is not incredibly beautiful the way it is, but it would be easy to move it to a dedicated shell script now. The reason I stepped away from the Perhaps one could do some magic to conditionally set arguments like I'd like to get some input if this is the right direction or if we should pursue the |
|
We could also just drop that check. To my knowledge the reason for that check has been that once upon a timer there was no ifneq (,$(filter periph_i2c,$(FEATURES_REQUIRED))
$(error "Sorry, that feature is not available here")
endifThe check was to ensure that old PRs don't get merged to re-introduce that legacy pattern. IMO that check has succeeded and could now be dropped. |
AnnsAnns
left a comment
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.
RE: I looked at this a week ago, it looks fine to me but I don't know enough about the RIOT docker to confidently approve this.
|
This is pretty much on hold until the riotdocker PR is merged anyway, but thanks for testing :) |
Contribution description
This PR is a first draft to add support for tinybuild and smallbuild in the RIOT build system and automagically selects the right image to use depending on the featureset.
Doing this as a bash-script inside of the make recipe isn't super nice, but I couldn't come up with anything better tbh.
The better solution would be to craft a Dockerfile that distinguishes between the platforms and possibly also gets
$(CPU_ARCH)and$(USEMODULE)as parameters. Then it would be easier to keep the platforms in sync, because it's in theriotdockerrepository and not here. Something likebuild-dispatcher.There is Multi-Stage building in Docker, but I have no experience with that and for the
build-dispatcherimage, we'd have to pull Alpine Linux again I guess? So there would be a size penalty.Testing procedure
To test whether the switch-case distinguishes correctly between the variants, I just replaced the
dockercommand withecho:Select the legacy
riotbuild:Issues/PRs references
Depends on RIOT-OS/riotdocker#259.