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.

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

Fix #416 Session class incompatibility with PHP 7.x/8+ #417

Closed
wants to merge 4 commits into from

Conversation

mitkola
Copy link

@mitkola mitkola commented Nov 19, 2022

Due to difference in SessionInterfaceHandler interface in PHP 7.x and PHP 8+ gc method return type was ommited and suppressed warning as PHP 8+ It's better for future to make different version of Stash for PHP 7.x and PHP 8+

Minor change was made also to remove cmpatibility with PHP version before 5.4.0 while Stash package is compatible with 7+.

Due to difference in SessionInterfaceHandler interface in PHP 7.x and PHP 8+
gc method return type was ommited and suppressed warning as PHP 8+
It's better for future to make different version of Stash for PHP 7.x and PHP 8+

Minor change was made also to remove cmpatibility with PHP version before 5.4.0
while Stash package is compatible with 7+.
@onli onli mentioned this pull request Feb 17, 2023
@KorvinSzanto
Copy link
Contributor

It's not clear to me what you're referring to in this PR @mitkola, it'd probably be better to reconsider how this might change now that this repository is PHP 8.0+ and describe the changes and why they are necessary in different words.

@mitkola
Copy link
Author

mitkola commented Jan 31, 2024

It's not clear to me what you're referring to in this PR @mitkola, it'd probably be better to reconsider how this might change now that this repository is PHP 8.0+ and describe the changes and why they are necessary in different words.

The problem exist when installing stash with composer in php 7.2 (7.4) environment.
Composer installs version 0.17.6 of stash which contains incompatible for php 7.x definition of Session interface.

public function gc($maxlifetime) : int|false

In short, when installing stash in php 7.2+ environment it produces syntax error at this line.
I have to make workaround with downgrade and manually require version 0.16.0 of stash instead of automatically suggested 0.17.6..

@tedivm
Copy link
Member

tedivm commented Jan 31, 2024

PHP7 hasn't even had security updates in over a year. Don't run insecure ancient versions of PHP.

@tedivm tedivm closed this Jan 31, 2024
@onli
Copy link

onli commented Jan 31, 2024

@tedivm We considered Stash for use in a blogging engine, but not supporting PHP 7.x was a deal breaker. This is less about using it for new installations, which should run PHP 8.x, but providing an upgrade path for old installations. Also, there are still LTS distros out there that will provide (and patch) PHP 7.x, so actually not erroring out with them would be important.

It would really be appreciated if you could reconsider, being backwards compatible would make Stash a lot more useful. At least for us.

(I have no connection to the author of the PR, was just watching here).

@tedivm
Copy link
Member

tedivm commented Jan 31, 2024

The existing PR wasn't suitable as it was doing things like setting up a workflow dispatch.

What would be your preference? What was the last version that will work with your installs, and do you see a path for 0.17 working for it?

@onli
Copy link

onli commented Jan 31, 2024

PHP 7.4 would be enough for us :) That's usually achievable in those old installations.

What was the last version that will work with your installs, and do you see a path for 0.17 working for it?

Does 0.17 work with PHP 7.4 and PHP 8.x? Then yes, it would be an option to pin Stash to that older version.

It would of course be nicer to be able to use the current version, but psr/cache also set the php minimum to 8.0 with its 2.0...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants