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 PHP 8.1 build failures #36813

Closed
driesvints opened this issue Mar 30, 2021 · 12 comments
Closed

Fix PHP 8.1 build failures #36813

driesvints opened this issue Mar 30, 2021 · 12 comments

Comments

@driesvints
Copy link
Member

driesvints commented Mar 30, 2021

We have a working PHP 8.1 build now on the 8.x branch but there are still quite a few build failures which we should try to fix.

https://www.stitcher.io/blog/new-in-php-81

@bogdankharchenko
Copy link
Contributor

@driesvints examples?

@driesvints
Copy link
Member Author

Sorry I should have linked to a failing build. See here: https://github.com/laravel/framework/runs/2247074654

Basically you can inspect the "Execute tests" step of the PHP 8.1 build on the 8.x branch.

@bogdankharchenko
Copy link
Contributor

@driesvints thanks, I'll PR some of these

@jrmajor
Copy link
Contributor

jrmajor commented Jun 1, 2021

Integers and floats in result sets will now be returned using native PHP types instead of strings when using emulated prepared statements. This matches the behavior of native prepared statements. You can restore the previous behavior by enabling the PDO::ATTR_STRINGIFY_FETCHES option.

(from https://github.com/php/php-src/blob/96fe8141c397518e4ee10e65a7b921d779d332b6/UPGRADING#L117-L126)

PDO::ATTR_STRINGIFY_FETCHES is explicitly disabled here:

protected $options = [
PDO::ATTR_CASE => PDO::CASE_NATURAL,
PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
PDO::ATTR_ORACLE_NULLS => PDO::NULL_NATURAL,
PDO::ATTR_STRINGIFY_FETCHES => false,
PDO::ATTR_EMULATE_PREPARES => false,
];

I don't know what's the difference between emulated and native prepared statements. What confuses me is PDO::ATTR_EMULATE_PREPARES => false, which looks like emulated prepared statements being disabled, and this change is supposed to apply only to them. I would appreciate it if somebody could demystify that for me.

Anyway, many DB tests are failing because of this change. Would you accept a PR adding version checks like this:

- $this->assertSame('2', $post->comments_count);
+ $this->assertSame(PHP_VERSION_ID >= 80100 ? 2 : '2', $post->comments_count);

or do you prefer another solution?

@driesvints
Copy link
Member Author

driesvints commented Jul 9, 2021

Atm this issue is blocked by aws/aws-sdk-php#2269 (resolved)

@jrmajor thanks for that. I'll take a look at it as soon as the above is unblocked.

@driesvints
Copy link
Member Author

Blocked by aws/aws-sdk-php#2284 for now.

@driesvints
Copy link
Member Author

AWS PHP SDK is now unblocked. Sent in a PR to the next blocking repo: dflydev/dflydev-dot-access-data#40

@driesvints
Copy link
Member Author

Next up is a Mockery fix for PHP 8.1 return types. Waiting until @GrahamCampbell has mockery/mockery@b4da544 committed to master.

@drbyte
Copy link
Contributor

drbyte commented Aug 19, 2021

Edit: sorry, the following isn't even related to PHP calls. 😏

Fortunately only 2 source files appear to be affected by this deprecation:
https://wiki.php.net/rfc/deprecations_php_8_1#strftime_and_gmstrftime
https://github.com/laravel/framework/search?q=strftime

@Sn0wCrack
Copy link

Looks like those references to strftime are in an SQLite generated query, so doesn't look like either is affected by this deprecation.

Also looks like Mockery is waiting on this PR now mockery/mockery#1140

@drbyte
Copy link
Contributor

drbyte commented Aug 24, 2021

Looks like those references to strftime are in an SQLite generated query, so doesn't look like either is affected by this deprecation.

Oy! True. I had apparently been staring at too much code when I dug into that. Thanks for pointing it out. 💯

@driesvints
Copy link
Member Author

Gonna close this to continue the discussion in the related PR.

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

No branches or pull requests

5 participants