Skip to content

Conversation

master-hax
Copy link
Contributor

@master-hax master-hax commented Sep 18, 2024

description

this PR adds 2 new dockerfiles that allow users & developers to build containers from source in reference to previous discussion, restoring functionality that was lost in #59. one issue i see people facing is that doing a docker build against a specific branch or commit unexpectedly ignores that and uses the latest release.

the original dockerfiles were:

they were all basically the same alpine build but used emulation to support different architectures. however we can now use the built in multi-arch support from docker. this still uses emulation but it is simpler and no longer needs specific handling in the dockerfile.

i condensed all three of those down to Dockerfile.alpine with small improvements & added a brand new ubuntu based Dockerfile.ubuntu.

i hope that we can use these to replace the current no-op dockerfile in a future PR. i have an example of releasing multi-arch builds with cross-compilation here.

usage examples:

local docker build:

docker build . -f ./Dockerfile.alpine
docker build . -f ./Dockerfile.ubuntu
docker build . -f ./Dockerfile.alpine --build-arg ALPINE_VERSION=3.19
docker build . -f ./Dockerfile.ubuntu --build-arg UBUNTU_RELEASE_VERSION=jammy

local buildx build (enables cross-compilation and multi-arch containers):

docker buildx build --platform linux/amd64,linux/arm64 . -f ./Dockerfile.alpine
docker buildx build --platform linux/amd64 . -f ./Dockerfile.ubuntu
docker buildx build --platform linux/arm64 . -f ./Dockerfile.alpine --build-arg ALPINE_VERSION=3.19
docker buildx build --platform linux/amd64,linux/arm64 . -f ./Dockerfile.ubuntu --build-arg UBUNTU_RELEASE_VERSION=jammy

compose file from remote source:

services:
  redlib:
    build:
      context: https://github.com/redlib-org/redlib.git
      dockerfile: Dockerfile.alpine
services:
  redlib:
    build:
      context: https://github.com/redlib-org/redlib.git#v0.31.2
      dockerfile: Dockerfile.ubuntu
      args:
        UBUNTU_RELEASE_VERSION: "jammy"

compose file from local source (best for development):

services:
  redlib:
    build:
      context: ~/git/redlib
      dockerfile: Dockerfile.ubuntu
services:
  redlib:
    build:
      context: "$PWD"
      dockerfile: Dockerfile.alpine
      args:
        ALPINE_VERSION: "3.20"

@master-hax
Copy link
Contributor Author

cc @sigaloid @chowder

@master-hax master-hax changed the title [build] add new dockerfiles [build] add new dockerfiles for building from source Sep 18, 2024
Copy link
Contributor

@pimlie pimlie left a comment

Choose a reason for hiding this comment

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

Nice trick to cache the cargo dependencies! Left some remarks on the Ubuntu file that mostly also apply to the Alpine file 👍

@Tokarak
Copy link
Contributor

Tokarak commented Sep 20, 2024

By The Way, I recommend using LukeMathWalker/cargo-chef to cache the compilation of dependencies. I created this pr (mattfbacon/typst-bot#23) for another rust project, which you can use along with the official documentation.

Sorry, I didn't read this pr fully — maybe cargo chef should be in a new pr.

@pimlie
Copy link
Contributor

pimlie commented Sep 22, 2024

@Tokarak Reading the Benefits of cargo-chef, the main benefit for using cargo chef seems to be when you are using workspaces. As redlib doesn't use those (and there is only one cargo.toml file), just using @master-hax's implementation here should be fine.

@master-hax Note that docker layer caching also requires extra configuration in the workflows, see https://docs.docker.com/build/ci/github-actions/cache/

@Tokarak
Copy link
Contributor

Tokarak commented Sep 22, 2024

@pimlie cargo chef cook builds all the dependencies as a seperate layer, same as in the hack used in this pr. The benefit over this hack is that it generates less layers, is more obvious with what it does, and is resistant to breaking changes in Redlib and Rust because it is maintained.

@pimlie
Copy link
Contributor

pimlie commented Sep 22, 2024

@Tokarak It also means you have to fully trust the maintainers of cargo chef and their release process. Have you met them in person?
Same applies ofc to other software used during building, but recent years have shown multiple attacks on various ecosystems so less dependencies is always better. Im happy to leave it to others to decide whether cargo chef is a useful dependency or not :)

