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

@WironeWirone commented Nov 21, 2024

Copy link
Copy Markdown
Contributor

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

Comment thread.github/workflows/release.yml
Comment threaddocs/usage.md Outdated
Comment threaddocs/usage.md Outdated
Comment thread.github/workflows/release.yml
@asgrimasgrim added the enhancement New feature or request label Nov 21, 2024
Comment threadDockerfile Outdated
Comment threadDockerfile Outdated
Comment threadDockerfile Outdated
Comment thread.github/workflows/release.yml Outdated
Comment thread.github/workflows/release.yml Outdated
Comment thread.github/workflows/release.yml Outdated
Comment threaddocs/usage.md Outdated
Comment threadDockerfile Outdated
@Wirone
Wironeforce-pushed the codito/docker-builds branch 2 times, most recently from 76493bf to 7eb5264CompareNovember 22, 2024 08:36
Comment threadDockerfile
@asgrimasgrim added the maintainer investigating This ticket is being investigated by maintainers. Please check with us before contributing PRs etc! label Nov 22, 2024
Comment threaddocs/usage.md Outdated
Comment threaddocs/usage.md Outdated
@Wirone
Wironeforce-pushed the codito/docker-builds branch from 7eb5264 to 61e4a07CompareNovember 22, 2024 15:38
Comment threaddocs/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 threaddocs/usage.md Outdated
Comment threaddocs/usage.md
Comment threadDockerfile
@Wirone
Wironeforce-pushed the codito/docker-builds branch 4 times, most recently from 777c6d9 to 47de137CompareNovember 29, 2024 10:38
@Wirone

Wirone commented Nov 29, 2024

Copy link
Copy Markdown
ContributorAuthor

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
Wironeforce-pushed the codito/docker-builds branch from 47de137 to e0c258dCompareNovember 29, 2024 10:50
Comment thread.github/workflows/build-phar.yml Outdated
Comment thread.github/workflows/release.yml Outdated
Comment threaddocs/usage.md Outdated
Comment threaddocs/usage.md
@Wirone

Copy link
Copy Markdown
ContributorAuthor

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

@Wirone

Copy link
Copy Markdown
ContributorAuthor

@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.

@TimWollaTimWolla left a comment

Copy link
Copy Markdown
Member

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 threaddocs/usage.md Outdated
Comment threaddocs/usage.md Outdated
Comment thread.github/workflows/release.yml Outdated
Comment thread.github/workflows/release.yml Outdated
@Wirone

Copy link
Copy Markdown
ContributorAuthor

@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 TimWollaDecember 3, 2024 15:33
Comment thread.github/workflows/release.yml Outdated
Comment threadDockerfile
@Wirone
Wirone requested a review from asgrimDecember 4, 2024 08:37
@Wirone
Wironeforce-pushed the codito/docker-builds branch from 460f0c3 to 61ba6a8CompareDecember 4, 2024 08:41

@TimWollaTimWolla left a comment

Copy link
Copy Markdown
Member

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
ContributorAuthor

@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 👍.

@asgrimasgrim left a comment

Copy link
Copy Markdown
Contributor

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!

@glensc

Copy link
Copy Markdown
Contributor

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

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

Labels

enhancementNew feature or requestmaintainer investigatingThis ticket is being investigated by maintainers. Please check with us before contributing PRs etc!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PIE installable in a Dockerfile?

4 participants

@Wirone@glensc@TimWolla@asgrim