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

Patch for working with ruby 2.5+ #12

Merged
merged 2 commits into from May 6, 2020

Conversation

ngoral
Copy link
Contributor

@ngoral ngoral commented May 1, 2020

Patch fixing illegal seek issue on Linux with Ruby 2.5 and higher

@utkarsh2102
Copy link

Wow, thanks @ngoral ❤️

@marcandre
Copy link
Owner

Thanks for the patch.
Can you please explain why the case of no arguments have to change for each and for write?

ngoral added 2 commits May 6, 2020 19:07
Since ruby 2.5 +write+ accepts more than one argument. Concider everyone calls +to_s+ on Hashes and Symbols
@ngoral ngoral force-pushed the patch_for_ruby_2_5_and_higher branch from 85bf430 to bb9fd45 Compare May 6, 2020 16:08
@ngoral
Copy link
Contributor Author

ngoral commented May 6, 2020

@marcandre I reconsidered the patch a bit. We now make a small consideration but unlike the previous version, this one won't break default writing to pipes

@marcandre
Copy link
Owner

I'm ok to merge this. The ideal patch in my opinion would be to use refinements instead for the method(s) that are now incompatible (or can potentially be). IIRC these cheats are not even needed anyways, right?

@marcandre marcandre closed this May 6, 2020
@marcandre marcandre reopened this May 6, 2020
@marcandre marcandre merged commit e2de9a4 into marcandre:master May 6, 2020
@marcandre
Copy link
Owner

Released v1.3.11

Are you up to writing a PR where either io.packed.write(66) or using Packable; io.write(66) would work? This would be v2.0.0

@ngoral ngoral deleted the patch_for_ruby_2_5_and_higher branch May 6, 2020 18:50
marcandre added a commit that referenced this pull request May 7, 2020
marcandre added a commit that referenced this pull request May 7, 2020
@marcandre
Copy link
Owner

Sorry, I reverted my mistake 🤦‍♂️ and excluded TTYs instead. v1.3.13 released

@ngoral ngoral restored the patch_for_ruby_2_5_and_higher branch May 14, 2020 09:53
akimd added a commit to akimd/packable that referenced this pull request Jun 16, 2020
I believe this is closely related to
marcandre#12: not all the streams are
seekable.  Unfortunately not all the non-tty streams are seekable too,
pipes are a problem.

So instead of checking if the stream is not a tty, let's use minitar's
seekable? test.
akimd added a commit to akimd/packable that referenced this pull request Jun 16, 2020
I believe this is closely related to
marcandre#12: not all the streams are
seekable.  Unfortunately not all the non-tty streams are seekable too,
pipes are a problem.

So instead of checking if the stream is not a tty, let's use minitar's
seekable? test.
akimd added a commit to akimd/packable that referenced this pull request Jun 16, 2020
I believe this is closely related to
marcandre#12: not all the streams are
seekable.  Unfortunately not all the non-tty streams are seekable too,
pipes are a problem.

So instead of checking if the stream is not a tty, let's use minitar's
seekable? test.
akimd added a commit to akimd/packable that referenced this pull request Jun 16, 2020
I believe this is closely related to
marcandre#12: not all the streams are
seekable.  Unfortunately not all the non-tty streams are seekable too,
pipes are a problem.

So instead of checking if the stream is not a tty, let's use minitar's
seekable? test.
akimd added a commit to akimd/packable that referenced this pull request Jun 16, 2020
I believe this is closely related to
marcandre#12: not all the streams are
seekable.  Unfortunately not all the non-tty streams are seekable too,
pipes are a problem.

So instead of checking if the stream is not a tty, let's use minitar's
seekable? test.
marcandre pushed a commit that referenced this pull request Jun 16, 2020
See #12
Adapted from minitar's seekable? test.
marcandre pushed a commit that referenced this pull request Jun 16, 2020
See #12
Adapted from minitar's seekable? test.
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

3 participants