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

Use docker to build locally and in CI #26

Merged

Conversation

philipstarkey
Copy link
Member

@philipstarkey philipstarkey commented Feb 3, 2024

This adds docker configuration files so that PrawnBlaster firmware can be built within a docker container on personal devices and in GitHub actions.

Changes

  • Updates the pico-sdk to v1.5.1
  • Remove uf2 files from the repository
  • Add docker file to build a container designed to build rp2040 firmware using the pico-sdk
  • Pin pico-sdk version via the docker file
  • Update license to 3-clause BSD license (from 2-clause BSD license) and add missing license file to the repository
  • Added GitHub action script that builds artefacts (containing the uf2 files) for pull requests and releases when commits are tagged.

@philipstarkey
Copy link
Member Author

This should probably be squash merged due to the messy history (I'm happy to handle this once approved)

Copy link
Collaborator

@dihm dihm left a comment

Choose a reason for hiding this comment

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

This is pretty nice! Can't say I've ever used docker for anything before, but this is a decent step up from the typical build install process on Windows. Guess I'll need to actually learn docker now ;)

@carterturn
Copy link
Contributor

It has been a while since I have looked through Docker best practices carefully, so there may be good reasons not to follow these suggestions.
I think it would be good to put the Dockerfile outside of the build folder, as that folder ends up with lots of cmake outputs that one may want to quickly delete in order to perform a clean build.
For the most common Docker setup on Linux, the build files end up being owned by root, which is a bit inconvenient. I think a fairly straightforward fix for this is to add && chown -R --reference=/prawnblaster /prawnblaster to the end of the docker-compose buildfirmware service command. This works as expected on my system, and I think it ought to be compatible with Github actions, but the latter should be tested.

@philipstarkey
Copy link
Member Author

I think it would be good to put the Dockerfile outside of the build folder, as that folder ends up with lots of cmake outputs that one may want to quickly delete in order to perform a clean build.

Good call! I've relocated it.

For the most common Docker setup on Linux, the build files end up being owned by root, which is a bit inconvenient. I think a fairly straightforward fix for this is to add && chown -R --reference=/prawnblaster /prawnblaster to the end of the docker-compose buildfirmware service command. This works as expected on my system, and I think it ought to be compatible with Github actions, but the latter should be tested.

I'm more hesitant on this one. Docker user/permission stuff on linux/OSX systems always gets messy. I think I'll merge as-is but open an issue around it. It might be as simple as using a slightly different command to run docker, or an addition to the docker compose file, or a wrapper script that passes in the current UID/GID to docker (although I've had trouble with the one in the past due to needing to create users with those UID/GUD in the container when building which is hard).

@philipstarkey philipstarkey merged commit a2fb893 into labscript-suite:master Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants