Skip to content

Add weekly GitHub action cron job to check for new PHP versions - #4026

Merged
fredrikekelund merged 8 commits into
codex/remove-tsx-from-scriptsfrom
codex/add-php-version-cron-check
Jul 2, 2026
Merged

Add weekly GitHub action cron job to check for new PHP versions#4026
fredrikekelund merged 8 commits into
codex/remove-tsx-from-scriptsfrom
codex/add-php-version-cron-check

Conversation

@fredrikekelund

@fredrikekelund fredrikekelund commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related issues

  • Fixes RSM-4591

How AI was used in this PR

Codex executed the implementation with iterative driving instructions from me.

Proposed Changes

The Build PHP CLI Binaries GitHub Actions workflow gives us a really smooth way of building new PHP binaries for all platforms. Input a PHP version and set apps_cdn_visibility to external, and you get a PR back that modifies the php-binary-cdn-metadata.mjs file. The missing piece is automation that looks for new PHP versions and automatically triggers those builds. This PR adds it as a weekly GitHub Actions cron job.

For added safety, I've changed the error_on_duplicate parameter in the Build PHP CLI Binaries job to true. Overwriting existing appscdn packages would lead to errors in some local Studio CLI installations. We should only do that if we really know what we're doing.

Testing Instructions

  1. Generate a GitHub access token: https://github.com/settings/personal-access-tokens
  2. Run GH_TOKEN=xxxxxx node scripts/check-php-cli-versions.mjs (this will only generate output and not trigger a CI workflow run)
  3. Ensure that the script runs successfully (right now, you'll see that there are new PHP versions available)

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@fredrikekelund
fredrikekelund requested a review from bcotrim July 1, 2026 11:53
@fredrikekelund fredrikekelund self-assigned this Jul 1, 2026
@fredrikekelund
fredrikekelund requested a review from a team as a code owner July 1, 2026 11:53
bcotrim
bcotrim reviewed Jul 1, 2026
View reviewed changes

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

Great improvement, some suggestions:

  1. Skip dispatch if the version’s already on the CDN. If the bump PR sits unmerged, the next cron re-builds all 4 platforms and only fails at upload so the whole build is wasted.
  2. Alert on failure. Let's add a Slack ping if this fails, we already do it on other CI checks, so it should be easy to add. Could be a follow-up if it's not as straightforward as I am assuming.
  3. Expose error_on_duplicate as a dispatch input (default true). Just to make it easier for a manual run to allow an overwrite. Let's say we notice an issue in a nightly build, we could manually trigger an overwrite without any side effects to our users.
@fredrikekelund

fredrikekelund commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Skip dispatch if the version’s already on the CDN

Good idea 👍 Updated.

Alert on failure

Ditto 👍

Expose error_on_duplicate as a dispatch input (default true)

I disagree here. I think we should be super careful about updating existing packages on appscdn, because local Studio installs expect a certain checksum for those packages. If the checksum doesn't match, those Studio installs will stop being able to install those PHP versions.

You and I discussed this a while back and concluded that we should probably get creative about the version numbers we use for PHP packages to mitigate this (i.e., make it so that we are able to rebuild PHP versions that are already on the CDN without breaking existing Studio installs).
@bcotrim

bcotrim commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

You and I discussed this a while back and concluded that we should probably get creative about the version numbers we use for PHP packages to mitigate this (i.e., make it so that we are able to rebuild PHP versions that are already on the CDN without breaking existing Studio installs).

That's fair, I was being optimistic about the runner but you make a very good point, a mistake on an updated version would result in a breaking state for our users. 👍
@fredrikekelund
fredrikekelund merged commit 72c3b06 into codex/remove-tsx-from-scripts Jul 2, 2026
7 checks passed
@fredrikekelund
fredrikekelund deleted the codex/add-php-version-cron-check branch July 2, 2026 05:42
This was referenced Jul 2, 2026
Merged
Merged
fredrikekelund added a commit that referenced this pull request Jul 2, 2026
@fredrikekelund
Revert "Add weekly GitHub action cron job to check for new PHP versio…
b30fb9c 
…ns" (#4051)

Temporarily reverts #4026, as I mistakenly merged that
PR onto a feature branch, while I actually wanted to wait and merge it
to trunk when #4021 has been
merged
@fredrikekelund
fredrikekelund restored the codex/add-php-version-cron-check branch July 2, 2026 10:46
@fredrikekelund fredrikekelund mentioned this pull request Jul 2, 2026
Merged
1 task
fredrikekelund added a commit that referenced this pull request Jul 2, 2026
@fredrikekelund
Include Visual C++ DLLs with Windows PHP builds and add `packageVersi…
e03d037 
…on` field to identify PHP packages (#4042)

## Related issues

<!--
Link a related issue to this PR. If the PR does not immediately resolve
the issue,
for example, it requires a separate deployment to production, avoid
using the "Fixes" keyword and use "Related to" instead.
-->

- Related to STU-1972

## How AI was used in this PR

<!--
Help reviewers understand what to look for and verify that you've
reviewed the code yourself.
-->

Codex drove the implementation based on previous diagnosis.

## Proposed Changes

<!--
Explain the intent of this PR:
- What problem does it solve, or what need does it address?
- How does it affect the user (new capability, fix, performance,
accessibility, etc.)?
- Note any user-visible behavior changes or trade-offs.

Focus on the "why" and the user impact. Avoid listing modified files or
describing implementation mechanics — the diff already shows that.
-->

Sentry shows that some users are running into `PHP command failed (code:
3221225781)` errors. This is because of missing DLLs on their system –
very likely the Visual C++ Redistributable ones, as PHP explicitly
[names that
dependency](https://www.php.net/downloads.php?os=windows#instructions).
This PR updates the native PHP build workflow to include the necessary
DLLs next to `php.exe`.

Because there aren't new PHP patch versions available yet for every
minor PHP version we offer, I also had to find a way to rebuild PHP
versions that already exist on appscdn. This cannot be done today
without breaking local Studio installations, because they expect the
appscdn package to have a predefined checksum. If the checksum of the
appscdn package changes, the Studio client will refuse to install it.

The solution is to add a `packageVersion` field that identifies the
package version separately from the PHP version. The workflow sets this
to `studio-1` by default – meaning a typical workflow run (that builds a
new PHP version) can just use the default. Moreover, I've set
`error_on_duplicate` to always be true (same as in
#4026, which I mistakenly
merged to another branch than trunk)

## Testing Instructions

<!--
Add as many details as possible to help others reproduce the issue and
test the fix.
"Before / After" screenshots can also be very helpful when the change is
visual.
-->

I'll manually verify that these builds do what we want on Windows.

## Pre-merge Checklist

<!--
Complete applicable items on this checklist **before** merging into
trunk. Inapplicable items can be left unchecked.

Both the PR author and reviewer are responsible for ensuring the
checklist is completed.
-->

- [ ] Have you checked for TypeScript, React or other console errors?
bcotrim added a commit that referenced this pull request Jul 14, 2026
@fredrikekelund @bcotrim
2nd take: Add weekly GitHub action cron job to check for new PHP vers…
8dea144 
…ions (#4052)

## Related issues

<!--
Link a related issue to this PR. If the PR does not immediately resolve
the issue,
for example, it requires a separate deployment to production, avoid
using the "Fixes" keyword and use "Related to" instead.
-->

- Fixes RSM-4591

## How AI was used in this PR

<!--
Help reviewers understand what to look for and verify that you've
reviewed the code yourself.
-->

Codex executed the implementation with iterative driving instructions
from me.

## Proposed Changes

<!--
Explain the intent of this PR:
- What problem does it solve, or what need does it address?
- How does it affect the user (new capability, fix, performance,
accessibility, etc.)?
- Note any user-visible behavior changes or trade-offs.

Focus on the "why" and the user impact. Avoid listing modified files or
describing implementation mechanics — the diff already shows that.
-->

See description in #4026. I
mistakenly merged that PR to the base branch
`codex/remove-tsx-from-scripts` before
#4021 has been merged to
`trunk` (which is the workflow I want).

This PR applies the changes from
#4026 again, and it'll be
merged after the base PR.

## Testing Instructions

<!--
Add as many details as possible to help others reproduce the issue and
test the fix.
"Before / After" screenshots can also be very helpful when the change is
visual.
-->

See #4026

## Pre-merge Checklist

<!--
Complete applicable items on this checklist **before** merging into
trunk. Inapplicable items can be left unchecked.

Both the PR author and reviewer are responsible for ensuring the
checklist is completed.
-->

- [ ] Have you checked for TypeScript, React or other console errors?

---------

Co-authored-by: bcotrim <bernardo.cotrim@a8c.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@fredrikekelund @bcotrim