[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Status report for Ruby bindings on Windows

From: Kouhei Sutou <kou_at_cozmixng.org>
Date: 2007-02-03 11:54:52 CET

Hi,

In <20070203.194319.222490146.kou@cozmixng.org>
  "Re: Status report for Ruby bindings on Windows" on Sat, 03 Feb 2007 19:43:19 +0900 (JST),
  Kouhei Sutou <kou@cozmixng.org> wrote:

> > > > > > > > * Normalizing the line breaks. Upon reflection, I've decided that I
> > > > > > > > started us down the wrong path when I modified the tests for the line
> > > > > > > > ending issues. As I've thought about it more, I now believe that the
> > > > > > > > bindings should do the conversions. My thinking is since Ruby treats
> > > > > > > > line endings as "'\n" instead of "\r\n" the bindings should provide
> > > > > > > > any multiline values Ruby expects. (This is where the 'b' comes from
> > > > > > > > when calling File::open. Without the 'b' on windows there is a \r\n
> > > > > > > > to \n conversion. With the 'b', no conversion. Maybe we should check
> > > > > > > > out what the Perl and Python bindings do about this issue on
> > > > > > > > windows?). (I've made zero effort to figure out what this will take).
> > > > > > >
> > > > > > > OK. We'll try the problem after we solve test environment
> > > > > > > problem on Windows.
> > > > > > >
> > > > > >
> > > > > > Removing all invocations and the definition of normalize_line_break
> > > > > > from the test\*.rb files and the attached patch get us:
> > > > >
> > > > > It seems that the changes make cat ignore svn:eof-style. I
> > > > > think tests have a problem. Could you try the attached
> > > > > patch?
> > > >
> > > > The client tests pass with the patch. I (now) still think normalizing
> > > > the line breaks in the test is the wrong solution. As you pointed
> > > > out, my patch wasn't right for cat, but I hope you'll consider parts
> > > > of it, or something like it for the the Core methods. Ruby on windows
> > > > uses \n for line breaks, even though windows uses \r\n. This gives us
> > > > a real problem to know when to pass the \r through and when not. I'll
> > > > see if I can figure out how (and if) the perl bindings deal with the
> > > > issue.
> > >
> > > OK. Could you give me a time to think this issue?
> > >
> >
> > Of course.
>
> The attached patch is my answer.
>
> 1. Remove normalize_line_break.
> 2. Use here document and binary mode open.
> 3. Don't use both gsub(/\r\n/, "\n") and gsub(/\n/, "\r\n").

I'm sorry. There are some typos. I'll attache a revised
patch.

Thanks,

--
kou

Index: subversion/bindings/swig/ruby/test/util.rb
===================================================================
--- subversion/bindings/swig/ruby/test/util.rb (revision 23318)
+++ subversion/bindings/swig/ruby/test/util.rb (working copy)
@@ -176,14 +176,6 @@
     auth_baton[Svn::Core::AUTH_PARAM_DEFAULT_USERNAME] = @author
   end
 
- def normalize_line_break(str)
- if windows?
- str.gsub(/\n/, "\r\n")
- else
- str
- end
- end
-
   module_function
   def windows?
     /cygwin|mingw|mswin32|bccwin32/.match(RUBY_PLATFORM)
Index: subversion/bindings/swig/ruby/test/test_client.rb
===================================================================
--- subversion/bindings/swig/ruby/test/test_client.rb (revision 23317)
+++ subversion/bindings/swig/ruby/test/test_client.rb (working copy)
@@ -820,7 +820,9 @@
   def test_merge
     log = "sample log"
     file = "sample.txt"
- src = normalize_line_break("sample\n")
+ src = <<-EOS
+sample
+EOS
     trunk = File.join(@wc_path, "trunk")
     branch = File.join(@wc_path, "branch")
     trunk_path = File.join(trunk, file)
@@ -834,7 +836,7 @@
     ctx.add(branch_path)
     rev1 = ctx.commit(@wc_path).revision
 
- File.open(branch_path, "w") {|f| f.print(src)}
+ File.open(branch_path, "wb") {|f| f.print(src)}
     rev2 = ctx.commit(@wc_path).revision
 
     ctx.merge(branch, rev1, branch, rev2, trunk)
@@ -867,7 +869,9 @@
   def test_merge_peg
     log = "sample log"
     file = "sample.txt"
- src = normalize_line_break("sample\n")
+ src = <<-EOS
+sample
+EOS
     trunk = File.join(@wc_path, "trunk")
     branch = File.join(@wc_path, "branch")
     trunk_path = File.join(trunk, file)
@@ -881,7 +885,7 @@
     ctx.add(branch_path)
     rev1 = ctx.commit(@wc_path).revision
 
- File.open(branch_path, "w") {|f| f.print(src)}
+ File.open(branch_path, "wb") {|f| f.print(src)}
     rev2 = ctx.commit(@wc_path).revision
 
     ctx.merge_peg(branch, rev1, rev2, trunk)
@@ -1273,12 +1277,16 @@
   
   def test_cat
     log = "sample log"
- src1 = normalize_line_break("source1\n")
- src2 = normalize_line_break("source2\n")
+ src1 = <<-EOS
+source1
+EOS
+ src2 = <<-EOS
+source2
+EOS
     file = "sample.txt"
     path = File.join(@wc_path, file)
 
- File.open(path, "w") {|f| f.print(src1)}
+ File.open(path, "wb") {|f| f.print(src1)}
 
     ctx = make_context(log)
     ctx.add(path)
@@ -1288,7 +1296,7 @@
     assert_equal(src1, ctx.cat(path, rev1))
     assert_equal(src1, ctx.cat(path))
     
- File.open(path, "w") {|f| f.print(src2)}
+ File.open(path, "wb") {|f| f.print(src2)}
 
     commit_info = ctx.commit(@wc_path)
     rev2 = commit_info.revision
@@ -1552,8 +1560,12 @@
 
   def test_switch
     log = "sample log"
- trunk_src = normalize_line_break("trunk source\n")
- tag_src = normalize_line_break("tag source\n")
+ trunk_src = <<-EOS
+trunk source
+EOS
+ tag_src = <<-EOS
+tag source
+EOS
     file = "sample.txt"
     file = "sample.txt"
     trunk_dir = "trunk"
@@ -1571,12 +1583,12 @@
     ctx = make_context(log)
 
     ctx.mkdir(trunk_dir_path)
- File.open(trunk_path, "w") {|f| f.print(trunk_src)}
+ File.open(trunk_path, "wb") {|f| f.print(trunk_src)}
     ctx.add(trunk_path)
     trunk_rev = ctx.commit(@wc_path).revision
     
     ctx.mkdir(tag_dir_path, tag_name_dir_path)
- File.open(tag_path, "w") {|f| f.print(tag_src)}
+ File.open(tag_path, "wb") {|f| f.print(tag_src)}
     ctx.add(tag_path)
     tag_rev = ctx.commit(@wc_path).revision
 
@@ -1614,12 +1626,14 @@
 
   def test_authentication
     log = "sample log"
- src = normalize_line_break("source\n")
+ src = <<-EOS
+source
+EOS
     file = "sample.txt"
     path = File.join(@wc_path, file)
     svnserve_uri = "#{@repos_svnserve_uri}/#{file}"
 
- File.open(path, "w") {|f| f.print(src)}
+ File.open(path, "wb") {|f| f.print(src)}
 
     ctx = make_context(log)
     ctx.add(path)
@@ -1659,12 +1673,14 @@
 
   def test_simple_provider
     log = "sample log"
- src = normalize_line_break("source\n")
+ src = <<-EOS
+source
+EOS
     file = "sample.txt"
     path = File.join(@wc_path, file)
     svnserve_uri = "#{@repos_svnserve_uri}/#{file}"
     
- File.open(path, "w") {|f| f.print(src)}
+ File.open(path, "wb") {|f| f.print(src)}
 
     ctx = make_context(log)
     setup_auth_baton(ctx.auth_baton)
@@ -1675,7 +1691,7 @@
     setup_auth_baton(ctx.auth_baton)
     ctx.add_simple_provider
     assert_raises(Svn::Error::RaNotAuthorized) do
- assert_equal(src, ctx.cat(svnserve_uri))
+ ctx.cat(svnserve_uri)
     end
 
     ctx = Svn::Client::Context.new
@@ -1698,12 +1714,14 @@
     return unless Svn::Core.respond_to?(:add_windows_simple_provider)
 
     log = "sample log"
- src = normalize_line_break("source\n")
+ src = <<-EOS
+source
+EOS
     file = "sample.txt"
     path = File.join(@wc_path, file)
     svnserve_uri = "#{@repos_svnserve_uri}/#{file}"
     
- File.open(path, "w") {|f| f.print(src)}
+ File.open(path, "wb") {|f| f.print(src)}
 
     ctx = make_context(log)
     setup_auth_baton(ctx.auth_baton)
@@ -1714,7 +1732,7 @@
     setup_auth_baton(ctx.auth_baton)
     ctx.add_windows_simple_provider
     assert_raises(Svn::Error::RaNotAuthorized) do
- assert_equal(src, ctx.cat(svnserve_uri))
+ ctx.cat(svnserve_uri)
     end
 
     ctx = Svn::Client::Context.new
Index: subversion/bindings/swig/ruby/test/test_core.rb
===================================================================
--- subversion/bindings/swig/ruby/test/test_core.rb (revision 23317)
+++ subversion/bindings/swig/ruby/test/test_core.rb (working copy)
@@ -343,9 +343,11 @@
     modified_header = "(mod)"
 
     original.open
+ original.binmode
     original.print(original_src)
     original.close
     modified.open
+ modified.binmode
     modified.print(modified_src)
     modified.close
 
@@ -361,8 +363,7 @@
 + c
 EOD
     diff = Svn::Core::Diff.file_diff(original.path, modified.path)
- assert_equal(normalize_line_break(expected),
- diff.unified(original_header, modified_header))
+ assert_equal(expected, diff.unified(original_header, modified_header))
 
     options = Svn::Core::DiffFileOptions.parse("--ignore-space-change")
     expected = <<-EOD
@@ -376,8 +377,7 @@
    c
 EOD
     diff = Svn::Core::Diff.file_diff(original.path, modified.path, options)
- assert_equal(normalize_line_break(expected),
- diff.unified(original_header, modified_header))
+ assert_equal(expected, diff.unified(original_header, modified_header))
 
     options = Svn::Core::DiffFileOptions.parse("--ignore-all-space")
     expected = <<-EOD
@@ -390,8 +390,7 @@
    c
 EOD
     diff = Svn::Core::Diff.file_diff(original.path, modified.path, options)
- assert_equal(normalize_line_break(expected),
- diff.unified(original_header, modified_header))
+ assert_equal(expected, diff.unified(original_header, modified_header))
   end
 
   def test_diff_merge
@@ -421,12 +420,15 @@
 EOS
 
     original.open
+ original.binmode
     original.print(original_src)
     original.close
     modified.open
+ modified.binmode
     modified.print(modified_src)
     modified.close
     latest.open
+ latest.binmode
     latest.print(latest_src)
     latest.close
 
@@ -440,7 +442,7 @@
     diff = Svn::Core::Diff.file_diff3(original.path,
                                       modified.path,
                                       latest.path)
- assert_equal(normalize_line_break(expected), diff.merge)
+ assert_equal(expected, diff.merge)
 
     options = Svn::Core::DiffFileOptions.parse("--ignore-space-change")
     expected = <<-EOD
@@ -454,7 +456,7 @@
                                       modified.path,
                                       latest.path,
                                       options)
- assert_equal(normalize_line_break(expected), diff.merge)
+ assert_equal(expected, diff.merge)
 
     options = Svn::Core::DiffFileOptions.parse("--ignore-all-space")
     expected = <<-EOD
@@ -468,7 +470,7 @@
                                       modified.path,
                                       latest.path,
                                       options)
