Use IO.copy_stream when reading, writing #6958
Conversation
martinemde
force-pushed
the
martinemde/io-copy-stream
branch
from
77ae281
to
846dd72
Compare
martinemde
changed the title
|
Strangely truffleruby fails every time but only on this branch. IO.copy_stream discrepancy? It fails, If I'm not mistaken, because a binary file from a gem (a file that would pass through this change) is producing a different checksum. It is presumably checking this same checksum on the other rubies. |
martinemde
force-pushed
the
martinemde/io-copy-stream
branch
from
c047809
to
2a7d462
Compare
segiddins
force-pushed
the
martinemde/io-copy-stream
branch
from
2a7d462
to
f94a88a
Compare
martinemde
force-pushed
the
martinemde/io-copy-stream
branch
from
f94a88a
to
5ae7af7
Compare
| @@ -712,6 +712,16 @@ def verify_gz(entry) # :nodoc: | |||
| rescue Zlib::GzipFile::Error => e | |||
| raise Gem::Package::FormatError.new(e.message, entry.full_name) | |||
| end | |||
|
|
|||
| if RUBY_ENGINE == "truffleruby" | |||
There was a problem hiding this comment.
add a ruby engine version check as well?
There was a problem hiding this comment.
Is there a fixed version? I haven't heard anything. A ticket was mentioned but it had to do with the return value of copy_stream. I don't think this is cause by the return value.
There was a problem hiding this comment.
oracle/truffleruby#3280 (comment) -- 23.1.2, to be released next month
There was a problem hiding this comment.
Oh, that makes sense. If it leaves our tar reader pos at the beginning, we can't possibly read the files right.
lib/rubygems/package.rb
Outdated
|
|
||
| if RUBY_ENGINE == "truffleruby" | ||
| def copy_stream(src, dst) # :nodoc: | ||
| dst.write src.read 16_384 until src.eof? |
There was a problem hiding this comment.
if tests keep failing, maybe change this to dst.write src.read ?
There was a problem hiding this comment.
We have to assume it's the second change that's failing, reading from the tar.gz. This is the original code for the first change.
|
If this doesn't work I'll just make one change at a time. The befuddlement to code ratio of this change is surprisingly high. |
|
@martinemde I think this is ready to merge? |
|
I'll merge now, then when truffleruby is released we can follow up with an engine version when we know it works. |
Use IO.copy_stream when reading, writing (cherry picked from commit 558f516)
What was the end-user or developer problem that led to this PR?
Looking at increasing read/write efficiency. I'm not sure if this is actually better, but I believe that's the goal of IO.copy_stream.
What is your fix for the problem, implemented in this PR?
Use IO.copy_stream when writing gems to files from the gem archive.
Make sure the following tasks are checked