N°9675 - Set up crash - cannot get class from production by odain-cbd · Pull Request #968 · Combodo/iTop

odain-cbd · 2026-07-09T06:40:06Z

No description provided.

Copilot

Pull request overview

This PR addresses issue N°9675, where the setup/CLI process could crash ("cannot get class from production") because CheckCliPhpVersionFromOutput required the CLI PHP version to exactly match the UI PHP major.minor version and threw a CoreException on any mismatch. The fix reworks the CLI PHP check to instead validate the CLI PHP version against the application's minimum-supported version by reusing the existing CheckPhpVersion() logic, which is a much more tolerant and correct criterion.

Changes:

CheckPhpVersion() now accepts optional $sPhpVersion and $sPhpVersionDesc parameters so the same check can be reused for the CLI version with a custom label.
CheckCliPhpVersionFromOutput() dropped the UI-vs-CLI equality comparison and now parses the version from php --version output and runs it through CheckPhpVersion(), throwing only on WARNING/ERROR-level results.
The two CLI-check methods were relocated next to CheckPhpVersion(), and tests were updated/added for the new behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
setup/setuputils.class.inc.php	Reworks CLI PHP version validation to check against the minimum supported version instead of an exact UI match; parameterizes `CheckPhpVersion()`.
tests/php-unit-tests/unitary-tests/setup/SetupUtilsTest.php	Updates and adds tests for the new CLI/PHP version-check behavior.

Notes for the author:

A latent bug remains in the version-parse guard: false === preg_match(...) only catches internal regex errors, so an unparseable output falls through to an undefined $aMatches[0] and ends up checking the UI's phpversion() rather than returning early.
Minor: the CheckPhpVersion docblock omits the new $sPhpVersionDesc parameter and lost its asterisk alignment, and the CheckOKProvider data provider now feeds a "Fail" test, making its name misleading.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+		if (false === preg_match('/(\d+\.\d+)(?:\.\d+)?/', $sFoundVersion, $aMatches)) {
+			return;
+		}


 	/**
-	 * @param CheckResult[] $aResult checks log
-	 *
-	 * @since 3.0.0 N°2214 replace SetupLog::Log calls by CheckResult::TRACE
+	* @param CheckResult[] $aResult checks log
+	* @param string|null $sPhpVersion: if not provided, use current version
+	* @return void
 	 */


-		$this->InvokeNonPublicStaticMethod(SetupUtils::class, 'CheckCliPhpVersionFromOutput', [ModelReflectionSerializer::ERROR_LABEL, '6.6', 'sPHPExec', [$sOuput]]);
-	}
-
 	public function CheckOKProvider()


+	public function testCheckCliPhpVersionFromOutputFail($sOuput, $sFoundVersion = '7.4')
 	{
 		$this->RequireOnceItopFile('/setup/feature_removal/ModelReflectionSerializer.php');
-		$this->InvokeNonPublicStaticMethod(SetupUtils::class, 'CheckCliPhpVersionFromOutput', [ModelReflectionSerializer::ERROR_LABEL, '7.4', 'sPHPExec', [$sOuput]]);
+
+		$this->expectException(\CoreException::class);
+		$sDetails = sprintf('The current CLI PHP Version (%s) is lower than the minimum version required to run %s, which is (%s)', $sFoundVersion, ITOP_APPLICATION, SetupUtils::PHP_MIN_VERSION);
+
+		$this->expectExceptionMessage("Data consistency check failed: $sDetails");
+		$this->InvokeNonPublicStaticMethod(SetupUtils::class, 'CheckCliPhpVersionFromOutput', [ModelReflectionSerializer::ERROR_LABEL, 'sPHPExec', [$sOuput]]);


greptile-apps · 2026-07-09T06:46:20Z

Greptile Summary

This PR changes CheckCliPhpVersionIsOk from verifying that the CLI PHP version matches the web-server PHP version (major.minor) to verifying that the CLI PHP version meets the minimum supported version requirement. CheckPhpVersion is made reusable by accepting an optional version string and description, and CheckCliPhpVersionFromOutput is reworked to call it directly.

CheckPhpVersion gains optional $sPhpVersion / $sPhpVersionDesc parameters so it can be called with a CLI-extracted version instead of always checking phpversion().
CheckCliPhpVersionFromOutput now parses the full semver string from CLI output and delegates to CheckPhpVersion, replacing the old major.minor equality comparison.
Tests are updated and extended to cover the new min-version logic, with new direct tests for CheckPhpVersion.

Confidence Score: 2/5

Two correctness issues in the new CheckCliPhpVersionFromOutput logic make this risky to merge without fixes.

The preg_match guard only exits on a PCRE error but not on no-match, silently falling back to the web PHP version check. The <= WARNING threshold also converts unvalidated-PHP advisory warnings into fatal setup exceptions. Both issues are in the single new function introduced by this PR.

setup/setuputils.class.inc.php — specifically CheckCliPhpVersionFromOutput at lines 525-539.

Important Files Changed

Filename	Overview
setup/setuputils.class.inc.php	Refactors CLI PHP version check from must-match to must-meet-minimum; introduces two bugs: preg_match no-match case not handled, and <= WARNING threshold converts unvalidated-PHP warnings into fatal exceptions.
tests/php-unit-tests/unitary-tests/setup/SetupUtilsTest.php	Renames and extends tests for the new min-version behavior; CheckOKProvider name is now misleading as it feeds a fail test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CheckCliPhpVersionIsOk] --> B[exec php_path --version]
    B --> C{exit code != 0?}
    C -->|yes| D[throw CoreException]
    C -->|no| E[CheckCliPhpVersionFromOutput]
    E --> F[trim aOutput-0]
    F --> G{preg_match version pattern}
    G -->|false PCRE error| H[return silently]
    G -->|0 no match bug| I[aMatches-0 undefined to null]
    G -->|1 match| J[sFoundVersion = aMatches-0]
    I --> K[CheckPhpVersion null uses phpversion]
    J --> L[CheckPhpVersion with CLI version]
    K --> M{iSeverity <= WARNING?}
    L --> M
    M -->|ERROR or WARNING bug| N[throw CoreException]
    M -->|INFO or TRACE| O[return OK]

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[CheckCliPhpVersionIsOk] --> B[exec php_path --version]
    B --> C{exit code != 0?}
    C -->|yes| D[throw CoreException]
    C -->|no| E[CheckCliPhpVersionFromOutput]
    E --> F[trim aOutput-0]
    F --> G{preg_match version pattern}
    G -->|false PCRE error| H[return silently]
    G -->|0 no match bug| I[aMatches-0 undefined to null]
    G -->|1 match| J[sFoundVersion = aMatches-0]
    I --> K[CheckPhpVersion null uses phpversion]
    J --> L[CheckPhpVersion with CLI version]
    K --> M{iSeverity <= WARNING?}
    L --> M
    M -->|ERROR or WARNING bug| N[throw CoreException]
    M -->|INFO or TRACE| O[return OK]

_{Reviews (1): Last reviewed commit: "N°9675 - Set up crash - cannot get class..." | Re-trigger Greptile}

greptile-apps · 2026-07-09T06:46:24Z

+		if (false === preg_match('/(\d+\.\d+)(?:\.\d+)?/', $sFoundVersion, $aMatches)) {
+			return;
+		}


Use 1 !== preg_match(...) so the early-return fires for both "no match" (returns 0) and "PCRE error" (returns false), preventing the silent fall-through to an undefined $aMatches[0].

Suggested change

if (false === preg_match('/(\d+\.\d+)(?:\.\d+)?/', $sFoundVersion, $aMatches)) {

return;

}

if (1 !== preg_match('/(\d+\.\d+)(?:\.\d+)?/', $sFoundVersion, $aMatches)) {

return;

}

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+	private static function CheckCliPhpVersionFromOutput(string $sErrorLabel, string $sPHPExec, $aOutput): void
+	{
+		$sFoundVersion = trim($aOutput[0] ?? "");
+		if (false === preg_match('/(\d+\.\d+)(?:\.\d+)?/', $sFoundVersion, $aMatches)) {


 	 * @dataProvider CheckOKProvider
 	 */
-	public function testCheckCliPhpVersionFromOutputOK($sOuput)
+	public function testCheckCliPhpVersionFromOutputFail($sOutput, $sFoundVersion = '7.4')


+	* @param CheckResult[] $aResult checks log
+	* @param string|null $sPhpVersion: if not provided, use current version
+	* @return void


N°9675 - Set up crash - cannot get class from production

b30096e

Copilot AI review requested due to automatic review settings July 9, 2026 06:40

odain-cbd self-assigned this Jul 9, 2026

CombodoApplicationsAccount added the internal Work made by Combodo label Jul 9, 2026

odain-cbd added the bug Something isn't working label Jul 9, 2026

Copilot started reviewing on behalf of odain-cbd July 9, 2026 06:40 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

N°9675 - add some comments

dd57f69

Copilot AI review requested due to automatic review settings July 9, 2026 06:59

Copilot started reviewing on behalf of odain-cbd July 9, 2026 06:59 View session

odain-cbd merged commit dce1994 into develop Jul 9, 2026
1 check passed

odain-cbd deleted the feature/9675-cliphpcheck branch July 9, 2026 06:59

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

N°9675 - Set up crash - cannot get class from production#968

N°9675 - Set up crash - cannot get class from production#968
odain-cbd merged 2 commits into
developfrom
feature/9675-cliphpcheck

odain-cbd commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 9, 2026

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Jul 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

odain-cbd commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 9, 2026

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants