Skip to content

chore: modernize PHP 8 runtime support#792

Open
anonymoususer72041 wants to merge 18 commits into
opencats:masterfrom
anonymoususer72041:chore/upgrade-php-8.5
Open

chore: modernize PHP 8 runtime support#792
anonymoususer72041 wants to merge 18 commits into
opencats:masterfrom
anonymoususer72041:chore/upgrade-php-8.5

Conversation

@anonymoususer72041

@anonymoususer72041 anonymoususer72041 commented May 30, 2026

Copy link
Copy Markdown
Contributor

This updates OpenCATS to require PHP 8.4.1 and adds PHP 8.5 support, aligning the Composer requirements, CI runtime, installer checks and Docker PHP image with that requirement.

The Composer dependencies are refreshed for modern PHP 8.x and the mbstring extension is declared explicitly because it is required by the replacement for deprecated encoding helpers. The CI setup now also declares the PHP extensions needed by the application and test dependencies instead of relying on runner defaults.

The application code is updated to avoid deprecated PHP APIs by replacing strftime() usage, removing utf8_encode() calls, adding explicit casts where PHP 8.5 no longer accepts nullable values implicitly and updating legacy runtime patterns such as old implode() argument order, callable syntax and dynamic properties.

Legacy PHP compatibility code that is no longer needed with the new minimum PHP version is removed while preserving the session/login behavior required by the test suite.

The Docker image now uses php:8.5-fpm-alpine. The unrtf build step sets compatibility CFLAGS because unrtf 0.21.10 uses legacy C constructs that fail with newer Alpine/GCC defaults.

This also updates the custom BrowserKit client signature for the Symfony version pulled in by the refreshed dependencies and stabilizes the Behat company autocomplete wait so the tests continue to exercise the real UI suggestion element.

@anonymoususer72041 anonymoususer72041 added this to the 1.0.0 milestone May 30, 2026
@anonymoususer72041 anonymoususer72041 requested a review from RussH May 30, 2026 15:21
@anonymoususer72041 anonymoususer72041 marked this pull request as draft May 31, 2026 09:49
@anonymoususer72041 anonymoususer72041 removed the request for review from RussH May 31, 2026 09:49
@anonymoususer72041 anonymoususer72041 requested a review from RussH June 1, 2026 13:19
@anonymoususer72041 anonymoususer72041 marked this pull request as ready for review June 1, 2026 13:19
@anonymoususer72041 anonymoususer72041 force-pushed the chore/upgrade-php-8.5 branch 2 times, most recently from e01143d to ff8a804 Compare June 4, 2026 12:37
@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

Please merge #797 first

@ocjorge

ocjorge commented Jun 17, 2026

Copy link
Copy Markdown

Thank you for this comprehensive PHP 8.5 upgrade. I closed my PR #809
since this covers the same ground more thoroughly and is already part
of the v1.0.0 roadmap.

I noticed two fixes from #809 that might not be covered here:

  1. CareerPortal.php: Wrapping PHPMailer send() in try/catch with
    error_log() — without this, the Career Portal returns a fatal 500
    error when no mail server is configured, which breaks the candidate
    application flow entirely.

  2. AJAXInterface.php: Guarding session_start() with
    session_status() === PHP_SESSION_NONE — prevents duplicate session
    warnings in AJAX calls.

Both were tested in production on PHP 8.4.21. Happy to open targeted PRs
for these specific fixes if they are not already covered in this PR or
in #797. Just let me know what would be most useful.

RussH
RussH previously approved these changes Jun 24, 2026

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

A big one!

Just to confirm - the intention is to move CI to PHP 8.5 only?

Looks as good as it's going to be, certianly ticks all the boxes, no choice but to fix forwards!

@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

Just to confirm - the intention is to move CI to PHP 8.5 only?

Good point. I think the cleaner approach is to avoid making this PR implicitly decide that CI should be PHP 8.5-only.

I can create a preceding PR that introduces PHP matrix testing first. Based on the current dependency constraints, the lowest practical PHP version appears to be 8.4.1, mainly because PHPUnit 13 requires PHP >=8.4.1.

Once that is in place, this PR can build on the matrix and test the relevant supported versions instead of only PHP 8.5.

@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

I am moving this back to draft for now.

This PR should no longer be merged immediately after #797. With #816 now opened as a preparation PR for PHP matrix testing, I think the cleaner order is to merge #797 first, then #816 and then update this PR on top of both.

That should avoid making this PR implicitly decide that CI only targets PHP 8.5 and it should let this PR use the matrix setup once the supported PHP version range is adjusted here.

P.S. I also created a v0.11.0 milestone and manually ordered the related PRs there so the intended merge order is visible. The items at the top are intended to be merged first.

@anonymoususer72041 anonymoususer72041 marked this pull request as draft June 24, 2026 09:52
@RussH

RussH commented Jul 3, 2026

Copy link
Copy Markdown
Member

@anonymoususer72041 can you update this one?

@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

@RussH please take a look at my last comment:

This PR should no longer be merged immediately after #797. With #816 now opened as a preparation PR for PHP matrix testing, I think the cleaner order is to merge #797 first, then #816 and then update this PR on top of both.

I will update this PR as soon as #797 and #816 got merged.

@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

Just saw #797 and #816 already got approved, so even if it was not quite the intended order, I squashed them into master so I can continue my work on this PR.

@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

Thank you for this comprehensive PHP 8.5 upgrade. I closed my PR #809 since this covers the same ground more thoroughly and is already part of the v1.0.0 roadmap.

I noticed two fixes from #809 that might not be covered here:

1. **CareerPortal.php**: Wrapping PHPMailer `send()` in try/catch with
   `error_log()` — without this, the Career Portal returns a fatal 500
   error when no mail server is configured, which breaks the candidate
   application flow entirely.

2. **AJAXInterface.php**: Guarding `session_start()` with
   `session_status() === PHP_SESSION_NONE` — prevents duplicate session
   warnings in AJAX calls.

Both were tested in production on PHP 8.4.21. Happy to open targeted PRs for these specific fixes if they are not already covered in this PR or in #797. Just let me know what would be most useful.

Thanks for spotting these, @ocjorge.

The AJAXInterface session guard is already covered in this PHP PR, although the implementation is slightly different from #809. #797 does not cover that part.

The CareerPortal mailer try/catch is not covered here or in #797. I agree it is a useful fix, especially because it prevents candidate applications from failing when mail is not configured. Since it is a focused production bug fix rather than directly part of the PHP upgrade work, a small targeted PR for that one would probably be easiest to review.

So: AJAXInterface is covered here, CareerPortal would be great as a separate targeted PR.

@anonymoususer72041 anonymoususer72041 marked this pull request as ready for review July 3, 2026 11:38
@anonymoususer72041 anonymoususer72041 changed the title chore: upgrade to PHP 8.5 chore: modernize PHP 8 runtime support Jul 3, 2026
@anonymoususer72041 anonymoususer72041 requested a review from RussH July 3, 2026 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants