From 460c240d5222a806975acde61486dbd005ed55b6 Mon Sep 17 00:00:00 2001 From: Warren Seen Date: Thu, 3 Jan 2013 20:01:28 +1100 Subject: [PATCH 1/5] Add ensure_open_and_* methods to StringIO. These methods are required to allow IO.copy_stream to work against a StringIO instance. Alternatively, it may be better to refactor IO.copy_stream to not use these methods, as they aren't API-compatible with the MRI IO implementation? --- kernel/common/io19.rb | 2 +- lib/19/stringio.rb | 7 +++++++ spec/ruby/core/io/copy_stream_spec.rb | 24 ++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/kernel/common/io19.rb b/kernel/common/io19.rb index a01ffd372f..292895742e 100644 --- a/kernel/common/io19.rb +++ b/kernel/common/io19.rb @@ -305,7 +305,7 @@ def run @from.close end - @to.close unless @to_io + @to.close unless @to_io || @to.closed? end end diff --git a/lib/19/stringio.rb b/lib/19/stringio.rb index 8fbf8bc0fa..32677de53d 100644 --- a/lib/19/stringio.rb +++ b/lib/19/stringio.rb @@ -66,13 +66,16 @@ def check_readable raise IOError, "not opened for reading" unless @readable end + alias_method :ensure_open_and_readable, :check_readable private :check_readable + def check_writable raise IOError, "not opened for writing" unless @writable raise IOError, "unable to modify data" if @__data__.string.frozen? end + alias_method :ensure_open_and_writable, :check_writable private :check_writable def set_encoding(external, internal=nil, options=nil) @@ -306,6 +309,10 @@ def pid nil end + def pipe? + true + end + def pos @__data__.pos end diff --git a/spec/ruby/core/io/copy_stream_spec.rb b/spec/ruby/core/io/copy_stream_spec.rb index 865b6bea49..eb033fd441 100644 --- a/spec/ruby/core/io/copy_stream_spec.rb +++ b/spec/ruby/core/io/copy_stream_spec.rb @@ -1,5 +1,6 @@ require File.expand_path('../../../spec_helper', __FILE__) require File.expand_path('../fixtures/classes', __FILE__) +require 'stringio' ruby_version_is "1.9" do describe :io_copy_stream_to_file, :shared => true do @@ -235,5 +236,28 @@ it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream end end + + describe "from a string IO" do + before :each do + @from_io = StringIO.new(@content) + IOSpecs::CopyStream.from = @from_io + end + + describe "to a file name" do + it_behaves_like :io_copy_stream_to_file, nil, IOSpecs::CopyStream + end + + describe "to an IO" do + before :each do + @to_io = new_io @to_name, "w:utf-8" + end + + after :each do + @to_io.close + end + + it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream + end + end end end From 24aeecb170daad58a0b8112b0df176e1f95e85ef Mon Sep 17 00:00:00 2001 From: Warren Seen Date: Mon, 1 Apr 2013 17:33:15 +1100 Subject: [PATCH 2/5] Correctly emulate MRI copy_stream behavior when operating on a StringIO instance. --- kernel/common/io19.rb | 13 +++++++++++-- lib/19/stringio.rb | 4 ---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/kernel/common/io19.rb b/kernel/common/io19.rb index 292895742e..ac47bb450e 100644 --- a/kernel/common/io19.rb +++ b/kernel/common/io19.rb @@ -280,7 +280,14 @@ def run saved_pos = 0 end - @from.seek @offset, IO::SEEK_CUR if @offset + if @offset + if @from.respond_to?(:descriptor) && @from.descriptor != -1 + @from.seek @offset, IO::SEEK_CUR + else + raise ArgumentError, "cannot specify src_offset for non-IO" + end + end + size = @length ? @length : 16384 bytes = 0 @@ -296,7 +303,9 @@ def run # done reading end - @to.flush + # Should only flush if @from and @to are both descended from IO + # in other words, MRI does not flush copy_stream from/to a StringIO + @to.flush if @from.kind_of?(IO) && @to.kind_of?(IO) return bytes ensure if @from_io diff --git a/lib/19/stringio.rb b/lib/19/stringio.rb index 32677de53d..d035851723 100644 --- a/lib/19/stringio.rb +++ b/lib/19/stringio.rb @@ -309,10 +309,6 @@ def pid nil end - def pipe? - true - end - def pos @__data__.pos end From 8a69d17c0ab3b416527e4e5cbbd0ec1393e375b2 Mon Sep 17 00:00:00 2001 From: Warren Seen Date: Mon, 1 Apr 2013 17:34:20 +1100 Subject: [PATCH 3/5] Update copy_stream spec to reflect the need to flush when operating with a StringIO parameter. --- spec/ruby/core/io/copy_stream_spec.rb | 12 ++++++++++++ spec/ruby/core/io/fixtures/classes.rb | 10 +++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/spec/ruby/core/io/copy_stream_spec.rb b/spec/ruby/core/io/copy_stream_spec.rb index eb033fd441..43ab2bfe44 100644 --- a/spec/ruby/core/io/copy_stream_spec.rb +++ b/spec/ruby/core/io/copy_stream_spec.rb @@ -47,6 +47,7 @@ describe :io_copy_stream_to_io, :shared => true do it "copies the entire IO contents to the IO" do IO.copy_stream(@object.from, @to_io) + @to_io.flush if @object.flush? @to_name.should have_data(@content) IO.copy_stream(@from_bigfile, @to_io) @to_name.should have_data(@content + @content_bigfile) @@ -60,6 +61,7 @@ it "starts writing at the destination IO's current position" do @to_io.write("prelude ") IO.copy_stream(@object.from, @to_io) + @to_io.flush if @object.flush? @to_name.should have_data("prelude " + @content) end @@ -81,6 +83,7 @@ it "copies only length bytes when specified" do IO.copy_stream(@object.from, @to_io, 8).should == 8 + @to_io.flush if @object.flush? @to_name.should have_data("Line one") end end @@ -241,10 +244,15 @@ before :each do @from_io = StringIO.new(@content) IOSpecs::CopyStream.from = @from_io + IOSpecs::CopyStream.flush = true end describe "to a file name" do it_behaves_like :io_copy_stream_to_file, nil, IOSpecs::CopyStream + + it "raises an error when an offset is specified" do + lambda { IO.copy_stream(@from_io, @to_name, 8, 4) }.should raise_error(ArgumentError) + end end describe "to an IO" do @@ -257,6 +265,10 @@ end it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream + + it "raises an error when an offset is specified" do + lambda { IO.copy_stream(@from_io, @to_io, 8, 4) }.should raise_error(ArgumentError) + end end end end diff --git a/spec/ruby/core/io/fixtures/classes.rb b/spec/ruby/core/io/fixtures/classes.rb index 02ec95d1fe..928d3f4123 100644 --- a/spec/ruby/core/io/fixtures/classes.rb +++ b/spec/ruby/core/io/fixtures/classes.rb @@ -121,7 +121,7 @@ def self.closed_io io.close io end - + # Creates a pipe-based IO fixture containing the specified # contents def self.pipe_fixture(content) @@ -146,5 +146,13 @@ def self.from=(from) def self.from @from end + + def self.flush=(flush) + @flush = true + end + + def self.flush? + @flush + end end end From 662f5a889e97c0a6c720097943f622cefa8fb909 Mon Sep 17 00:00:00 2001 From: Warren Seen Date: Tue, 2 Apr 2013 09:19:13 +1100 Subject: [PATCH 4/5] Actually set the flush ivar based on the assigned param. --- spec/ruby/core/io/fixtures/classes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ruby/core/io/fixtures/classes.rb b/spec/ruby/core/io/fixtures/classes.rb index 928d3f4123..9c8b6ae2ce 100644 --- a/spec/ruby/core/io/fixtures/classes.rb +++ b/spec/ruby/core/io/fixtures/classes.rb @@ -148,7 +148,7 @@ def self.from end def self.flush=(flush) - @flush = true + @flush = flush end def self.flush? From 83b271fa8052861348250026e77f0036bda14a61 Mon Sep 17 00:00:00 2001 From: Warren Seen Date: Tue, 2 Apr 2013 09:19:44 +1100 Subject: [PATCH 5/5] Remove stray whitespace. --- lib/19/stringio.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/19/stringio.rb b/lib/19/stringio.rb index d035851723..40623bec26 100644 --- a/lib/19/stringio.rb +++ b/lib/19/stringio.rb @@ -69,7 +69,6 @@ def check_readable alias_method :ensure_open_and_readable, :check_readable private :check_readable - def check_writable raise IOError, "not opened for writing" unless @writable raise IOError, "unable to modify data" if @__data__.string.frozen?