Skip to content

Commit c11ad23

Browse files
KingPinKingPin
authored andcommitted
fix: address PR review feedback
- Rename hyphenated output keys to underscores (php_versions, s6_version) - Generate full matrix (including trixie includes) in setup job so PR matrix reduction correctly skips PHP 8.3 everywhere - Harden s6-overlay API call with -f flag, auth token, and null check - Remove useless apk cache mount (apk --no-cache bypasses cache dir) - Fix retry.sh comment: backoff is linear, not exponential
1 parent 9a2b160 commit c11ad23

4 files changed

Lines changed: 43 additions & 58 deletions

File tree

.github/workflows/docker-ci.yml

Lines changed: 42 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,56 @@ jobs:
3333
setup:
3434
runs-on: ubuntu-latest
3535
outputs:
36-
php-versions: ${{ steps.matrix.outputs.php-versions }}
37-
s6-version: ${{ steps.s6.outputs.version }}
36+
matrix: ${{ steps.matrix.outputs.matrix }}
37+
s6_version: ${{ steps.s6.outputs.version }}
3838
steps:
39-
- name: Determine PHP versions to test
39+
- name: Compute build matrix
4040
id: matrix
4141
run: |
4242
if [ "${{ github.event_name }}" = "pull_request" ]; then
43-
echo 'php-versions=["8.4","8.2"]' >> $GITHUB_OUTPUT
43+
PHP_VERSIONS='["8.4","8.2"]'
4444
echo "::notice::PR detected — testing PHP 8.4 + 8.2 only (skipping 8.3)"
4545
else
46-
echo 'php-versions=["8.4","8.3","8.2"]' >> $GITHUB_OUTPUT
46+
PHP_VERSIONS='["8.4","8.3","8.2"]'
4747
fi
4848
49+
# Build trixie include list for v2 based on selected PHP versions
50+
INCLUDES="[]"
51+
for ver in $(echo "$PHP_VERSIONS" | jq -r '.[]'); do
52+
for type in fpm cli apache; do
53+
INCLUDES=$(echo "$INCLUDES" | jq -c ". + [{\"variant\":\"v2\",\"php-version\":\"$ver\",\"php-type\":\"$type\",\"php-base\":\"trixie\"}]")
54+
done
55+
done
56+
57+
MATRIX=$(jq -n -c \
58+
--argjson versions "$PHP_VERSIONS" \
59+
--argjson includes "$INCLUDES" \
60+
'{
61+
"variant": ["v1","v2"],
62+
"php-version": $versions,
63+
"php-type": ["fpm","cli","apache"],
64+
"php-base": ["alpine","bookworm"],
65+
"exclude": [
66+
{"php-type":"apache","php-base":"alpine"},
67+
{"variant":"v2","php-base":"bookworm"}
68+
],
69+
"include": $includes
70+
}')
71+
72+
echo "matrix=$MATRIX" >> $GITHUB_OUTPUT
73+
4974
- name: Get latest s6-overlay version
5075
id: s6
5176
run: |
52-
S6_OVERLAY_VERSION="$(curl -s https://api.github.com/repos/just-containers/s6-overlay/releases/latest | jq -r .tag_name)"
77+
set -euo pipefail
78+
RESPONSE="$(curl -fSLs \
79+
-H "Authorization: Bearer ${{ github.token }}" \
80+
https://api.github.com/repos/just-containers/s6-overlay/releases/latest)"
81+
S6_OVERLAY_VERSION="$(echo "$RESPONSE" | jq -r .tag_name)"
82+
if [ -z "$S6_OVERLAY_VERSION" ] || [ "$S6_OVERLAY_VERSION" = "null" ]; then
83+
echo "::error::Failed to determine s6-overlay version"
84+
exit 1
85+
fi
5386
echo "version=${S6_OVERLAY_VERSION}" >> $GITHUB_OUTPUT
5487
echo "✅ Latest s6-overlay version: ${S6_OVERLAY_VERSION}"
5588
@@ -58,53 +91,7 @@ jobs:
5891
runs-on: ubuntu-latest
5992
strategy:
6093
fail-fast: false
61-
matrix:
62-
variant: [v1, v2]
63-
php-version: ${{ fromJson(needs.setup.outputs.php-versions) }}
64-
php-type: [fpm, cli, apache]
65-
php-base: [alpine, bookworm]
66-
exclude:
67-
- php-type: apache
68-
php-base: alpine
69-
- variant: v2
70-
php-base: bookworm
71-
include:
72-
- variant: v2
73-
php-version: '8.4'
74-
php-type: fpm
75-
php-base: trixie
76-
- variant: v2
77-
php-version: '8.4'
78-
php-type: cli
79-
php-base: trixie
80-
- variant: v2
81-
php-version: '8.4'
82-
php-type: apache
83-
php-base: trixie
84-
- variant: v2
85-
php-version: '8.3'
86-
php-type: fpm
87-
php-base: trixie
88-
- variant: v2
89-
php-version: '8.3'
90-
php-type: cli
91-
php-base: trixie
92-
- variant: v2
93-
php-version: '8.3'
94-
php-type: apache
95-
php-base: trixie
96-
- variant: v2
97-
php-version: '8.2'
98-
php-type: fpm
99-
php-base: trixie
100-
- variant: v2
101-
php-version: '8.2'
102-
php-type: cli
103-
php-base: trixie
104-
- variant: v2
105-
php-version: '8.2'
106-
php-type: apache
107-
php-base: trixie
94+
matrix: ${{ fromJson(needs.setup.outputs.matrix) }}
10895

