Skip to content

Enable docker build by only cloning the repository #604

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.

Sign up for GitHub

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

Open
Checker8763 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Enable docker build by only cloning the repository #604

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f86cd6
Update setup.py
Checker8763 May 17, 2025
fff5bc8
Update Dockerfile
Checker8763 May 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
FROM python
FROM python:3-slim AS builder

COPY websockify-*.tar.gz /
WORKDIR /build/websockify

RUN python3 -m pip install websockify-*.tar.gz
RUN rm -rf /websockify-* /root/.cache
COPY ./* /build/websockify

RUN pip3 install setuptools && \
python3 setup.py sdist --dist-dir docker/

# Actual final image
FROM python:3-slim

@CendioOssman CendioOssman May 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are we losing in this switch? We have a bunch of optional dependencies, like numpy, that users probably want.

@Checker8763 Checker8763 May 26, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There is essentially nothing lost, only gained.
With the slim version the final image size reduced by only having the necessary runtime requirenments for python.
With specifying version 3 the builds are more explicit which helps anybody reading the Dockerfile, and making the build more predictable by actually being explicit to the dependend python version (Imagine python 4 is released and the dockerfile simply states python with an implicit latest).
By using the slim version there is less traffic ~1.2GB vs ~200MB and less stored.

Also by splitting the build into two stages the size is also reduced by only copying the necessary files to the final image, leaving any cache, temporary files etc. behind.

@Checker8763 Checker8763 May 26, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Forgott that there is nothing lost, the full requirenments are installed.

Only the base Image i think Debian or Ubuntu, is not installed in a full fledged way but in a minimal with python runtime.

@CendioOssman CendioOssman Jun 3, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, it seems like the Python side are indeed the same.

@Checker8763 Checker8763 Jun 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When I run the build in the "specific_package" branch in the git root with the command

docker build -t websockify -f docker/Dockerfile .

I get this error:

 > [builder 4/4] RUN pip3 install setuptools &&     python3 setup.py sdist --dist-dir docker/:
3.736 Collecting setuptools
3.786   Downloading setuptools-80.9.0-py3-none-any.whl.metadata (6.6 kB)
3.977 Downloading setuptools-80.9.0-py3-none-any.whl (1.2 MB)
4.359    ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.2/1.2 MB 2.6 MB/s eta 0:00:00
4.373 Installing collected packages: setuptools
5.149 Successfully installed setuptools-80.9.0
5.149 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager, possibly rendering your system unusable. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv. Use the --root-user-action option if you know what you are doing and want to suppress this warning.
5.540 running sdist
5.540 running egg_info
5.545 creating websockify.egg-info
5.551 writing websockify.egg-info/PKG-INFO
5.551 writing dependency_links to websockify.egg-info/dependency_links.txt
5.552 writing entry points to websockify.egg-info/entry_points.txt
5.552 writing requirements to websockify.egg-info/requires.txt
5.552 writing top-level names to websockify.egg-info/top_level.txt
5.552 writing manifest file 'websockify.egg-info/SOURCES.txt'
5.558 error: package directory 'websockify' does not exist
------
Dockerfile:7
--------------------
   6 |
   7 | >>> RUN pip3 install setuptools && \
   8 | >>>     python3 setup.py sdist --dist-dir docker/
   9 |
--------------------
ERROR: failed to solve: process "/bin/sh -c pip3 install setuptools &&     python3 setup.py sdist --dist-dir docker/" did not complete successfully: exit code: 1

@Checker8763 Checker8763 Jun 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It seems to be tied to the environment, if I wrap the statements in a shell script and execute them with bash it works just fine, but when executed as a plain command in the docker file in a RUN statement it somehow breaks...

@Checker8763 Checker8763 Jun 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have debugged a bit further and found that find_packages() resolves to []

When I hardcode packages=[] it builds just fine.

@Checker8763 Checker8763 Jun 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have found the following section in the python docs:

~ https://docs.python.org/3.11/distutils/setupscript.html#listing-whole-packages

The keys to this dictionary are package names, 
and an empty package name stands for the root package. 
The values are directory names relative to your distribution root. 
In this case, when you say packages = ['foo'], 
you are promising that the file lib/foo/__init__.py exists.

@Checker8763 Checker8763 Jun 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

While reading in https://setuptools.pypa.io/en/latest/userguide/package_discovery.html

Another workaround I have found was to include package_dir = {"websockify": ""}, or package_dir = {"websockify": "."}, or package_dir = {"websockify": "/"},

This seems to be the least required amount of changes, to make the dockerfile work while leaving the statements in the dockerfile unchanged.

@CendioOssman CendioOssman Jun 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm afraid I cannot reproduce the issue here. And it really should work. The documentation you refer to states:

Thus, when you say packages = ['foo'] in your setup script, you are promising that the Distutils will find a file foo/__init__.py (which might be spelled differently on your system, but you get the idea) relative to the directory where your setup script lives.

And since we say packages=['websockify'], we claim that there is a websockify directory next to setup.py. Which there is.

It seems like there is something more in your environment that is fouling thing.

COPY --from=builder /build/websockify/websockify-*.tar.gz /
RUN python3 -m pip install websockify-*.tar.gz && \
rm -rf /websockify-* /root/.cache
Comment thread
Checker8763 marked this conversation as resolved.

VOLUME /data

Expand Down