Skip to content

Fix some SPL bugs #8644

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
Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Fix some SPL bugs #8644

wants to merge 5 commits into from
+136 −33

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented May 27, 2022

The fix for #8563 can be backported to 8.0 but I'm not sure that is correct either due to some of the other tests I needed to fix afterwards.

The one for #8561 and #8562 cannot be backported due to the ABI changes.

The moving of tests files in a subfolder was so that one doesn't need to run the 600+ files just to check SplFileObject

@Girgias Girgias requested a review from cmb69 May 27, 2022 13:02
@Girgias
Copy link
Member Author

Girgias commented May 27, 2022 •
edited
Loading

However, maybe as READ_AHEAD is impossible to enable, and is arguably the correct behaviour we can just make the default be that and this should fix most of the issues in a BC and non ABI breaking way...

I didn't see it passed the intern->flags field directly to ZPP so it does do stuff...

@ramsey ramsey added this to the PHP 8.2 milestone May 27, 2022
@cmb69
Copy link
Member

cmb69 commented May 27, 2022

The moving of tests files in a subfolder was so that one doesn't need to run the 600+ files just to check SplFileObject

Yeah, that makes sense (modulo following upward merges), but I suggest to do that in a separate PR first.

@Girgias
Copy link
Member Author

Girgias commented May 27, 2022

The moving of tests files in a subfolder was so that one doesn't need to run the 600+ files just to check SplFileObject

Yeah, that makes sense (modulo following upward merges), but I suggest to do that in a separate PR first.

I'll cherry-pick this commit into master and then rebase

Girgias added 4 commits May 27, 2022 18:34
@Girgias
Use bool for silent parameter
52e9b1c
@Girgias
Fix phpGH-8561 and phpGH-8562
db7ff5a 
These bugs need a header change, and thus are an ABI break
@Girgias
Fix phpGH-8563
b3eb7e8 
With memory streams if we get a NULL buffer we must not instantiate an empty line
@Girgias
"Fix" tests, clearly there is a bug with Phar
45f5b63
@Girgias Girgias force-pushed the spl-fileinfo-bugs branch from b5d9c07 to 45f5b63 Compare May 27, 2022 17:34
@Girgias
Maybe fix Phar bug?
033c182 
At least I figured out something is hapenning when setting a custom InfoClass with a custom current()
@Girgias Girgias mentioned this pull request May 29, 2022
Closed
@Girgias Girgias marked this pull request as draft June 8, 2022 14:07
@adoy adoy removed this from the PHP 8.2 milestone Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmb69 cmb69 Awaiting requested review from cmb69

Assignees
No one assigned
Labels
Category: Tests Extension: spl Waiting on Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Girgias @cmb69 @ramsey @adoy