-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
Build libcec for Docker image #5230
Conversation
@armills, thanks for your PR! By analyzing the history of the files in this pull request, we identified @auduny, @balloob and @allanglen to be potential reviewers. |
FYI, this won't work for rpi, since you need to have the development headers for broadcom installed. See https://github.com/Pulse-Eight/libcec/blob/master/docs/README.raspberrypi.md, but I'm not sure if hass runs on docker on rpi. Could you also add these changes to |
I updated the PR to modify the development Dockerfile. I do think it's a good idea to include the Broadcom development headers for the rpi, but I think someone who can test that is going to need to add it. |
The problem with rpi is that the cmake argements are different. So a different docker image should be made depending on where the image will be ran, and that is against the purpose of docker images... |
Looking quickly at their build system it looks like if the rpi headers are included it builds rpi support alongside support for other devices, so they could all be run from the same image. I could be wrong there. I agree building different images for different targets is a bad idea. |
Well you sadly will need different images for things like an RPi because it has a different architecture, so you usually depend on a different base image that already has a kernel that supports the image. There are some people that have suggested RPi Dockerfile but until now I have denied officially supporting them because they need to be manually build. We do have a community one at https://hub.docker.com/r/homeassistant/rpi2-home-assistant/ |
mkdir -p build && cd build | ||
|
||
cmake \ | ||
-DPYTHON_LIBRARY="/usr/local/lib/libpython3.5m.so" \ |
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.
Could we make this depend on the current running version of Python? Because I was planning on updating our docker image to use Python 3.6 but I don't want such updates to be blocked by installing a dependency.
Updated to dynamically load python paths from the active python executable. I've also opened an issue upstream to track the hardcoded Debian path in their cmake files: Pulse-Eight/libcec#288 |
apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* | ||
|
||
COPY script/build_python_openzwave script/build_python_openzwave | ||
RUN script/build_python_openzwave && \ | ||
mkdir -p /usr/local/share/python-openzwave && \ | ||
ln -sf /usr/src/app/build/python-openzwave/openzwave/config /usr/local/share/python-openzwave/config | ||
|
||
COPY script/build_libcec script/build_libcec | ||
RUN script/build_libcec |
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.
You should remove the build directory after the script invocation.
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 wasn't done in this PR.
-> #5243
Description:
Adds libcec to the docker image.