ci: add cache for ClamAV and use dedicated script for package installation#2752
Closed
georglauterbach wants to merge 8 commits intomasterfrom
Closed
ci: add cache for ClamAV and use dedicated script for package installation#2752georglauterbach wants to merge 8 commits intomasterfrom
georglauterbach wants to merge 8 commits intomasterfrom
Conversation
This commit introduces a new stage (`clamav-stage`) and the `--link` flag for teh `COPY` directive. We now require `DOCKER_BUILDKIT` to be set to `1`. Moreover, the `Dockerfile` syntax is explictly set in the `Dockerfile` (required for `--link`). A bit of refactoring was done in the Dockerfile to keep everything consistent.
This commit does more refactoring. The build arguments names' were changed too. I did this because I was not really sure what `_REF` was for.
This commit prepares functionality for the next commit. It refactors the script a bit too. The style of function definitions was changed, because I think many people do not like the style I introduced a (long) while back, so I want to make an effort here.
This commit outsources package installation into a dedicated script. I think the `Dockerfile` was cluttered by this huge `RUN` statement, a dedicated script is more fitting in my eyes. I made sure to properly separate all different steps when it comes to the installation process. Moreover, you will notices that 1. many packages were removed - these packages are almost all packages related to compressing / decompressing / reading archives, etc. which the core image of DMS does not rely on 2. the `rspamd` package was added (there is no configuration done at this point in time however - a separate PR will follow) Therefore, THIS COMMIT IS A BREAKING CHANGE. Users that rely on the various packages for reading certain archives will now need to install these packages themselves.
Member
Author
11 tasks
Accidently removed too many packages.. I hope this is the only package that needs to be re-added..
The environemnt variables `LOG_LEVEL` and `DEBIAN_FRONTEND` are now properly located in the `Dockerfile`. Previously, they were not respected, because they were only build arguments.
polarathene
requested changes
Aug 30, 2022
polarathene
reviewed
Aug 30, 2022
This comment was marked as outdated.
This comment was marked as outdated.
Member
|
It was said somewhere else already and I second this again: To make reviews easier, keep them as small as possible and avoid changes not directly needed for the goal a PR achieves (such as beautifying things or introducing other little additions like rspamd). Edit: Ok, you already addressed this here. |
Member
Author
|
I see, my PR became a bit bit and not UNIX-oid.. I will provide smaller ones that incorporate the review feedback - thank's @polarathene :) |
7 tasks
georglauterbach
added a commit
that referenced
this pull request
Aug 30, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR does two things:
The separate script provides a better overview of the installation process. While I was at it, I removed non-essential packages from the packages list. Therefore, this PR is a breaking change.
Moreover, users that want to build DMS themselves are now required to use a recent version of Docker because we need BuildKit (one needs Docker version
>= 20.10.14).Type of change
Checklist:
docs/)