Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Deprecated: version_compare(): Passing null to parameter #1 ($version1) of type string is deprecated in app/plugins/Diagnostics/Diagnostic/PhpVersionCheck.php on line 66 #19352

Closed
mattmary opened this issue Jun 14, 2022 · 15 comments · Fixed by #20133
Closed

Deprecated: version_compare(): Passing null to parameter #1 ($version1) of type string is deprecated in app/plugins/Diagnostics/Diagnostic/PhpVersionCheck.php on line 66 #19352

mattmary opened this issue Jun 14, 2022 · 15 comments · Fixed by #20133
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.

Comments

@mattmary
Copy link
Contributor

mattmary commented Jun 14, 2022 •
edited

there is a deprecated warning on php8.1 for a variable is null and should be a string

Expected Behavior

No warning displayed when displaying the system report page within the wordpress plugin.

Current Behavior

Screenshot from 2022-06-14 13-11-09
A warning is displayed when running with php8.1 the system report page

Possible Solution

define the $piwik_minimumPHPVersion variable as global in the app/core/testMinimumPhpVersion.php solves this issue.

Steps to Reproduce (for Bugs)

1.install the wordpress plugin
2.switch to php8.1 version for your apache
3.open the wp-admin/admin.php?page=matomo-systemreport url
4.views the warning
5.add global $piwik_minimumPHPVersion; in line 25 of the app/core/testMinimumPhpVersion.php
6. refresh the page
7. see the warning message has disappeared

Context

Your Environment

  • Matomo Version: 4.10.0
  • PHP Version: 8.1.7
  • Server Operating System: Apache 2.4
  • Additionally installed plugins:
  • Browser:
  • Operating System:
@mattmary mattmary added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Jun 14, 2022
@mattmary
Copy link
Contributor Author

another related warning

Deprecated: version_compare(): Passing null to parameter #1 ($version1) of type string is deprecated in app/core/CliMulti/CliPhp.php on line 76

@mattmary mattmary mentioned this issue Jun 14, 2022
Closed
@sgiehl
Copy link
Member

sgiehl commented Jun 14, 2022

@mattmary I'm not seeing this error when using Matomo the "normal" way on PHP 8.1.

@mattmary
Copy link
Contributor Author

@sgiehl when you say the normal way is matomo on premise?

@mattmary
Copy link
Contributor Author

refs matomo-org/matomo-for-wordpress#614

@sgiehl
Copy link
Member

sgiehl commented Jun 14, 2022

Yes. On premise

@mattmary
Copy link
Contributor Author

in php8.1 @sgiehl ?

@sgiehl
Copy link
Member

sgiehl commented Jun 14, 2022

tested it with 8.1.5

@mattmary
Copy link
Contributor Author

hi @sgiehl

I've tried different ways to fix this issue. the only one which work is to define as global the variable $piwik_minimumPHPVersion in this file. which impact would it have to do so?

@sgiehl
Copy link
Member

sgiehl commented Jun 16, 2022

That variable actually should be global. But maybe that isn't the case as WordPress includes Matomo 🤔

@mattmary mattmary mentioned this issue Jun 26, 2022
Merged
@justinvelluppillai
Copy link
Contributor

@sgiehl yes it looks like due to the way wordpress is including matomo it needs a workaround here. Any advice on whether we can apply @mattmary 's fix to core would be appreciated, or alternative ways to work around so he doesn't have to make this change every release for wordpress.

@sgiehl
Copy link
Member

sgiehl commented Jul 6, 2022

@justinvelluppillai I don't have Matomo for Wordpress set up, so I'm not able to say if there would a smarter solution. But adding a global keyword should not produce any problems I guess.

@sgiehl
Copy link
Member

sgiehl commented Nov 9, 2022

@mattmary Is this something we still need to change in core, or is the fix for wordpress a suitable solution and we can close this one?

@sgiehl sgiehl added Waiting for user feedback Indicates the Matomo team is waiting for feedback from the author or other users. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Nov 9, 2022
@mattmary
Copy link
Contributor Author

@sgiehl Sure I can fix it, but I'll have to do it each time we publish a new release. It would be definitively better if you could update the core.

@sgiehl sgiehl removed the Waiting for user feedback Indicates the Matomo team is waiting for feedback from the author or other users. label Nov 15, 2022
@sgiehl
Copy link
Member

sgiehl commented Nov 15, 2022

@mattmary Would you mind creating a pull request for core?

@mattmary
Copy link
Contributor Author

Hum @sgiehl

Now with the latest branch release, I've some fatal errors:
Screenshot from 2022-11-18 13-50-38
I guess I'll have to fix these updates first when we'll deploy the 4.13 release.

So I would suggest postponing this fix in the release after the 4.13.

This was referenced Dec 22, 2022
Merged
Closed
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fixes notice in wordpress plugin in PHP 8.1 matomo-org/matomo
3 participants
@justinvelluppillai @sgiehl @mattmary