@master-hax
Copy link
Contributor Author

master-hax commented Sep 22, 2024

@Tokarak says:

Get rid of this hack and use cargo-chef.

as i have mentioned, a similar optimization was merged a year ago. also as @pimlie pointed it, using cargo-chef adds a major (& unnecessary) dependency to the build system.

if we prefer to use cargo-chef, i think someone should put up a separate PR to add that functionality so we can keep the scope of this PR small.

@master-hax
Copy link
Contributor Author

i think all the comments have been resolved

Copy link
Contributor

@pimlie pimlie left a comment

Choose a reason for hiding this comment

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

LGTM after these 2 small changes

This might be known already, but please note that docker layer caching of github actions also requires additional configuration in the github action workflow. Without that addtional config the layers won't ever be cached, see https://docs.docker.com/build/ci/github-actions/cache/

@master-hax
Copy link
Contributor Author

@sigaloid do you think this looks good to merge?

@sigaloid
Copy link
Member

I think this looks good. I just switched to openssl via native-tls in a96bebb. What impacts will this have on your dockerfiles, since we no longer use 100% rust-native dependencies?

ARM builds are currently broken (#326) so if we could replace our builds with this, provided it still works, that would be fantastic.

PS: I have very little knowledge of cross-building especially for other architectures, and the native dependencies only make it worse. So please bear with me :)

@master-hax
Copy link
Contributor Author

What impacts will this have on your dockerfiles, since we no longer use 100% rust-native dependencies?

rebased the PR branch for testing

@master-hax
Copy link
Contributor Author

master-hax commented Nov 22, 2024

both dockerfiles seem to work fine on both of my x86_64 & aarch64 machines without explicitly installing libssl-dev - i can add it if needed

@master-hax
Copy link
Contributor Author

bump cc @sigaloid can we move forward with this pull request?

bdamokos added a commit to bdamokos/redlib that referenced this pull request Jan 6, 2025
@sigaloid
Copy link
Member

sigaloid commented Feb 3, 2025

LGTM. Looking forward to the future PRs that enable this functionality. Release handling is honestly my least favorite part of development, I'm glad to have expertise in this area from the community :)

@sigaloid sigaloid merged commit 0703fa1 into redlib-org:main Feb 3, 2025
2 of 3 checks passed
mlightsys added a commit to mlightsys/redlib that referenced this pull request Feb 6, 2025
rinlabs pushed a commit to rinlabs/redlib that referenced this pull request Feb 16, 2025
* add new dockerfiles

* update default ubuntu base images

* updates

* update comment

* update cargo command

Co-authored-by: Pim <[email protected]>

* update cargo command

Co-authored-by: Pim <[email protected]>

* specify binary

* use label instead of maintainer

---------

Co-authored-by: Pim <[email protected]>
disconn3ct added a commit to disconn3ct/redlib that referenced this pull request Feb 18, 2025
* upstream/mistress: (34 commits)
  Supply GIT_HASH via build.sh
  Remove ci/cd stuff
  Build with rust 1.84.1
  Revert "[build] add new dockerfiles for building from source (redlib-org#244)"
  Remove push from build.sh
  Add .editorconfig
  Disable update check
  Build only redlib
  Update .dockerignore
  Add .dockerignore
  Use --no-cache with apk add
  Add build-deploy.sh
  Update Dockerfile
  Fix Dockerfile
  fix: remove stray trace
  chore: remove scraper cli
  fix Code blocks err redlib-org#227  (redlib-org#323)
  fix(clippy): minor clippy changes
  refactor(utils): avoid redundant String conversions & use match (redlib-org#347)
  fix: fix clippy + tests
  ...
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.

4 participants