Skip to content

feat: CI job for building Docker binary-only images#122

Merged
asgrim merged 9 commits into
php:mainfrom
Wirone:codito/docker-builds
Dec 4, 2024
Merged

feat: CI job for building Docker binary-only images#122
asgrim merged 9 commits into
php:mainfrom
Wirone:codito/docker-builds

Conversation

@Wirone
Copy link
Copy Markdown
Contributor

@Wirone Wirone commented Nov 21, 2024 •
edited
Loading

Similar to composer/docker#250 (more info), this PR introduces a CI process for releasing binary-only images for Pie 🥳!

You can see the result here, there is an image in my fork's Package Registry, built here.

Here's how the image looks like (dive ghcr.io/wirone/php-pie:latest-bin):
image

PS: I decided to not verify GPG signature during build because we can trust our own workflow (we copy PHAR file built in previous stage), but also I had some problems with verification (GPG imported correctly, but verification failed). Users may verify the file on their own, if they want, using attached pie.asc. Signing was dropped.

Fixes #137

asgrim
asgrim reviewed Nov 21, 2024
View reviewed changes
Comment thread .github/workflows/release.yml
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
Comment thread .github/workflows/release.yml
@asgrim asgrim added the enhancement New feature or request label Nov 21, 2024
TimWolla
TimWolla requested changes Nov 21, 2024
View reviewed changes
Comment thread Dockerfile Outdated
Comment thread Dockerfile Outdated
Comment thread Dockerfile Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread docs/usage.md Outdated
asgrim
asgrim reviewed Nov 22, 2024
View reviewed changes
Comment thread Dockerfile Outdated
@Wirone Wirone force-pushed the codito/docker-builds branch 2 times, most recently from 76493bf to 7eb5264 Compare November 22, 2024 08:36
Wirone
Wirone commented Nov 22, 2024
View reviewed changes
Comment thread Dockerfile
@Wirone Wirone requested review from TimWolla and asgrim November 22, 2024 08:47
@asgrim asgrim added the maintainer investigating This ticket is being investigated by maintainers. Please check with us before contributing PRs etc! label Nov 22, 2024
TimWolla
TimWolla reviewed Nov 22, 2024
View reviewed changes
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
@Wirone Wirone force-pushed the codito/docker-builds branch from 7eb5264 to 61e4a07 Compare November 22, 2024 15:38
@asgrim asgrim mentioned this pull request Nov 25, 2024
Closed
asgrim
asgrim requested changes Nov 29, 2024
View reviewed changes
Comment thread docs/usage.md Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md
asgrim
asgrim reviewed Nov 29, 2024
View reviewed changes
Comment thread Dockerfile
@Wirone Wirone force-pushed the codito/docker-builds branch 4 times, most recently from 777c6d9 to 47de137 Compare November 29, 2024 10:38
@Wirone
Copy link
Copy Markdown
Contributor Author

Wirone commented Nov 29, 2024 •
edited
Loading

Rebased interactively with squashing into easier to grasp commits (workflow + usage), some changes enforced by changes in the upstream were applied during the rebase. The effect of current state of the PR can be seen here and here. At this point I didn't address attestation.

@Wirone Wirone force-pushed the codito/docker-builds branch from 47de137 to e0c258d Compare November 29, 2024 10:50
asgrim
asgrim reviewed Nov 29, 2024
View reviewed changes
Comment thread .github/workflows/build-phar.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md
@Wirone
Copy link
Copy Markdown
Contributor Author

Wirone commented Nov 29, 2024

@asgrim Docker build attestation from latest build can be found here.

@Wirone Wirone requested review from TimWolla and asgrim November 29, 2024 11:10
@Wirone
Copy link
Copy Markdown
Contributor Author

Wirone commented Dec 3, 2024

@asgrim Is there any actionable thing on my side? I believe this can wait for non-labs release, as this is only a cosmetic change.

TimWolla
TimWolla reviewed Dec 3, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

I'm still concerned about https://github.com/php/pie/pull/122/files#r1854054535, given that we only make the Phar available as “latest” and as a specific version, not by any “major” / “minor” wildcards.

The workflow steps look fine to me, except for the few remarks I've made.

Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread docs/usage.md Outdated
Comment thread docs/usage.md Outdated
TimWolla
TimWolla reviewed Dec 3, 2024
View reviewed changes
Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
@Wirone
Copy link
Copy Markdown
Contributor Author

Wirone commented Dec 3, 2024

@TimWolla @asgrim tested new release, images have been built correctly, you can see them in the registry. dive inspection:

image

@Wirone Wirone requested a review from TimWolla December 3, 2024 15:33
asgrim
asgrim reviewed Dec 4, 2024
View reviewed changes
Comment thread .github/workflows/release.yml Outdated
Comment thread Dockerfile
@Wirone Wirone requested a review from asgrim December 4, 2024 08:37
@Wirone Wirone force-pushed the codito/docker-builds branch from 460f0c3 to 61ba6a8 Compare December 4, 2024 08:41
TimWolla
TimWolla approved these changes Dec 4, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

LGTM now. Thanks for your patience with my reviews 😄

@Wirone
Copy link
Copy Markdown
Contributor Author

Wirone commented Dec 4, 2024

@TimWolla after @keradus' reviews nothing seems to be annoying 😂. But seriously, thanks for the great feedback, I've learned new things during the process so for me this was added value. I like good code reviews 👍.

asgrim
asgrim approved these changes Dec 4, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@asgrim asgrim left a comment

Choose a reason for hiding this comment

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

Thank you @Wirone for your patience, this LGTM now 👍 got there in the end 😁 nice work, and thanks @TimWolla for your great feedback too!

@asgrim asgrim self-assigned this Dec 4, 2024
@asgrim asgrim added this to the 0.3.0 milestone Dec 4, 2024
@asgrim asgrim merged commit 8cfcff9 into php:main Dec 4, 2024
@Wirone Wirone deleted the codito/docker-builds branch December 4, 2024 22:28
This was referenced Jan 2, 2025
Closed
Draft
glensc added a commit to glensc/pie that referenced this pull request May 13, 2026
@glensc
Add granular publish tags for "-bin" docker image
fdf8b91 
Implement TODO:
- https://github.com/php/pie/blob/5c3fa89c5f63728f60fda04756b4044c01855ff7/.github/workflows/release.yml#L90-L96

See:
- php#122 (review)
- php#122 (comment)
@glensc glensc mentioned this pull request May 13, 2026
Open
4 tasks
@glensc
Copy link
Copy Markdown

glensc commented May 13, 2026

1.x released, so adding the versioned "-bin" tags make sense now:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimWolla TimWolla TimWolla approved these changes

@asgrim asgrim asgrim approved these changes

Assignees

@asgrim asgrim

Labels

enhancement New feature or request maintainer investigating This ticket is being investigated by maintainers. Please check with us before contributing PRs etc!

Projects

None yet

Milestone

0.3.0

Development

Successfully merging this pull request may close these issues.

PIE installable in a Dockerfile?

4 participants

@Wirone @glensc @TimWolla @asgrim