Skip to content

Fix compatibility of minimum PHP version check with older versions #20358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 16, 2023

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented Feb 14, 2023

Description:

Fixes #20350

The testMinimumPhpVersion.php check contains type hints which are not compatible with PHP < 7. When attempting to run command line tasks with PHP 5.6 this caused the following error to be shown instead of an incompatible version warning:

PHP Fatal error: Default value for parameters with a class type hint can only be NULL in /var/www/matomo/core/testMinimumPhpVersion.php on line 128

Although the minimum PHP version for Matomo is 7.2+, this specific file needs to be able to run on any PHP version > 4 so it can show the version error.

This PR removes the type hints.

Review

@bx80 bx80 added Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Feb 14, 2023
@bx80 bx80 added this to the 4.13.4 milestone Feb 14, 2023
@bx80 bx80 self-assigned this Feb 14, 2023
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bx80 The file still doesn't seem to be compatible with older PHP versions. There is a usage of a short array syntax on line 215.
Also we can't get the file compatible for PHP lower than 5.3 actually, as we are using namespaced classes inside it.
Btw. you can simply paste the files content on https://3v4l.org/ and check which php version it would run with...

…patible with PHP 5.3 as namespaces are in use
@bx80
Copy link
Contributor Author

bx80 commented Feb 14, 2023

@sgiehl I'd just tested locally with PHP5.6, hard to believe anyone would still be running < 5.5 at this stage almost 10yrs since it was released. I've removed the short array syntax and updated the header comment to state that the file should be compatible with PHP 5.3 (since we're using namespaces). Thanks for the 3v4l link 👍

@bx80 bx80 requested a review from sgiehl February 14, 2023 22:37
@sgiehl sgiehl removed the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Feb 15, 2023
@sgiehl sgiehl merged commit 31568a4 into 4.x-dev Feb 16, 2023
@sgiehl sgiehl deleted the m20350-php-min-ver-deprecation branch February 16, 2023 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review
Development

Successfully merging this pull request may close these issues.

PHP Fatal error: Default value for parameters with a class type hint can only be NULL
2 participants