Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions .github/workflows/CI-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ jobs:

runs-on: ubuntu-latest
steps:
- name: Maximize build space
uses: AdityaGarg8/remove-unwanted-software@v5
with:
remove-android: "true"
remove-dotnet: "true"
remove-haskell: "true"

- name: Checkout
uses: actions/checkout@v4
with:
Expand Down Expand Up @@ -49,7 +56,6 @@ jobs:
lychee0/finalrip:${{ matrix.dockerfile }}-cuda-dev
build-args: |
BASE_CONTAINER_TAG=cuda-dev
no-cache: true

- name: Build and push for other services
if: matrix.dockerfile != 'worker-encode'
Expand All @@ -60,4 +66,3 @@ jobs:
platforms: linux/amd64, linux/arm64
push: ${{ github.event_name != 'pull_request' }}
tags: lychee0/finalrip:${{ matrix.dockerfile }}-dev
no-cache: true
9 changes: 7 additions & 2 deletions .github/workflows/Release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ jobs:

runs-on: ubuntu-latest
steps:
- name: Maximize build space
uses: AdityaGarg8/remove-unwanted-software@v5
with:
remove-android: "true"
remove-dotnet: "true"
remove-haskell: "true"

- name: Checkout
uses: actions/checkout@v4
with:
Expand Down Expand Up @@ -43,7 +50,6 @@ jobs:
lychee0/finalrip:${{ matrix.dockerfile }}-cuda-${{ github.ref_name }}
build-args: |
BASE_CONTAINER_TAG=cuda-v0.4.0
no-cache: true

- name: Build and push for other services
if: matrix.dockerfile != 'worker-encode'
Expand All @@ -56,7 +62,6 @@ jobs:
tags: |
lychee0/finalrip:${{ matrix.dockerfile }}
lychee0/finalrip:${{ matrix.dockerfile }}-${{ github.ref_name }}
no-cache: true

github:
needs: [docker]
Expand Down
2 changes: 1 addition & 1 deletion common/version/version.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package version

const FINALRIP_VERSION = "v0.4.0"
const FINALRIP_VERSION = "v0.5.0"
9 changes: 5 additions & 4 deletions deploy/worker-merge.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@
FROM debian:bookworm AS app

# prepare environment
RUN apt update -y && apt upgrade -y
RUN apt install -y wget
RUN wget -O /etc/apt/keyrings/gpg-pub-moritzbunkus.gpg https://mkvtoolnix.download/gpg-pub-moritzbunkus.gpg
RUN apt update -y && apt upgrade -y
RUN apt update -y && apt upgrade -y && \
apt install -y wget && \
wget -O /etc/apt/keyrings/gpg-pub-moritzbunkus.gpg https://mkvtoolnix.download/gpg-pub-moritzbunkus.gpg && \
apt update -y && apt upgrade -y
Comment on lines +17 to +20

Choose a reason for hiding this comment

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

high

While combining RUN instructions is a good step towards optimizing the Docker image, this can be improved further for correctness and efficiency. I'm using apt-get in my suggestion as it's generally recommended for scripting in Dockerfiles over apt.

  • Correctness: To install mkvtoolnix from the mkvtoolnix.download repository, you need to add its source to apt's sources list. This step is currently missing, which likely results in installing an older version from the default Debian repositories.
  • Efficiency: The apt upgrade commands are often unnecessary, slow down the build, and increase image size. It's better to install specific packages from an up-to-date base image.

The suggestion below fixes these issues for the lines you've changed. For maximum benefit, you should also combine the RUN apt install -y mkvtoolnix command into this single RUN block and add cleanup steps at the end. A fully optimized command would look like this:

RUN apt-get update && \
    apt-get install -y --no-install-recommends wget ca-certificates && \
    wget -O /etc/apt/keyrings/gpg-pub-moritzbunkus.gpg https://mkvtoolnix.download/gpg-pub-moritzbunkus.gpg && \
    echo "deb [signed-by=/etc/apt/keyrings/gpg-pub-moritzbunkus.gpg] https://mkvtoolnix.download/debian/ bookworm main" > /etc/apt/sources.list.d/mkvtoolnix.list && \
    apt-get update && \
    apt-get install -y --no-install-recommends mkvtoolnix && \
    apt-get purge -y --auto-remove wget && \
    rm -rf /var/lib/apt/lists/*

Since I can only suggest changes to the modified lines, my suggestion below only covers the preparation steps.

RUN apt-get update && \
    apt-get install -y --no-install-recommends wget ca-certificates && \
    wget -O /etc/apt/keyrings/gpg-pub-moritzbunkus.gpg https://mkvtoolnix.download/gpg-pub-moritzbunkus.gpg && \
    echo "deb [signed-by=/etc/apt/keyrings/gpg-pub-moritzbunkus.gpg] https://mkvtoolnix.download/debian/ bookworm main" > /etc/apt/sources.list.d/mkvtoolnix.list && \
    apt-get update


RUN apt install -y mkvtoolnix

WORKDIR /app

ENV TZ=Asia/Shanghai

ENV FINALRIP_EASYTIER_HOST 10.126.126.251

Check warning on line 28 in deploy/worker-merge.dockerfile

View workflow job for this annotation

GitHub Actions / docker (worker-merge)

Legacy key/value format with whitespace separator should not be used

LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format More info: https://docs.docker.com/go/dockerfile/rule/legacy-key-value-format/

COPY --from=mwader/static-ffmpeg:8.0 /ffmpeg /usr/local/bin/
COPY --from=mwader/static-ffmpeg:8.0 /ffprobe /usr/local/bin/
Expand Down
Loading