- assert_equal(normalize_line_break(expected), diff.merge)
+ assert_equal(expected, diff.merge)
   end
 
   def test_diff_file_options
Index: subversion/bindings/swig/ruby/test/test_fs.rb
===================================================================
--- subversion/bindings/swig/ruby/test/test_fs.rb (revision 23317)
+++ subversion/bindings/swig/ruby/test/test_fs.rb (working copy)
@@ -293,20 +293,44 @@
   def test_delta
     log = "sample log"
     file = "source.txt"
- src = "a\nb\nc\nd\ne\n"
- modified = "A\nb\nc\nd\nE\n"
- result = "a\n\n\n\ne\n"
- expected = "A\n\n\n\nE\n"
+ src = <<-EOS
+a
+b
+c
+d
+e
+EOS
+ modified = <<-EOS
+A
+b
+c
+d
+E
+EOS
+ result = <<-EOS
+a
+
+
+
+e
+EOS
+ expected = <<-EOS
+A
+
+
+
+E
+EOS
     path_in_repos = "/#{file}"
     path = File.join(@wc_path, file)
     
     ctx = make_context(log)
     
- File.open(path, "w") {|f| f.print(src)}
+ File.open(path, "wb") {|f| f.print(src)}
     ctx.add(path)
     rev1 = ctx.ci(@wc_path).revision
 
