Skip to content

[2.x] ci: rebuild the backend matrix around declared database support - #4959

Merged
imorland merged 3 commits into
2.xfrom
im/ci-db-matrix-revamp
Aug 22, 2026
Merged

[2.x] ci: rebuild the backend matrix around declared database support#4959
imorland merged 3 commits into
2.xfrom
im/ci-db-matrix-revamp

Conversation

@imorland

@imorland imorland commented Aug 21, 2026

Copy link
Copy Markdown
Member

Changes proposed in this pull request:

The backend matrix had drifted away from what Flarum declares it supports. DatabaseRequirements names a MINIMUM and RECOMMENDED version per driver, and CI tested neither for most of them:

driver MINIMUM RECOMMENDED CI tested before
MySQL 5.7.8 8.4.0 5.7, 8.0.30 — 8.0 reached EOL 2026-04-30
MariaDB 10.3.0 11.8.0 mariadb — unpinned, so :latest
PostgreSQL 10.0.0 15.0 postgres:10
SQLite 3.35.0 whatever the runner ships

So the primary MySQL was a version that had gone EOL, MariaDB's tested version drifted silently with Docker Hub, and none of the recommended versions were exercised at all — we were warning admins to be on versions we never tested.

The matrix now covers, per driver, the declared minimum, the declared recommendation, and the newest long-term release: MySQL 5.7 / 8.4 / 9.7, MariaDB 10.3 / 11.8 / 12.3, PostgreSQL 10 / 15 / 18, plus SQLite. Every image is pinned so a run is reproducible.

The exclude block is gone. It carried a warning that it had to be kept in sync by hand with the size of php_versions, and it grew as the product of drivers and PHP versions. The base matrix is now latest_php × db_versions, with older PHP versions and the prefixed runs added as explicit include entries. Adding a database is one line; adding a PHP version no longer needs an entry per database. db_versions keeps its existing meaning, so a repo passing a custom list still gets exactly that list.

That produces 14 jobs, against 11 before:

PHP 8.5 / MySQL 5.7          PHP 8.5 / MariaDB 10.3   PHP 8.5 / PostgreSQL 10
PHP 8.5 / MySQL 8.4          PHP 8.5 / MariaDB 11.8   PHP 8.5 / PostgreSQL 15
PHP 8.5 / MySQL 9.7          PHP 8.5 / MariaDB 12.3   PHP 8.5 / PostgreSQL 18
PHP 8.5 / MySQL 9.7 (prefix)                          PHP 8.5 / PostgreSQL 18 (prefix)
PHP 8.3 / MySQL 8.4                                   PHP 8.5 / SQLite
PHP 8.4 / MySQL 8.4

PHP coverage is deliberately not crossed with every database. A PHP-version bug is not specific to a database engine, so those combinations could only fail alongside another job. The three PHP versions instead run against the recommended MySQL, which also moves that axis off the EOL 8.0 it was pinned to.

Prefixed runs are now aimed at what they are for. They exist to catch identifier length limits, but the prefix was flarum_ — seven characters, which will not reach a limit. It is now 19, and applied to the newest MySQL and PostgreSQL rather than to four engines, since those are where the limits bite: 64 characters on MySQL/MariaDB, 63 bytes on PostgreSQL, which truncates silently instead of erroring. That is also why PostgreSQL is the interesting case — a collision after truncation produces no error at all.

Two pre-existing bugs fixed along the way.

ini-values read ${{ matrix.php_ini_values }}, and that matrix key was set to [inputs.php_ini_values] — a literal string rather than an expression. The last run on 2.x confirms what setup-php actually received:

php_ini_values: error_reporting=E_ALL     <- the input
ini-values: inputs.php_ini_values         <- what setup-php got

So error_reporting=E_ALL has never been applied, and notice and deprecation level diagnostics have been suppressed relative to intent across the whole suite. Now taken from the input directly.

The mariadb service was also unpinned, so that job silently followed :latest. All images are pinned now.

Capacity. Concurrent job limits are account-wide, and this account's limit is 20 on the Free plan. A push currently requests 15 jobs (11 backend, 3 static analysis, 1 frontend); this takes that to 18. To make room, all three entry workflows gain a concurrency group so superseded runs are cancelled rather than holding slots until they finish — cancel-in-progress is disabled on 2.x so each merge commit still gets its own result. fail-fast: false is also set, so one database failing no longer cancels the rest and costs us the run.

SQLite cannot be pinned, since it is compiled into PHP rather than run as a service. There is now a step that reports the runner's version and fails if it ever drops below SQLITE_MINIMUM, so at least we know what was tested.

Reviewers should focus on:

  • What this run actually reports, which is the point of the draft. Three things could plausibly go red for reasons unrelated to the restructuring: the E_ALL fix surfacing previously suppressed diagnostics; the longer prefix reaching PostgreSQL's 63-byte limit; and the three EOL databases that are now explicit rather than incidental. Each of those is information rather than a fault in this change — if the EOL versions fail, that is an argument for raising the declared minimums.
  • The prefix length. 19 characters is a guess at "longer than realistic but not absurd". If it fails, the useful outcome is deciding what maximum prefix length Flarum supports, which is not documented anywhere today.
  • One wart in the include entries. They name mysql:8.4, mysql:9.7 and postgres:18 explicitly, so a repo passing a db_versions list without those still gets those four jobs. Harmless but surprising. Fixing it properly needs a job that computes the matrix before the tests run, which felt like more machinery than this is worth.

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered? — a scheduled workflow for PHP pre-releases and the older LTS lines was considered and deliberately left out of this change; so was replacing db_versions with a richer input, which would have broken the ability of extension repos to specify their own database set.
  • For core PRs, does this need to be in core, or could it be in an extension? — this is the shared workflow every extension consumes.
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation — no frontend changes.
  • Frontend changes: tests are green — no frontend changes.
  • Frontend changes: tests have been added — no frontend changes.
  • Backend changes: tests are green — that is what this draft is for.
  • Backend changes: tests have been added — no test changes; this is CI configuration.
  • Where applicable, changes are suitable for all supported database drivers — that is the substance of the change.
  • Core developer confirmed locally this works as intended.
  • The description above is written by me and describes what this pull request actually does.

@imorland
imorland force-pushed the im/ci-db-matrix-revamp branch from 6362185 to ca07559 Compare August 22, 2026 00:10
@imorland
imorland marked this pull request as ready for review August 22, 2026 07:33
@imorland
imorland requested a review from a team as a code owner August 22, 2026 07:33
@imorland imorland added this to the 2.0.0-rc.6 milestone Aug 22, 2026
@imorland
imorland merged commit 798d0c0 into 2.x Aug 22, 2026
30 checks passed
@imorland
imorland deleted the im/ci-db-matrix-revamp branch August 22, 2026 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant