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

PHP 8.0 and 8.1 #85

Merged
merged 5 commits into from
Oct 17, 2022
Merged

PHP 8.0 and 8.1 #85

merged 5 commits into from
Oct 17, 2022

Conversation

Tybaze
Copy link
Collaborator

@Tybaze Tybaze commented Jun 10, 2022

PHP 8.0 & 8.1 compatibility

[No Enhancement - please considere to use Doctrine2 for your new Project]

Target : Maintains maximum backward compatibility down to 5.3 for the moment

PHP 8.0 and 8.1 brings a lot of incompatibility.
https://www.php.net/manual/en/migration80.incompatible.php
https://www.php.net/manual/en/migration81.incompatible.php

Tests

I have to update travis, to include new php versions, and to remove php5.3 on Precise (letsencrypt root certificate is broken, impossible to download composer anymore)

I tested localy on PHP 5.3 and passed the build successfully.

alt text
Travis Build Link

PHP version Status
php5.3 🟢
php5.4 🟢
php5.5 🟢
php5.6 🟢
php7.0 🟢
php7.1 🟢
php7.2 🟢
php7.3 🟢
php7.4 🟢
php8.0 🟢
php8.1 🟢

Future

One Day PHP 9 will broke compatibility with php < 7.4 due to the lacks of "mixed" keywords implementations.

Please considere rewriting asap your project to another framework, like Doctrine2 if your app cannot not support at least PHP 7.4 within a few months/years

@alquerci
Copy link

Hello @Tybaze

Thanks for your contribution for the huge goal of PHP 8 support.

Did you execute tests based on the fix of tests execution, see #56 ?

@Tybaze
Copy link
Collaborator Author

Tybaze commented Jun 11, 2022

Did you execute tests based on the fix of tests execution, see #56 ?

No sorry didn't see it, I run test directly with travis.
I'll try them and #86 on monday.

@Tybaze
Copy link
Collaborator Author

Tybaze commented Jun 14, 2022

I was using a wrong way to run test this PR

I have reviewed #56 and patched my branch localy to do the test.
I also use your new #86 with docker compose, great thanks @alquerci

Now I have updated the first post of the PR, new status will be, once #56 will be merged

PHP version Status
php8.0 🟢
php8.1 🔴

Working on PHP8.1...

@Tybaze
Copy link
Collaborator Author

Tybaze commented Jun 14, 2022

Everything is working now on PHP8.0 and PHP8.1 👍

How do I test :

git clone git@github.com:Tybaze/doctrine1.git
git remote add alquerci https://github.com/alquerci/doctrine1.git
git fetch alquerci

# PR85
git checkout compat_php8.1

# PR86 
git merge alquerci/add-docker-compose

# PR56 
git merge alquerci/fix-tests-to-be-able-to-finish-it-without-a-fatal-error

# Fix a very small conflict in .gitignore
git commit 

# modify file ./tests/bin/test , replace variable "skipPHPVersions" with "php9"
# Then run the test
./tests/bin/test

And we get it :

PHP version Status
php5.3 🟢
php5.4 🟢
php5.5 🟢
php5.6 🟢
php7.0 🟢
php7.1 🟢
php7.2 🟢
php7.3 🟢
php7.4 🟢
php8.0 🟢
php8.1 🟢

@alquerci
Copy link

alquerci commented Jul 25, 2022

A pass of code structure fixes of modified parts will be appreciated.

@Tybaze Tybaze force-pushed the compat_php8.1 branch 2 times, most recently from 80ee295 to c66908b Compare August 2, 2022 15:12
@Tybaze Tybaze requested a review from thePanz August 2, 2022 15:23
@Tybaze
Copy link
Collaborator Author

Tybaze commented Oct 5, 2022

Hi Everyone ^^
Do I miss something to merge this PR ?

@alquerci
Copy link

alquerci commented Oct 5, 2022

@Tybaze
Copy link
Collaborator Author

Tybaze commented Oct 5, 2022

