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

[WIP]: Add Dockerfile #75

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

arjxn-py
Copy link

@arjxn-py arjxn-py commented Apr 5, 2024

Still a work in progress.
This PR aims to add a Dockerfile to containerize the package & publish Docker images of the same.

  • Can be GLIBC-based
  • Needs to derive from official Python images
  • Should be multi-architecture
  • Should come with a C compiler (GCC/Clang)
  • Should come with Git installed
  • Should come with Go installed (same version as upstream)
  • Should come with Zig installed
  • Building & testing of Image in CI

Related to #74

@arjxn-py arjxn-py marked this pull request as draft April 5, 2024 17:55
@agriyakhetarpal
Copy link
Owner

I just released v0.125.0, which brings some changes to how wheels are named and a lot of other improvements. It might or might not break things here – I would suggest testing against the new changes on main here and rebasing when you are ready to come back to this.

@arjxn-py arjxn-py force-pushed the add-dockerfile branch 2 times, most recently from de7acc6 to 6c171e7 Compare April 19, 2024 10:01
@arjxn-py
Copy link
Author

Brief Update - A preliminary Dockerfile has been set up, Parameterization of Dockerfile for dynamic version installation remains, plus there are some issues while activating venv, I'm on it :)

@agriyakhetarpal
Copy link
Owner

Sure, could we skip the venv? I think we do not plan to add any external dependencies from PyPI, so we won't ever need the utilities of the isolation that a virtual environment provides, because the Docker container would provide that. Installing straight with the system pip should be alright.

@agriyakhetarpal
Copy link
Owner

Parameterization of Dockerfile for dynamic version installation remains

I'm not sure I follow. Do you mean installing multiple Hugo versions? I think just one Hugo installation with one Python version is okay (for me, at least).

@arjxn-py
Copy link
Author

I'm not sure I follow. Do you mean installing multiple Hugo versions?

No no, by this I meant about the version of go & zig. I should have specified that above 😅

@agriyakhetarpal
Copy link
Owner

Fair enough. I don't have any release automation for bumping versions set up right now given how minimal the entire project is, so it will be okay to keep the Zig and Go tarballs hardcoded (as they currently are).

@agriyakhetarpal
Copy link
Owner

agriyakhetarpal commented Apr 19, 2024

We can't really control the C compiler that we receive from the system package manager. However, I determine the Go toolchain version from the notes for every Hugo release. That is more important to keep in sync because of security concerns; sometimes some CVEs are fixed – so I make sure to do so in all relevant files when bumping the Hugo version (which is at the time of publishing a release).

@agriyakhetarpal
Copy link
Owner

Through #81 I added some non-GitHub Linux aarch64/arm64 runners (the capacity is limited to 800 minutes per month or so, as I am on 5$ of free credit as a trial). Feel free to use them in a workflow or two here in your PR (won't work on your fork); I would happily approve workflow runs for them. It will be much better than running QEMU and waiting for 40 minutes to build.

@arjxn-py
Copy link
Author

arjxn-py commented Apr 22, 2024

Through #81 I added some non-GitHub Linux aarch64/arm64 runners (the capacity is limited to 800 minutes per month or so, as I am on 5$ of free credit as a trial). Feel free to use them in a workflow or two here in your PR (won't work on your fork); I would happily approve workflow runs for them. It will be much better than running QEMU and waiting for 40 minutes to build.

Thanks for this @agriyakhetarpal, I am in the process to define +1 workflow for Docker images hence it is nicer to use aarch64/arm64 runners instead, I anticipate that the capacity wouldn't be an issue at the moment as it takes ~10 minutes to build the image for me locally.
Plus, as you suggested, I've gotten rid of venv for now and added hardcoded tarballs for go & zig.

Dockerfile Outdated Show resolved Hide resolved
@arjxn-py arjxn-py marked this pull request as ready for review April 22, 2024 12:55
Copy link
Owner

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

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

Thanks for writing this out, @arjxn-py, much appreciated! I had initially planned to publish these Docker images when I opened the issue, yes – however, I now am backing away from it for two reasons and some of my opinions about this have changed:

  1. Hugo has Docker images already where it is being distributed, and those run without the need for a Python base image (considering that Hugo is a standalone binary and does not necessarily need a Python wrapper around it). Those will be much more reliable, and I would myself suggest people use those instead of ours, and use our built wheels instead if needed (also because the current Dockerfile targets only a stable release and not a development build)
  2. Where I think these images would be more useful would be for something like getting access to a development build on a Linux runtime while being on a Windows one (where setting up compilers and other things is a bit tricky) or on any other platforms where Docker images can work. Or, for building MUSL-linked static binaries for Hugo extended via Zig using an Alpine base image (which is something that even the upstream Hugo project does not do right now, they link against GLIBC for those binaries, and the non-extended version is statically linked I think).

Therefore, I would suggest building a MUSL image for both architectures with separate Dockerfiles (the current implementation is amd64 specific so it would not have worked in the docker/build-push-action@v5 step) and using Zig to compile it with static linkage (some instructions for this are available in the documentation which you can use). This way, we can remove the Golang installation in the Docker images too and keep everything quite minimal. Publishing the images could be looked at some later time when we would have tested these images properly, but personally I think we shouldn't release them at all since the original plan of the project was to provide isolation and easy management of Hugo binaries, which remains largely accomplished now with tools like pipx and Python virtual environments.

@agriyakhetarpal
Copy link
Owner

Further context: I had previously tried MUSL wheels in PRs related to #91 and received a few segmentation faults. I didn't do a debug build and look at them in detail, though. Maybe we'll need to change BUILD_FOR_WINDOWS to BUILD_STATIC or something to be able to build a static library. If that doesn't work, we can publish a GLIBC-based image.

setup.py Outdated Show resolved Hide resolved
Copy link
Owner

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

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

Thanks, @arjxn-py! Let me test the Dockerfile locally to see if the MUSL builds are working, but for now, maybe we need to think about the predicament imposed by the docker.yml workflow. I have thought that the Dockerfile should be a method to build any released versions of Hugo from source or for those running MUSL systems that need a working build, but I'll still need to think about whether this should be published to Docker Hub or not. Hugo has had a recent PR for official images that could be accepted by the maintainers: gohugoio/hugo#12573, and in general everything would be better shaped if an official image is decided upon (because there have been other images in the past that gained much more popularity than the official images, see gohugoio/hugo#10760 but maintenance is an issue).

We should still modify the workflow here to test the hugo version and hugo env commands inside the built Docker image. We shouldn't push it (so maybe change to push: false).

If the MUSL builds don't work, then it's probably best to switch to a glibc-based Python image as discussed above.

setup.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request packaging infrastructure Releases, installation, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants