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

Simplify the Docker image build process #123

Closed
wants to merge 9 commits into from

Conversation

np22-jpg
Copy link
Contributor

@np22-jpg np22-jpg commented May 31, 2024

The original PR by @nohoster, #59, aimed to completely overhaul the container build pipeline to greatly speed up builds by de-deduplicating work by downloading redlib binaries built in CI. However, build-artifacts for whatever reason, does not trigger the upload artifacts step. This means issues such as #104, #122, and apparently, #118 (based on the comment about updating) appear. This has not been fixed in awhile (5 releases!).

As a result, I propose moving back towards a more traditional Dockerfile. Instead of downloading a pre-completed build, the contents of the git tree are built as usual.

This PR also re-introduces testing in the Dockerfile, and both the builder and release images are on the same distro, as opposed to the ubuntu -> alpine pattern as before. Doing it this way also ensures users can build for more platforms than 64-bit x86 and ARM, as well as 32-bit ARM.

@np22-jpg
Copy link
Contributor Author

The build currently does spuriously fail with:

failures:

---- utils::test_fetching_nsfw_subreddit stdout ----
thread 'utils::test_fetching_nsfw_subreddit' panicked at src/utils.rs:1187:5:
assertion failed: subreddit.is_ok()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    utils::test_fetching_nsfw_subreddit

In the future, it might be best to comment out the line RUN cargo test --release until it can be fixed properly

@np22-jpg np22-jpg changed the title Simplify the dokcer image build process Simplify the Docker image build process May 31, 2024
@np22-jpg
Copy link
Contributor Author

np22-jpg commented May 31, 2024

I went ahead and dropped building armv7. I kept getting random errors:

[linux/arm/v7 builder 4/5] RUN cargo test --release:
17.25 error: failed to determine package fingerprint for build script for redlib v0.34.0 (/app)
17.26
17.26 Caused by:
17.26 failed to determine the most recently modified file in /app
17.26
17.26 Caused by:
17.26 failed to determine list of files in /app
17.26
17.26 Caused by:
17.26 could not read directory '/app/.github': Value too large for defined data type; class=Os (2)


In addition, armv7 is currently not a "tier 1 platform" according to Rust. Mainstream distros, such as Fedora, don't even support it anymore. Users still using older Raspberry Pis may be able to use older images, or re-contribute support.

In addition, I ended up commenting out testing in the Dockerfile. For the most part, this Dockerfile is just to get an MVP back up and running again. If requested, I'd be totally fine squashing all my commits to make reverting easier down the line.

Here's a successful build, please ignore #125 as I meant to make it on my fork:
https://github.com/np22-jpg/redlib/actions/runs/9315087740/job/25640545699

Edit: #126 might fix this as a whole

@np22-jpg
Copy link
Contributor Author

np22-jpg commented Jun 2, 2024

Closing as #126 was merged!

@np22-jpg np22-jpg closed this Jun 2, 2024
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.

1 participant