10996
name: ${{ matrix.variant }}-${{ matrix.php-version }}-${{ matrix.php-type }}-${{ matrix.php-base }}
11097

@@ -150,7 +137,7 @@ jobs:
150137
VERSION=${{ steps.vars.outputs.VERSION }}
151138
PHPVERSION=${{ matrix.php-version }}
152139
BASEOS=${{ matrix.php-base }}
153-
S6_OVERLAY_VERSION=${{ needs.setup.outputs.s6-version }}
140+
S6_OVERLAY_VERSION=${{ needs.setup.outputs.s6_version }}
154141
BUILD_DATE=${{ steps.vars.outputs.BUILD_DATE }}
155142
VCS_REF=${{ github.sha }}
156143
tags: test-${{ steps.vars.outputs.TAG }}
@@ -434,7 +421,7 @@ jobs:
434421
VERSION=${{ steps.vars.outputs.VERSION }}
435422
PHPVERSION=${{ matrix.php-version }}
436423
BASEOS=${{ matrix.php-base }}
437-
S6_OVERLAY_VERSION=${{ needs.setup.outputs.s6-version }}
424+
S6_OVERLAY_VERSION=${{ needs.setup.outputs.s6_version }}
438425
BUILD_DATE=${{ steps.vars.outputs.BUILD_DATE }}
439426
VCS_REF=${{ github.sha }}
440427
tags: |

Dockerfile.v1

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ RUN chmod +x /usr/local/bin/retry
1212
# Install dependencies based on the base OS
1313
RUN --mount=type=cache,target=/var/cache/apt,sharing=locked,id=apt-$BASEOS \
1414
--mount=type=cache,target=/var/lib/apt/lists,sharing=locked,id=aptlists-$BASEOS \
15-
--mount=type=cache,target=/var/cache/apk,sharing=locked,id=apk-$BASEOS \
1615
if [ "$BASEOS" = "bookworm" ]; then \
1716
echo 'deb http://deb.debian.org/debian bookworm main' > /etc/apt/sources.list && \
1817
apt-get update && \

Dockerfile.v2

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ LABEL org.opencontainers.image.title="php-docker" \
3131
# Then clean up build-only packages in a single layer to minimize image size
3232
RUN --mount=type=cache,target=/var/cache/apt,sharing=locked,id=apt-$BASEOS \
3333
--mount=type=cache,target=/var/lib/apt/lists,sharing=locked,id=aptlists-$BASEOS \
34-
--mount=type=cache,target=/var/cache/apk,sharing=locked,id=apk-$BASEOS \
3534
if [ "$BASEOS" = "trixie" ] || [ "$BASEOS" = "bookworm" ]; then \
3635
apt-get update && \
3736
apt-get -y upgrade && \

extras/retry.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/sh
2-
# Retry a command with exponential backoff
2+
# Retry a command with linear backoff
33
# Usage: retry <max_attempts> <command...>
44
# Example: retry 3 curl -sSLf -o /tmp/file https://example.com/file
55

0 commit comments

Comments
 (0)