Rebase:
Merge commits about the TaskName declaration, using the latest @thePanz version:
if ('' === trim($taskName)) {

Merge commits review of this PR about annotation, and conding style.

@thePanz
Copy link
Member

thePanz commented Oct 7, 2022

@Tybaze coudl you rebase on the latest master branch?

  • the first two commits from xNatek are already merged
  • squash commits to group related changes togerter
  • remove the "merge master" commits from my side (I tried to use the GitHub feature, but we should not have those commits in the project's history)

Thanks! then we will be able to merge this! 🎉

Tybaze and others added 5 commits October 17, 2022 10:09
…lue anymore.

Method str_replace/strtotime now require a string, not null

FIX: PDOStatement::fetch, $cursorOffset must be a int
Doctrine_Connection_Statement->fetch() default value to null

FIX: Private methods cannot be final as they are never overridden by other classes
Doctrine_Query_Having->_parseAliases(), remove "final"
sfYamlInline, backport fix from Symfony1.

Doctrine_Hydrator_Graph fix array_map, rtrim(): Passing null to parameter #1 ($string) of type string is deprecated
I emmit the hypothese that this array_map was broken, because array_map result is not assigned.

Doctrine_Migration_Diff:333, str_replace(): Passing null to parameter #2 ($replace) of type array|string is deprecated

Doctrine_Migration_Builder:78:, is_dir(): Passing null to parameter #1 ($filename) of type string is deprecated

Doctrine_Validator_Notblank, allow null value
HydrationListener, in HydrateTestCase.php, fix strtoupper(): Passing null to parameter #1 ($string) of type string is deprecated

internal_method_return_types
https://wiki.php.net/rfc/internal_method_return_types
see 2b2d173 for details
Doctrine_Collection_OnDemand
Doctrine_Validator_Exception

PHP 8.1 PDO stringify is now disable by default.

Activate it for Mysql + Sqlite
https://www.php.net/manual/en/migration81.incompatible.php#migration81.incompatible.pdo.mysql

PHP 8.1 Fix: Warning: strtotime() : Epoch doesn't fit in a PHP integer in Doctrine_Record.
This is only happening on 32bit system, because int 32bit could not map the whole strtotime date scope.
Example value: "0000-00-00 00:00:00"
Before 8.1 strtotime returns false, after it return false but also raise a Warning.
@ is slightly lowering performance, it should not trigger any unwanted error, as if format is invalid strtotime should return "false"
As this old project need BC for old system, seems the best fix.

PHP 8.1 > Automatic conversion of false to array is deprecated
Fix Doctrine_Record _invokedSaveHooks cannot assign array value to boolean
Declaration to array instead of boolean

PHP 8.1 > Serializable Phase Out
https://wiki.php.net/rfc/phase_out_serializable

PHP 7.4 add a new Serialize mecanism
PHP 8.1 made old method, "Serializable implementation" deprecated
PHP 9.0 (no release date at this moment) will drop the support.

Temporary Fix: Adding both method serialize/unserialize and __serialize/__unserialize

In order to be compatible with future PHP 9.0, once it will be release, we will have to drop the support to PHP Version before 7.4.

Currently a lot of Unix distribution in LTS are running a PHP Version older than 7.4 so moving to the final solution of "add return type" should break a lot of setup for the moment.

PHP 8.1 > internal_method_return_types
https://wiki.php.net/rfc/internal_method_return_types

PHP 8.0 added return type for abstract methods on Iterator, ArrayAccess, Countable, IteratorAggregate
PHP 8.1 made non implementation as a Deprecated Warning
PHP 9.0 (no release date at this moment) will drop the support.

Temporary Fix : adding this Attribute
#[\ReturnTypeWillChange]
Will drop the Deprecated warning.

Adding return type will break compatibility before PHP 7.4,
Return type has been added on PHP 7.0, but "mixed" special type is required, and it has been added on PHP 7.4.
In order to be compatible with future PHP 9.0, once it will be release, we will have to drop the support to PHP Version before 7.4

Currently a lot of Unix distribution in LTS are running a PHP Version older than 7.4 so moving to the final solution of "add return type" should break a lot of setup for the moment.

Update Travis to PHP up to 8.1

PHP 8.0 > Doctrine_Query:36, uncaught TypeError: Unsupported operand types: string % int

Doctrine_Parser_Xml:89, htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated

https://wiki.php.net/rfc/internal_method_return_types for
Doctrine_Node
Doctrine_Adapter_Mock
Doctrine_EventListener_TestLogger
Doctrine_Parser_Xml

Doctrine_Ticket_1254_TestCase, replace stftime() by date() with format adaptation.
Fix BC compatibility for any dev using fetch($currentOffset = null)
Fix SQLite Connect to return a boolean
Remove useless string cast by testing null before
Check TaskName declaration

Fix test 1783 - 64bit compatibility
On 32 bit system, PHP use a float to overflow a bigint.
On 64 bit, PHP int is the same as a database bigint, so this test is not relevant anymore
@Tybaze
Copy link
Collaborator Author

Tybaze commented Oct 17, 2022

@Tybaze coudl you rebase on the latest master branch?

* the first two commits from xNatek are already merged

* squash commits to group related changes togerter

* remove the "merge master" commits from my side (I tried to use the GitHub feature, but we should not have those commits in the project's history)

Thanks! then we will be able to merge this! 🎉

  • Rebased on master (including xNatek commit)
  • Squashed by modification type (Compat 8.0, 8.1, PR Review)
  • Cleaned Merge master

@thePanz You're welcome !
Sorry for the delay ^^

@thePanz thePanz merged commit f8ea8b7 into FriendsOfSymfony1:master Oct 17, 2022
@thePanz
Copy link
Member

thePanz commented Oct 17, 2022

Merged @Tybaze @alquerci
Thanks for your efforts!

@thirsch
Copy link
Collaborator

thirsch commented Oct 20, 2022

Hi @thePanz, thanks for merging. Could you also create a new version and maybe have a look at the php 8.0/8.1 pr on the symfony1 repo as well? EOL of php 7.4 is coming close.

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.

4 participants