Conversation
Build all 7 PHP FPM images concurrently instead of sequentially, reducing total build time from 7×T to ~T (the longest single build). Each build's output is prefixed with [phpX.Y] for readability. If any build fails, remaining builds are killed immediately.
5b51d07 to
175ccef
Compare
Build all 7 PHP FPM images concurrently instead of sequentially, reducing total build time from 7xT to ~T (the longest single build). Each build's output is prefixed with [phpX.Y] for readability. Results are collected via a FIFO in completion order: on first failure, all remaining builds are killed immediately (podman children first, then bash wrappers) and the script exits.
There was a problem hiding this comment.
Pull request overview
Parallelizes building the suite of PHP-FPM container images in build-images.sh to reduce total build time while keeping interleaved build logs readable.
Changes:
- Replaces sequential PHP-FPM builds with parallel background builds for a defined set of PHP versions.
- Prefixes each build’s output with
[phpX.Y]for readability. - Adds early-failure handling via traps to terminate remaining builds and clean up resources.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use --cache-from to reuse published layers from ghcr.io registry, avoiding full recompilation when base image and Containerfile are unchanged. Remove apt upgrade: base image updates are handled by Renovate bot, running upgrade on every build breaks layer caching unnecessarily.
- Replace mktemp -u with mktemp -d to avoid TOCTOU race on FIFO creation
- Close both sides of FD 3 (exec 3<&-; exec 3>&-) in EXIT trap
- Replace subshell EXIT traps with monitor processes using wait <pid>,
which reliably reports exit code even when build is killed with SIGKILL
- Add wait "${pids[@]}" after completion loop to reap zombie processes
Monitor subshells using wait <pid> cannot wait on sibling processes in bash. Revert to EXIT trap approach which correctly handles normal exit, SIGTERM and Ctrl+C. SIGKILL bypassing the trap is an acceptable limitation for a build script.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… process groups Replace EXIT trap in build subshells with a wrapper+monitor pattern: each build runs in a nested subshell, and the wrapper calls wait <build_pid> to capture its exit code — works even under SIGKILL. Capture PIPESTATUS[0] explicitly so sed log-prefix failures cannot mark a successful podman build as failed. Enable set -m so each background wrapper runs in its own process group; kill_all_builds uses kill -- -pid to terminate the entire tree (wrapper + nested subshell + podman + sed) on first failure or Ctrl+C.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| kill_all_builds() { | ||
| for pid in "${pids[@]}"; do | ||
| # With set -m each wrapper runs in its own process group (PGID=pid); | ||
| # kill -- -pid sends SIGTERM to the whole group (wrapper + nested subshell + podman + sed) | ||
| kill -- -"${pid}" 2>/dev/null || true | ||
| done |
There was a problem hiding this comment.
PR description mentions killing remaining builds via pkill -P (podman children) and then the bash wrappers, but the implementation here only uses kill -- -<pid> (process-group SIGTERM). Please either update the PR description to match the actual mechanism, or add the documented pkill -P/child-kill behavior if it’s required for correctness in your environment.
Summary
7×Tto~T(the duration of the longest single build)[phpX.Y]to remain readable in the terminal when outputs are interleavedmktemp -d) is used to create the FIFO, avoiding the TOCTOU race ofmktemp -uwait <pid>— this survives SIGKILL (unlike EXIT traps) and reports the exit code reliablypkill -Pfor podman children, then the bash wrappers) and the script exitsCtrl+CandSIGTERMare handled via traps, cleanly stopping all builds and removing the temp dir--cache-fromreuses published layers from ghcr.io registry, avoiding full recompilation when the base image andContainerfileare unchangedapt upgraderemoved fromContainerfile: base image updates are managed by Renovate bot, running upgrade on every build breaks layer caching unnecessarily