- File.open(path, "w") {|f| f.print(modified)}
+ File.open(path, "wb") {|f| f.print(modified)}
     @fs.transaction do |txn|
       checksum = MD5.new(result).hexdigest
       stream = txn.root.apply_text(path_in_repos, checksum)
@@ -314,7 +338,7 @@
       stream.close
     end
     ctx.up(@wc_path)
- assert_equal(expected, File.open(path){|f| f.read})
+ assert_equal(expected, File.open(path) {|f| f.read})
 
     rev2 = ctx.ci(@wc_path).revision
     stream = @fs.root(rev2).file_delta_stream(@fs.root(rev1),
Index: subversion/bindings/swig/ruby/test/test_info.rb
===================================================================
--- subversion/bindings/swig/ruby/test/test_info.rb (revision 23317)
+++ subversion/bindings/swig/ruby/test/test_info.rb (working copy)
@@ -176,7 +176,7 @@
     file4_prop_key = "XXX"
     file4_prop_value = "YYY"
     File.open(file1_path, "w") {|f| f.puts "changed"}
- File.open(file2_path, "w") {|f| f.puts "removed\nadded"}
+ File.open(file2_path, "w") {|f| f.puts "removed"; f.puts "added"}
     FileUtils.touch(file4_path)
     ctx.add(file4_path)
     ctx.propdel(file1_prop_key, file1_path)
@@ -205,10 +205,14 @@
     assert_equal(0, info.diffs[file4][:added].deleted_line)
     assert_equal(0, info.diffs[file5_key][:copied].added_line)
     assert_equal(0, info.diffs[file5_key][:copied].deleted_line)
- assert_equal("Name: #{file1_prop_key}\n - #{file1_prop_value}\n",
- info.diffs[file1][:property_changed].body)
- assert_equal("Name: #{file4_prop_key}\n + #{file4_prop_value}\n",
- info.diffs[file4][:property_changed].body)
+ assert_equal(<<-EOD, info.diffs[file1][:property_changed].body)
+Name: #{file1_prop_key}
+ - #{file1_prop_value}
+EOD
+ assert_equal(<<-EOD, info.diffs[file4][:property_changed].body)
+Name: #{file4_prop_key}
+ + #{file4_prop_value}
+EOD
     assert_equal(commit_info.revision, info.revision)
     assert_equal(log, info.log)
   end
Index: subversion/bindings/swig/ruby/svn/util.rb
===================================================================
--- subversion/bindings/swig/ruby/svn/util.rb (revision 23317)
+++ subversion/bindings/swig/ruby/svn/util.rb (working copy)
@@ -76,7 +76,7 @@
     def filename_to_temp_file(filename)
       file = Tempfile.new("svn-ruby")
       file.binmode
- file.print(File.read(filename))
+ file.print(File.open(filename, "rb") {|f| f.read})
       file.close
       file.open
       file.binmode

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Feb 3 11:55:13 2007

This is an archived mail posted to the Subversion Dev mailing list.