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

[soon-to-be-rfc] Implement GH-9673: $length argument for fpassthru #10476

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jan 30, 2023

Implements GH-9673

This implements an optional $length argument to fpassthru, SplFileObject::fpassthru and gzpassthru for partially copying the file to the output. The behaviour of this new argument has the same behaviour as the $length argument of stream_copy_to_stream.

Internally, a new macro and function is introduced: _php_stream_passthru_with_length. _php_stream_passthru now calls the new function to perform its tasks without introducing a BC break internally.

This unfortunately has one BC break: classes overriding SplFileObject must update the method signature of fpassthru to add the new argument.

My gut feeling says this probably needs an RFC? If that is true, I'm willing to pursue that process. At least this PR will show a PoC implementation in any case that can be commented on.

@devnexen
Copy link
Member

My gut feeling says this probably needs an RFC? If that is true, I'm willing to pursue that process. At least this PR will show a PoC implementation in any case that can be commented on.

You can always gather opinions on the mailing list at least.

@nielsdos
Copy link
Member Author

My gut feeling says this probably needs an RFC? If that is true, I'm willing to pursue that process. At least this PR will show a PoC implementation in any case that can be commented on.

You can always gather opinions on the mailing list at least.

Oh, yeah that's a good idea. I'll do that later today, thanks.

This implements an optional $length argument to fpassthru,
SplFileObject::fpassthru and gzpassthru for partially copying the file
to the output. The behaviour of this new argument has the same behaviour
as the $length argument of stream_copy_to_stream.

Internally, a new macro and function is introduced:
_php_stream_passthru_with_length. _php_stream_passthru now calls the new
function to perform its tasks without introducing a BC break internally.

This unfortunately has one BC break: classes overriding SplFileObject
must update the method signature of fpassthru to add the new argument.
@nielsdos
Copy link
Member Author

I did send out an email last week, but looks like nobody has replied.

@nielsdos nielsdos changed the title Implement GH-9673: $length argument for fpassthru [soon-to-be-rfc] Implement GH-9673: $length argument for fpassthru Feb 11, 2023
@iluuu1994 iluuu1994 added the RFC label Feb 17, 2023
@nielsdos
Copy link
Member Author

I no longer intend to RFC this, because of the BC break for implementers of SplFileObject.
I read the recent mailing list threads about BC breaks in old code bases and it got me thinking.
I think the benefit from this niche use case is not high enough to justify a BC break.
Although it is possible to keep the old signature for SplFileObject to avoid the BC break, I think introducing such an inconsistency is not much better.

@nielsdos nielsdos closed this Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants