Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ jobs:

name: Build binary
uses: ./.github/workflows/reusable-build-binary.yml
with:
use-cache: false

verify:
name: Validate binary on PHP ${{ matrix.php }}
Expand Down
39 changes: 36 additions & 3 deletions .github/workflows/reusable-build-binary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ name: Build Binary

on:
workflow_call:
inputs:
use-cache:
description: "Whether to use the Box PHAR cache"
required: false
type: boolean
default: true

jobs:
bundle:
Expand Down Expand Up @@ -31,11 +37,38 @@ jobs:
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

# Note: do NOT turn on the requirement checker in the box config as it is no longer
# compatible with PHP < 7.2.
- name: Get latest Box version
id: box-version
run: |
latest_url=$(curl -Ls -o /dev/null -w '%{url_effective}' https://github.com/box-project/box/releases/latest)
echo "version=${latest_url##*/}" >> "$GITHUB_OUTPUT"
Comment on lines +40 to +44

@jrfnl jrfnl Sep 3, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I realize this mirrors the current version download of the latest release, but I wonder if we should start to take into account that the latest release may not always be the semantically latest release. This is typically something which would/could happen if a project releases for multiple majors.

Not sure whether that is applicable here at this time, but whether it is or isn't, isn't even that relevant as it could still happen in the future and if it does, we don't want to get bitten by it.

We may also want to consider limiting to the "latest of the current major", as if Box releases a new major, the configuration we use may need updates etc.

This action runner might be helpful to solve the above feedback:
https://github.com/oprypin/find-latest-tag

I've used it successfully in another repo.

Note: if the above suggestions get implemented, the "Install box" step will also need to be updated to download the PHAR based on the version number, rather than on latest.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks again for the detailed review, and sorry for the long silence.

To answer your question about LLMs: yes, I used an AI assistant when writing the original PR, and I should have said so when you asked. I'm also using one for this follow-up: it helped me compare the branch with your review, find the cache evidence in the workflow logs, and draft this comment.

On testing the cache: the run for the latest commit shows it being created earlier and then reused. In the "Build binary / Build binary" job of https://github.com/php-parallel-lint/PHP-Parallel-Lint/actions/runs/33920260753 the "Cache Box PHAR" step logs "Cache hit for: box-phar-4.7.0" and "Cache restored from key: box-phar-4.7.0", the "Install Box" step is skipped, and the save step is skipped because there was nothing new to save. release.yml passes use-cache: false, so the release build never touches the cache.

Still open from your review, and what I plan to do (please tell me if you'd prefer something different):

  1. Bump actions/cache/restore and actions/cache/save from v4 to v6, which also removes the Node 20 warning.
  2. Download Box by the version resolved in the previous step, so the cache key and the downloaded file always match.
  3. On latest not always being the newest release of the current major: Box is on 4.x now. Would you prefer limiting to the latest 4.x with oprypin/find-latest-tag, or with a small gh api call so no extra third-party action is needed?


- name: Cache Box PHAR
id: cache-box
if: inputs.use-cache
uses: actions/cache/restore@v6
with:
path: box.phar
key: box-phar-${{ steps.box-version.outputs.version }}

- name: Install Box
run: wget https://github.com/box-project/box/releases/latest/download/box.phar -O box.phar && chmod 0755 box.phar && pwd
if: ${{ !inputs.use-cache || steps.cache-box.outputs.cache-hit != 'true' }}
env:
BOX_VERSION: ${{ steps.box-version.outputs.version }}
run: wget "https://github.com/box-project/box/releases/download/${BOX_VERSION}/box.phar" -O box.phar

- name: Save Box PHAR cache
if: inputs.use-cache && steps.cache-box.outputs.cache-hit != 'true'
uses: actions/cache/save@v6
with:
path: box.phar
key: box-phar-${{ steps.box-version.outputs.version }}

- name: Set Box permissions
run: chmod 0755 box.phar

# Note: do NOT turn on the requirement checker in the box config as it is no longer
# compatible with PHP < 7.2.
- name: Validate configuration
run: php box.phar validate -i box.json

Expand Down
Loading