Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Dec 4, 2024

Conversation

Wirone
Copy link
Contributor

@Wirone Wirone commented Nov 21, 2024

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

.github/workflows/release.yml Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
@asgrim asgrim added the enhancement New feature or request label Nov 21, 2024
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@Wirone Wirone force-pushed the codito/docker-builds branch 2 times, most recently from 76493bf to 7eb5264 Compare November 22, 2024 08:36
Dockerfile Show resolved Hide resolved
@Wirone Wirone requested review from asgrim and TimWolla November 22, 2024 08:47
@asgrim asgrim added the maintainer feedback needed Needs details or feedback to be added by maintainers label Nov 22, 2024
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@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
Contributor Author

Wirone commented Nov 29, 2024

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
.github/workflows/build-phar.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Show resolved Hide resolved
@Wirone
Copy link
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
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.

Copy link
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.

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
@Wirone
Copy link
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
.github/workflows/release.yml Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@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
Copy link
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
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 👍.

Copy link
Collaborator

@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
19 checks passed
@Wirone Wirone deleted the codito/docker-builds branch December 4, 2024 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request maintainer feedback needed Needs details or feedback to be added by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PIE installable in a Dockerfile?
3 participants