feat: optimize FrankenPHP images - #27
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Dockerfile now uses verified builder stages for runtime, development, and shell tooling. Image validation checks versions, binaries, forbidden resources, and writable paths. CI, tagging, documentation, environment configuration, and licensing are updated. ChangesImage build and release workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f867b0705b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| libavif15 \ | ||
| libavcodec59 \ | ||
| libavformat59 \ | ||
| libavutil57 \ | ||
| libswscale6 \ |
There was a problem hiding this comment.
When docker-bake.hcl expands a Trixie target, this step still requests Bookworm ABI package names such as libavcodec59, libavformat59, and libavutil57; Trixie supplies newer ABI/t64 package names, so apt-get install exits with packages not found and prevents both Trixie variants from building. Install the tools and their dependencies from the target suite rather than hard-coding Bookworm library versions.
Useful? React with 👍 / 👎.
| RUN apt update \ | ||
| && apt-get install -y gnupg lsb-release ca-certificates curl \ | ||
| # Copy image optimization tools from builder | ||
| COPY --from=tools-builder --chmod=755 /tmp/tools/* /usr/local/bin/ |
There was a problem hiding this comment.
When an image actually runs these copied executables, several fail at dynamic loading because only the binaries are copied: Bookworm's ffmpeg also requires libraries including libavdevice59, libavfilter8, and libpostproc56, while pngquant requires libimagequant0, none of which is installed in the final stage. The acceptance check only uses command -v, so it reports these unusable tools as present; install the packages in the final stage or copy their full dependency closure.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
| && useradd -m --no-user-group -o -g ${WWWGROUP} -u ${WWWUSER} -s /bin/zsh ${USER} \ | ||
| && setcap CAP_NET_BIND_SERVICE=+eip /usr/local/bin/frankenphp \ | ||
| && mkdir -p /home/${USER}/.local/bin \ | ||
| && chown -R ${USER}:${USER} /home/${USER} /data/caddy /config/caddy |
There was a problem hiding this comment.
When the image is used without a bind mount and the deploy user needs to create or modify application files, /app remains the root-owned directory inherited from the upstream image because it was removed from this chown list. Since both final stages subsequently switch to USER deploy, scaffolding an application or writing Laravel runtime files under /app fails with permission denied; include /app in the ownership setup as the previous image did.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Dockerfile (1)
23-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInconsistent
curlfailure handling across download steps.The pnpm download (Line 32) uses
-fsSLplus explicittestguards, but the Node.js (Lines 25-26),gh(Line 50), andeza(Line 58) downloads use plain-s/-sLwithout-f. On a GitHub API rate-limit or a 404, these will silently pipe an error page intotar, and only fail later with a confusing "not a gzip file" error instead of a clear HTTP failure.♻️ Suggested consistency fix
-RUN GH_VERSION=$(curl -s https://api.github.com/repos/cli/cli/releases/latest | jq -r .tag_name | sed 's/^v//') \ +RUN GH_VERSION=$(curl -fsSL https://api.github.com/repos/cli/cli/releases/latest | jq -r .tag_name | sed 's/^v//') \ && if [ "$TARGETARCH" = "amd64" ]; then ARCH="amd64"; else ARCH="arm64"; fi \ - && curl -sL "https://github.com/cli/cli/releases/download/v${GH_VERSION}/gh_${GH_VERSION}_linux_${ARCH}.tar.gz" -o gh.tar.gz \ + && curl -fsSL "https://github.com/cli/cli/releases/download/v${GH_VERSION}/gh_${GH_VERSION}_linux_${ARCH}.tar.gz" -o gh.tar.gz \ && tar -xzf gh.tar.gz \ && mv gh_${GH_VERSION}_linux_${ARCH}/bin/gh gh \ && rm -rf gh*.tar.gz gh_*Also applies to: 50-60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 23 - 29, Update the Node.js, gh, and eza download commands to use curl’s fail-on-HTTP-error option consistently with the pnpm download, while preserving their existing silent and redirect-following behavior. Apply this to the curl invocations in the Node version setup block and the gh/eza download sections so HTTP failures stop immediately before data is piped into tar or installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 66-85: Update the tools-builder stage and base runtime
dependencies for the binaries copied by the Dockerfile, especially pngquant and
ffmpeg. Use ldd on the staged executables to identify their transitive
shared-library requirements, then install the corresponding runtime packages in
the base image (including libimagequant0 and missing ffmpeg codec/runtime
libraries) before the COPY step. Ensure direct invocations of every copied tool
work without shared-library loading errors.
In `@README.md`:
- Line 18: Update the README.md PHP Extensions list to include ftp, matching the
extension installed by the Dockerfile while preserving the existing entries.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 23-29: Update the Node.js, gh, and eza download commands to use
curl’s fail-on-HTTP-error option consistently with the pnpm download, while
preserving their existing silent and redirect-following behavior. Apply this to
the curl invocations in the Node version setup block and the gh/eza download
sections so HTTP failures stop immediately before data is piped into tar or
installed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 30c51ff2-582b-4553-994c-cadeba8c81a3
📒 Files selected for processing (5)
AGENTS.mdCLAUDE.mdDockerfileREADME.mdtest.php
💤 Files with no reviewable changes (1)
- CLAUDE.md
| FROM debian:bookworm-slim AS tools-builder | ||
|
|
||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| jpegoptim \ | ||
| optipng \ | ||
| pngquant \ | ||
| gifsicle \ | ||
| libavif-bin \ | ||
| ffmpeg \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Copy binaries to a clean location | ||
| RUN mkdir -p /tmp/tools \ | ||
| && cp /usr/bin/jpegoptim /tmp/tools/ \ | ||
| && cp /usr/bin/optipng /tmp/tools/ \ | ||
| && cp /usr/bin/pngquant /tmp/tools/ \ | ||
| && cp /usr/bin/gifsicle /tmp/tools/ \ | ||
| && cp /usr/bin/avifenc /tmp/tools/ \ | ||
| && cp /usr/bin/ffmpeg /tmp/tools/ | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Run in a container built from tools-builder to enumerate missing libs, e.g.:
# ldd /tmp/tools/ffmpeg /tmp/tools/pngquant /tmp/tools/avifencRepository: prvious/frankenphp
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== Dockerfile relevant lines =="
sed -n '1,220p' Dockerfile | nl -ba | sed -n '60,170p'
echo "== search for runtime image and ffmpeg/pngquant references =="
rg -n -C 3 'tools-builder|FROM .*AS base|FROM base|ffmpeg|pngquant|apt-get install|ldd|/tmp/tools' Dockerfile .github 2>/dev/null || trueRepository: prvious/frankenphp
Length of output: 533
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Dockerfile relevant lines =="
awk '{printf "%5d\t%s\n", NR, $0}' Dockerfile | sed -n '55,170p'
echo "== Dockerfile lines 130-160 =="
awk '{printf "%5d\t%s\n", NR, $0}' Dockerfile | sed -n '125,160p'
echo "== search for stage references and binary usage =="
grep -EIn 'tools-builder|AS base|FROM base|FROM.*base|ffmpeg|pngquant|apt-get install|ldd|/tmp/tools|usr/local|COPY --from=tools-builder' Dockerfile .github 2>/dev/null || trueRepository: prvious/frankenphp
Length of output: 8657
🌐 Web query:
Debian Bookworm ffmpeg packages libavcodec59 libavformat59 libavutil57 ldd dependencies libjpeg62-turbo libpng16-16 libavif15 missing libraries
💡 Result:
In Debian Bookworm (Debian 12), the packages you mentioned are standard components of the FFmpeg multimedia framework and related image processing libraries [1][2]. These are official repository packages managed via APT, and they are not "missing" in the sense of being absent from the distribution [3][4][5]. The packages libavcodec59, libavformat59, and libavutil57 are core runtime libraries provided by the ffmpeg source package in Debian Bookworm [1][2]. Their dependencies are automatically resolved by the Debian package management system (APT/dpkg) during installation [6][7]. If you are encountering errors indicating missing libraries, it is typically due to a broken installation, an attempt to use libraries from a different release (e.g., mixing Bullseye or Sid repositories), or an environment configuration issue (e.g., incorrect LD_LIBRARY_PATH) [6][7]. Regarding the specific libraries you listed: 1. FFmpeg Libraries (libavcodec59, libavformat59, libavutil57): These are part of the main archive and depend on various codecs and system libraries (such as libaom3, libbluray2, libdav1d6, and zlib1g) to function [6][7]. You can verify their installation status by running apt list libavcodec59 libavformat59 libavutil57. 2. Image Libraries (libavif15, libjpeg62-turbo, libpng16-16): These are independent packages. libavif15 is used for AVIF image handling [8], while libjpeg62-turbo and libpng16-16 are essential JPEG and PNG runtime libraries [4][5]. These are also standard in Bookworm [8][4][5]. Troubleshooting Steps: If you are seeing "missing library" errors, follow these steps: 1. Update your package lists: Run sudo apt update. 2. Fix broken dependencies: Run sudo apt install -f or sudo apt --fix-broken install. 3. Reinstall the core packages: You can force a reinstall of these libraries to ensure no files are corrupted: sudo apt install --reinstall libavcodec59 libavformat59 libavutil57 libavif15 libjpeg62-turbo libpng16-16. 4. Check for library paths: If you are building software from source, ensure that your linker can find these libraries in /usr/lib/x86_64-linux-gnu/ (or the equivalent for your architecture) [9]. You can verify the location of a library using dpkg -L libavformat59. Avoid manually downloading individual.deb files from outside the official Debian repositories, as this often leads to the dependency hell you are experiencing. Always prefer the official repositories defined in your /etc/apt/sources.list file.
Citations:
- 1: https://packages.debian.org/source/bookworm/ffmpeg
- 2: https://packages.debian.org/search?searchon=sourcenames&keywords=ffmpeg
- 3: https://packages.debian.org/libavformat59
- 4: https://packages.debian.org/bookworm/libjpeg62-turbo
- 5: https://packages.debian.org/bookworm/libpng16-16
- 6: https://piuparts.debian.org/bookworm/pass/libavformat59_7:5.1.8-0+deb12u1.log
- 7: https://piuparts.debian.org/bookworm2next/pass/libavcodec59_7:5.1.8-0+deb12u1.log
- 8: https://packages.debian.org/bookworm/arm64/libavif-bin
- 9: https://packages.debian.org/bookworm/amd64/libavformat59/filelist
🌐 Web query:
Debian Bookworm pngquant package depends libimagequant0 libpng16-16 ldd /usr/bin/pngquant
💡 Result:
In Debian Bookworm, the pngquant utility (version 2.17.0-1) is a dynamically linked executable that relies on shared libraries for its functionality [1][2]. When you run ldd /usr/bin/pngquant, the output typically reveals dependencies on the C library, the math library, the OpenMP library (libgomp), and the specific image-processing libraries you mentioned [3]. Specifically, pngquant depends on libimagequant and libpng [4][5]. Regarding your specific inquiries: 1. Package Dependencies: The Debian pngquant package lists ${shlibs:Depends} in its control file [4][5]. This is a standard Debian mechanism that automatically resolves the necessary shared library dependencies (such as libimagequant0 and libpng16-16) at install time based on the binary's actual requirements [4][6]. 2. ldd /usr/bin/pngquant: Running this command on a standard Debian Bookworm system will show that the binary is linked against libimagequant.so.0 (provided by the libimagequant0 package) and libpng16.so.16 (provided by the libpng16-16 package), along with other standard system libraries like libc.so.6 and libm.so.6 [3]. In summary, libimagequant0 and libpng16-16 are essential runtime dependencies for the version of pngquant found in Debian Bookworm [3]. [1][4][3]
Citations:
- 1: https://packages.debian.org/bookworm/pngquant
- 2: https://packages.debian.org/pngquant
- 3: Pre-compiled are not statically linked imagemin/pngquant-bin#144
- 4: https://tracker.debian.org/media/packages/p/pngquant/control-2.17.0-1
- 5: https://tracker.debian.org/media/packages/p/pngquant/control-2.18.0-1
- 6: https://manpages.debian.org/bookworm/manpages/ldd.1.en.html
Include ffmpeg/transitive image-tool libraries in the base runtime.
COPY --from=tools-builder moves the ffmpeg and pngquant binaries into /usr/local/bin, but the base apt-get install only installs a small subset of related libraries. These copied binaries need their transitive runtime dependencies too (libimagequant0 for pngquant, plus ffmpeg codec/runtime libs not covered by the listed libjpeg/libpng/libav* packages), otherwise direct invocations can fail with shared-library loading errors. Enumerate the needs with ldd inside tools-builder and install the missing runtime packages, or copy/pack the required .so files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 66 - 85, Update the tools-builder stage and base
runtime dependencies for the binaries copied by the Dockerfile, especially
pngquant and ffmpeg. Use ldd on the staged executables to identify their
transitive shared-library requirements, then install the corresponding runtime
packages in the base image (including libimagequant0 and missing ffmpeg
codec/runtime libraries) before the COPY step. Ensure direct invocations of
every copied tool work without shared-library loading errors.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bc824c342
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Add user bin directories to PATH for fzf and zoxide | ||
| ENV PATH=/home/deploy/.local/bin:/home/deploy/.fzf/bin:$PATH | ||
| RUN starship preset no-nerd-font -o "/home/${USER}/.config/starship.toml" \ | ||
| && zsh -i -c 'exit 0' \ |
There was a problem hiding this comment.
When either final stage is built from a clean cache, fast-syntax-highlighting is declared with Zinit's wait (Turbo) ice, so zsh -i -c 'exit 0' exits before the first prompt hook where its installation is scheduled. The immediately following git -C ...fast-syntax-highlighting therefore has no repository to inspect and fails the RUN in both dev and prod; load this plugin synchronously during the build before asserting its commit.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/pipeline.yml (1)
68-76: 🩺 Stability & Availability | 🔵 TrivialRun the multi-architecture Buildx validation before merging.
Local validation was unavailable, so ensure the pull-request workflow completes for both
linux/amd64andlinux/arm64before relying on the dynamically selected versions and tags.Based on learnings, architecture-sensitive changes should be validated with the Buildx matrix or pull-request workflow for both platforms.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pipeline.yml around lines 68 - 76, Validate the pull-request workflow changes using the existing multi-architecture Buildx matrix before merging, ensuring it completes successfully for both linux/amd64 and linux/arm64. Confirm the dynamic minor-version selection and resulting image tags work on each platform.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.zshrc:
- Around line 19-24: Move the `FPATH` export for `$HOME/.eza/completions/zsh`
above the `compinit` call in the completion initialization block, ensuring
`compinit` scans the eza directory during startup while preserving the existing
completion setup.
In `@Dockerfile`:
- Around line 229-230: Update the group creation command in the Dockerfile to
use groupadd’s non-unique option instead of --force, preserving the configured
WWWGROUP GID even when that GID already exists. Keep the existing USER creation
flow unchanged.
In `@README.md`:
- Around line 37-44: Update the README tag table so it does not present
hard-coded PHP 8.4 and 8.5 entries as the complete supported set while CI
dynamically selects the newest two minor lines. Either automate the table
updates as part of the release workflow or explicitly label these entries as
current examples, preserving the documented tag and Debian variant formats.
---
Nitpick comments:
In @.github/workflows/pipeline.yml:
- Around line 68-76: Validate the pull-request workflow changes using the
existing multi-architecture Buildx matrix before merging, ensuring it completes
successfully for both linux/amd64 and linux/arm64. Confirm the dynamic
minor-version selection and resulting image tags work on each platform.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e65294bc-b198-440f-834a-25bde0f76115
📒 Files selected for processing (10)
.env.github/workflows/pipeline.yml.zshrc.zshrc.prodAGENTS.mdDockerfileLICENSEREADME.mddocker-bake.hcltest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
1 participant
Summary
/app, PsySH, pnpm, OpenCode, and shell initializationWhy
The previous multi-stage layout copied dynamically linked tools without their complete ABI-specific runtime closure, fetched mutable artifacts, and left several runtime paths or tools insufficiently validated. The workflow also selected stale PHP lines and could publish colliding major aliases.
Impact
Bookworm and Trixie now resolve their own compatible runtime packages, while pinned external artifacts are checksum-verified for amd64 and arm64. Production images reject Xdebug and development tools, and both variants validate executable behavior, pnpm storage, PsySH, and
/apppermissions. FFmpeg remains installed through APT in the final target stage; no manual shared-library closure is maintained.Validation
actionlint .github/workflows/pipeline.ymlphp -l test.phpzsh -n .zshrc .zshrc.proddocker build --check --build-arg VERSION=8.5.8-trixie .docker buildx bake --printtag and OCI revision assertionsgit diff --checkSummary by CodeRabbit
New Features
php8.5andphp8.5-dev, including tag/workflow updates.Documentation
Bug Fixes
test.phpenvironment checks for versions, writable paths, and allowed/required binaries/extensions..envpnpm store path.