feat: align requirements:check with TYPO3 v13 docs and add EOL checks#18
Conversation
- Update DB minimums (MariaDB 10.4.3, MySQL 8.0.17) - Add PHP minimum version validation (TYPO3: >= 8.2.0) - Update PHP extensions (remove json/soap, add session/tokenizer/filter) - Add pcre.jit setting, extract detectDatabaseProduct() helper
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds image-processing and end-of-life checks, composer version validation, dynamic PHP/version/extension configuration, a requirements listing task, and wires new tasks into the central requirements check; implements helper utilities for package/db detection and EOL API lookups. Changes
Sequence Diagram(s)sequenceDiagram
participant Task as EOL Check Task
participant Remote as Remote Host (ssh)
participant API as endoflife.date API
participant DB as DB Detection
Task->>Remote: run PHP one-liner to get PHP version/cycle
alt PHP version retrieved
Remote-->>Task: PHP cycle string
Task->>API: GET /api/v1/php (fetchEolCycles, timeout)
API-->>Task: cycles array
Task->>Task: findEolCycle & evaluateEolStatus('PHP', cycle, warnMonths)
Task->>Task: addRequirementRow(status, message)
else PHP retrieval failed
Remote-->>Task: error
Task->>Task: addRequirementRow(SKIP/INFO for PHP)
end
Task->>DB: detectDatabaseProduct()
alt DB product detected
DB-->>Task: {product, version, label}
Task->>API: GET /api/v1/{product} (fetchEolCycles, timeout)
API-->>Task: cycles array
Task->>Task: findEolCycle & evaluateEolStatus(label, cycle, warnMonths)
Task->>Task: addRequirementRow(status, message)
else No DB client found
DB-->>Task: null
Task->>Task: addRequirementRow(SKIP/INFO for DB)
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
deployer/requirements/task/list.php (1)
16-30: PHP sub-sections render regardless of which individual flag is enabled.The outer
ifon Line 16 uses an OR, so the block executes when either flag is on. However, theExtensionsline (Line 20) and the settings loop (Lines 24–29) are unconditional inside the block — if onlyrequirements_check_php_settings_enabledis true, the extensions list is still printed, and vice versa. The output would be misleading when one of the two checks is explicitly disabled.♻️ Suggested fix
- if (get('requirements_check_php_settings_enabled') || get('requirements_check_php_extensions_enabled')) { + if (get('requirements_check_php_settings_enabled') || get('requirements_check_php_extensions_enabled')) { writeln(''); writeln('<fg=yellow;options=bold>PHP</>'); writeln(sprintf(' Version: >= %s', get('requirements_php_min_version'))); - writeln(sprintf(' Extensions: %s', implode(', ', get('requirements_php_extensions')))); - - writeln(' Settings:'); - - foreach (get('requirements_php_settings') as $setting => $value) { - $isComparison = in_array($setting, REQUIREMENT_BYTE_SETTINGS, true) - || in_array($setting, REQUIREMENT_NUMERIC_SETTINGS, true); - $operator = $isComparison ? '>=' : '='; - writeln(sprintf(' %-30s %s %s', $setting, $operator, $value)); - } + + if (get('requirements_check_php_extensions_enabled')) { + writeln(sprintf(' Extensions: %s', implode(', ', get('requirements_php_extensions')))); + } + + if (get('requirements_check_php_settings_enabled')) { + writeln(' Settings:'); + + foreach (get('requirements_php_settings') as $setting => $value) { + $isComparison = in_array($setting, REQUIREMENT_BYTE_SETTINGS, true) + || in_array($setting, REQUIREMENT_NUMERIC_SETTINGS, true); + $operator = $isComparison ? '>=' : '='; + writeln(sprintf(' %-30s %s %s', $setting, $operator, $value)); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployer/requirements/task/list.php` around lines 16 - 30, The block keyed by get('requirements_check_php_settings_enabled') || get('requirements_check_php_extensions_enabled') prints both the "Extensions" line and the "Settings" subsection unconditionally; change it so each subsection is rendered only when its respective flag is true: wrap the writeln(sprintf(' Extensions: %s'...)) and its data in a guard checking get('requirements_check_php_extensions_enabled'), and wrap the writeln(' Settings:') plus the foreach over get('requirements_php_settings') (and its use of REQUIREMENT_BYTE_SETTINGS, REQUIREMENT_NUMERIC_SETTINGS and the operator calculation) in a guard checking get('requirements_check_php_settings_enabled'), so headers and content only appear when their checks are enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deployer/requirements/functions.php`:
- Around line 263-282: In evaluateEolStatus, guard against the case where isEol
is false but eolFrom is in the past by checking $eolDate < $now (or
$interval->invert) before computing months; if the EOL date is already before
$now, set $remaining to a past/imminent label (e.g. 'already passed' or
'imminent') instead of computing "in X month(s)" and then call
addRequirementRow("EOL: $label", REQUIREMENT_WARN, "EOL $remaining ($eolFrom)");
otherwise compute months as now and keep the existing behavior using
$warnMonths, $eolDate and $now.
- Around line 175-197: The code in detectDatabaseProduct incorrectly treats
generic "Distrib ..." from a mysql --version as MariaDB; modify the MariaDB
branch so it only applies when the version output actually contains the
"MariaDB" marker (e.g. use a string-presence check on $versionOutput before
running the MariaDB preg_match patterns in the foreach loop), so that the
generic Distrib match doesn't return product 'mariadb' for a pure MySQL binary
and the MySQL-specific preg_match (Ver ...) can run and return product 'mysql'
correctly.
In `@deployer/requirements/task/check_database.php`:
- Around line 12-29: detectDatabaseProduct() misclassifies MySQL 8.x because the
generic "Distrib" regex is evaluated before the "-MariaDB" suffix check; update
detectDatabaseProduct() (in deployer/requirements/functions.php) to test for the
"-MariaDB" suffix (or explicit "MariaDB" token) before the generic /Distrib\s+/
pattern, and ensure the "Distrib" match sets product = 'mysql' (not 'mariadb');
reorder the conditionals so the MariaDB-specific regex runs first and only
fallback to the Distrib pattern for MySQL.
In `@deployer/requirements/task/check_eol.php`:
- Around line 17-18: The $timeout value is not guarded and may be zero or
negative when passed to stream_context_create via fetchEolCycles; clamp it to a
safe positive minimum (e.g. use max(1, (int)
get('requirements_eol_api_timeout'))) so $timeout is always >= 1 before being
used, and keep the same protection pattern used for $warnMonths to avoid
undefined behavior from the HTTP stream wrapper in fetchEolCycles.
In `@deployer/requirements/task/check_image_processing.php`:
- Around line 21-33: The code currently returns unconditionally after detecting
GraphicsMagick output, which prevents falling through to the ImageMagick check
when the GM version is too old; change the logic around $gmOutput/preg_match ->
$actualVersion -> $meets so that you only return from the function when $meets
is true (i.e., GraphicsMagick satisfies the requirement) and when it is false
call addRequirementRow with REQUIREMENT_FAIL but do not return so the
ImageMagick detection block can run; keep the RunException handler for the case
GM is not present.
---
Nitpick comments:
In `@deployer/requirements/task/list.php`:
- Around line 16-30: The block keyed by
get('requirements_check_php_settings_enabled') ||
get('requirements_check_php_extensions_enabled') prints both the "Extensions"
line and the "Settings" subsection unconditionally; change it so each subsection
is rendered only when its respective flag is true: wrap the writeln(sprintf('
Extensions: %s'...)) and its data in a guard checking
get('requirements_check_php_extensions_enabled'), and wrap the writeln('
Settings:') plus the foreach over get('requirements_php_settings') (and its use
of REQUIREMENT_BYTE_SETTINGS, REQUIREMENT_NUMERIC_SETTINGS and the operator
calculation) in a guard checking get('requirements_check_php_settings_enabled'),
so headers and content only appear when their checks are enabled.
|
Re: list.php nitpick — Fixed. Extensions and Settings sub-sections in |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deployer/requirements/functions.php (1)
145-166:detectPackageVersionregex can silently drop single-integer versions.
/(\d+[\d.]+)/requires at least two characters (one\dfrom\d+plus one from[\d.]+), so a bare major-only output like"2"returnsnull. In practice most tools emitmajor.minor, but tools that output only a single integer (e.g., some stripped builds) will be silently treated as "not detected". Consider relaxing the quantifier:- if ($output !== '' && preg_match('/(\d+[\d.]+)/', $output, $matches)) { + if ($output !== '' && preg_match('/(\d+[\d.]*)/', $output, $matches)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployer/requirements/functions.php` around lines 145 - 166, The regex in detectPackageVersion can miss single-integer versions like "2"; update the preg_match inside function detectPackageVersion to accept a sequence of digits optionally followed by dot-separated numeric groups (i.e., match one or more digits possibly followed by one or more ".digits" groups) so both "2" and "2.3.4" are captured, and continue returning matches[1] as before when a match is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deployer/requirements/task/check_image_processing.php`:
- Around line 55-59: The final requirement message incorrectly states "Neither
GraphicsMagick (>= $gmMinVersion) nor ImageMagick (>= $imMinVersion) found" even
when GraphicsMagick was detected but its version is below $gmMinVersion; update
the check that calls addRequirementRow so you track a separate flag (e.g.,
gmDetectedButTooOld) when GM is present but fails the version check, and use
that flag to produce a clearer failure message (e.g., "GraphicsMagick found but
version X < $gmMinVersion; ImageMagick (>= $imMinVersion) not found") instead of
the current combined not-found text; adjust the logic around the GM/IM detection
blocks so addRequirementRow receives the correct, specific message based on
whether GM was absent, present-but-old, or IM absent.
---
Nitpick comments:
In `@deployer/requirements/functions.php`:
- Around line 145-166: The regex in detectPackageVersion can miss single-integer
versions like "2"; update the preg_match inside function detectPackageVersion to
accept a sequence of digits optionally followed by dot-separated numeric groups
(i.e., match one or more digits possibly followed by one or more ".digits"
groups) so both "2" and "2.3.4" are captured, and continue returning matches[1]
as before when a match is found.
|
Addressed remaining review findings:
|
Summary
requirements:checkwith official TYPO3 v13 system requirements documentationrequirements:listcommand for human-readable requirement outputChanges
Core alignment with TYPO3 v13 docs
config/set.php— Update DB minimums (MariaDB 10.4.3, MySQL 8.0.17), add PHP min version validation (>= 8.2.0), update extensions (remove json/soap, add session/tokenizer/filter), add pcre.jit settingcheck_php_settings.php— Validate PHP version against configurable minimum instead of just displaying itcheck_database.php— Refactored to use shareddetectDatabaseProduct()helperImage processing and package versions
check_image_processing.php— New check for GraphicsMagick (>= 1.3) OR ImageMagick (>= 6.0) with version validationcheck_packages.php— Display installed version for all packages, add Composer version validation (>= 2.1.0)EOL checks
check_eol.php— Check PHP and MariaDB/MySQL against endoflife.date API (FAIL=EOL, WARN=approaching/security-only, OK=maintained)functions.php— Add helpers:detectDatabaseProduct(),detectPackageVersion(),fetchEolCycles(),findEolCycle(),evaluateEolStatus(),checkEolForProduct()New command
list.php—dep requirements:listoutputs all requirements in a human-readable format for customers/sysadminsWiring and docs
autoload.php/requirements.php— Register new tasks in autoload and task chaindocs/REQUIREMENTS.md— Updated documentation with all new checks and configuration optionsSummary by CodeRabbit
New Features
Updated Configuration