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

Dockerfile that just works #1543

Closed
wants to merge 1 commit into from
Closed

Dockerfile that just works #1543

wants to merge 1 commit into from

Conversation

Fxlr8
Copy link

@Fxlr8 Fxlr8 commented Oct 26, 2018

Hi, I updated your Dockerfile to make it work. It is far from perfect right now, but it works.
Here are some ways to improve it:

  1. Transfer all logic from provision.sh so all the dependencies are handled by docker. This will let docker's cache work and speed up the container build
  2. Mount source code and result folders to the container, so you don't need to rebuild it every time you change the code. Also you will not need to copy the results from the container every time.
  3. Pass the board name as an env variable to the container.

Hi, I updated your Dockerfile to make it work. It is far from perfect right now, but it works.
Here are some ways to improve it:
1. Transfer all logic from provision.sh so all the dependencies are handled by docker. This will let docker's cache work and speed up the container build
2. Mount source code and result folders to the container, so you don't need to rebuild it every time you change the code. Also you will not need to copy the results from the container every time.
3. Pass the board name as an env variable to the container.
@gfwilliams
Copy link
Member

Thanks - but does FROM python:3 really pull in everything you need? How does it know to pull in a version of Linux for the build?

Nice ideas for improving it - although I'm totally against pulling in provision.sh just for a bit of speed. Very few people seem to have bothered maintaining the Docker file thus far - so if we did move the code there's no way it'd stay working past a few weeks :(

Since we used ENV BOARD, can the rest of the commands be made to use $BOARD?

@Fxlr8
Copy link
Author

Fxlr8 commented Oct 26, 2018

When I first tried to run your Dockerfile the first error I got was python not found So I switched FROM from vanilla Ubuntu to python:3. I don't know which version of Linux it is using, but if you need a specific one you can choose from a bunch of tags here https://hub.docker.com/_/python/

Because provision.sh script is making installations depending on board type, we have to rebuild the whole container image every time we need a build for another board. My idea was to minimize docker builds. Can we install all dependencies for every board type without conflicts? If so, then we can build one container and re-use it for every board by changing ENV BOARD. This will save you a lot of time. ENV BOARD will be available for every script that is running inside a container.

@gfwilliams
Copy link
Member

Ahh, interesting - it looks from Shared Tags like python:3 may actually use different OS images on some platforms? I guess we'd have to use 3-stretch.

Very odd about Ubuntu images though - pretty sure Ubuntu always comes with Python by default (it did when the Docker file was made by the look of it), but maybe they've changed it now and stripped it right back for Docker.

You can't get provision.sh to install everything right now, but yeah, it'd be nice to allow provision.sh ALL - and not too painful to do inside provision.sh. Nothing should conflict - however it would make it quite a bit slower to set up the image. We could just run ./provision several times for the ESP8266/32 and ARM - but it's not great for maintainability :)

Ideally people would just be able to point Docker at github without ever having to clone the Repo to change the Docker file - if it enabled that then it'd be well worth it.

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.

None yet

2 participants