* lib/fileutils.rb (fu_each_src_dest): raise if src==dest. [ruby-talk:85344] [ruby-core:01699]

* lib/fileutils.rb: use Object#is_a? instead of Class#=== to allow e.g. remote objects for receivers.
* lib/fileutils.rb: FileTest -> File.
* lib/fileutils.rb: put parentheses for arguments of File.xxxx?
* test/fileutils/test_fileutils.rb (test_cp): test "cp a a".
* test/fileutils/test_fileutils.rb (test_mv): test "mv a a".
* test/fileutils/test_fileutils.rb (test_ln): test "ln a a".
* test/fileutils/test_fileutils.rb (test_ln_s): test "ln_s a a".
* test/fileutils/test_fileutils.rb (test_install): test "install a a".
* test/fileutils/fileasserts.rb: new method assert_symlink.
* test/fileutils/fileasserts.rb: assert_is_directory -> assert_directory.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@4976 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,29 @@
+Tue Nov 18 14:06:35 2003  Minero Aoki  <aamine@loveruby.net>
+
+	* lib/fileutils.rb (fu_each_src_dest): raise if src==dest.
+	  [ruby-talk:85344] [ruby-core:01699]
+
+	* lib/fileutils.rb: use Object#is_a? instead of Class#=== to allow
+	  e.g. remote objects for receivers.
+
+	* lib/fileutils.rb: FileTest -> File.
+
+	* lib/fileutils.rb: put parentheses for arguments of File.xxxx?
+
+	* test/fileutils/test_fileutils.rb (test_cp): test "cp a a".
+
+	* test/fileutils/test_fileutils.rb (test_mv): test "mv a a".
+
+	* test/fileutils/test_fileutils.rb (test_ln): test "ln a a".
+
+	* test/fileutils/test_fileutils.rb (test_ln_s): test "ln_s a a".
+
+	* test/fileutils/test_fileutils.rb (test_install): test "install a a".
+
+	* test/fileutils/fileasserts.rb: new method assert_symlink.
+
+	* test/fileutils/fileasserts.rb: assert_is_directory -> assert_directory.
+
 Mon Nov 17 10:50:27 2003  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* lib/optparse.rb (OptionParser::Completion::complete): allow least

lib/fileutils.rb

@@ -114,10 +114,10 @@ module FileUtils
   def uptodate?( new, old_list, options = nil )
     raise ArgumentError, 'uptodate? does not accept any option' if options
 
-    return false unless FileTest.exist?(new)
+    return false unless File.exist?(new)
     new_time = File.mtime(new)
     old_list.each do |old|
-      if FileTest.exist?(old)
+      if File.exist?(old)
         return false unless new_time > File.mtime(old)
       end
     end
@@ -172,7 +172,7 @@ module FileUtils
     mode = options[:mode] || (0777 & ~File.umask)
     list.map {|n| File.expand_path(n) }.each do |dir|
       stack = []
-      until FileTest.directory?(dir)
+      until File.directory?(dir)
         stack.push dir
         dir = File.dirname(dir)
       end
@@ -237,7 +237,7 @@ module FileUtils
     fu_output_message "ln#{options[:force] ? ' -f' : ''} #{[src,dest].flatten.join ' '}" if options[:verbose]
     return if options[:noop]
 
-    fu_each_src_dest(src, dest) do |s,d|
+    fu_each_src_dest0(src, dest) do |s,d|
       remove_file d, true if options[:force]
       File.link s, d
     end
@@ -272,7 +272,7 @@ module FileUtils
     fu_output_message "ln -s#{options[:force] ? 'f' : ''} #{[src,dest].flatten.join ' '}" if options[:verbose]
     return if options[:noop]
 
-    fu_each_src_dest(src, dest) do |s,d|
+    fu_each_src_dest0(src, dest) do |s,d|
       remove_file d, true if options[:force]
       File.symlink s, d
     end
@@ -343,7 +343,7 @@ module FileUtils
     return if options[:noop]
 
     fu_each_src_dest(src, dest) do |s,d|
-      if FileTest.directory?(s)
+      if File.directory?(s)
         fu_copy_dir s, d, '.', options[:preserve]
       else
         fu_p_copy s, d, options[:preserve]
@@ -354,10 +354,10 @@ module FileUtils
   def fu_copy_dir( src, dest, rel, preserve ) #:nodoc:
     fu_preserve_attr(preserve, "#{src}/#{rel}", "#{dest}/#{rel}") {|s,d|
         dir = File.expand_path(d)   # to remove '/./'
-        Dir.mkdir dir unless FileTest.directory? dir
+        Dir.mkdir dir unless File.directory?(dir)
     }
     Dir.entries("#{src}/#{rel}").each do |fname|
-      if FileTest.directory? File.join(src,rel,fname)
+      if File.directory?(File.join(src,rel,fname))
         next if /\A\.\.?\z/ === fname
         fu_copy_dir src, dest, "#{rel}/#{fname}", preserve
       else
@@ -437,14 +437,14 @@ module FileUtils
     return if options[:noop]
 
     fu_each_src_dest(src, dest) do |s,d|
-      if cannot_overwrite_file? and FileTest.file?(d)
+      if cannot_overwrite_file? and File.file?(d)
         File.unlink d
       end
 
       begin
         File.rename s, d
       rescue
-        if FileTest.symlink?(s)
+        if File.symlink?(s)
           File.symlink File.readlink(s), dest
           File.unlink s
         else
@@ -574,7 +574,7 @@ module FileUtils
     Dir.foreach(dir) do |file|
       next if /\A\.\.?\z/ === file
       path = "#{dir}/#{file}"
-      if FileTest.directory? path
+      if File.directory?(path)
         remove_dir path, force
       else
         remove_file path, force
@@ -639,7 +639,7 @@ module FileUtils
     return if options[:noop]
 
     fu_each_src_dest(src, dest) do |s,d|
-      unless FileTest.exist?(d) and compare_file(s,d)
+      unless File.exist?(d) and compare_file(s,d)
 	remove_file d, true
 	st = File.stat(s) if options[:preserve]
 	copy_file s, d
@@ -707,15 +707,22 @@ module FileUtils
   end
 
   def fu_list( arg )
-    Array === arg ? arg : [arg]
+    arg.is_a?(Array) ? arg : [arg]
   end
 
   def fu_each_src_dest( src, dest )
-    unless Array === src
+    fu_each_src_dest0(src, dest) do |s, d|
+      raise ArgumentError, "same file: #{s} and #{d}" if fu_same?(s, d)
+      yield s, d
+    end
+  end
+
+  def fu_each_src_dest0( src, dest )
+    unless src.is_a?(Array)
       yield src, fu_dest_filename(src, dest)
     else
       dir = dest
-      # FileTest.directory? dir or raise ArgumentError, "must be dir: #{dir}"
+      #raise ArgumentError, "not a directory: #{dir}" unless File.directory?(dir)
       dir += (dir[-1,1] == '/') ? '' : '/'
       src.each do |fname|
         yield fname, dir + File.basename(fname)
@@ -724,13 +731,45 @@ module FileUtils
   end
 
   def fu_dest_filename( src, dest )
-    if FileTest.directory? dest
+    if File.directory?(dest)
       (dest[-1,1] == '/' ? dest : dest + '/') + File.basename(src)
     else
       dest
     end
   end
 
+  def fu_same?( a, b )
+    fu_resolve_symlink(a) == fu_resolve_symlink(b)
+  end
+
+  def fu_resolve_symlink( path, limit = 128 )
+    raise Errno::ELOOP, "too many levels of symlic links: #{path}" if limit < 0
+    if File.symlink?(path)
+    then fu_resolve_symlink(fu_readlink(File.expand_path(path)), limit-1)
+    else path
+    end
+  end
+
+  def fu_readlink( path )
+    dest = File.readlink(path)
+    if absolute_path?(dest)
+    then dest
+    else File.dirname(File.expand_path(path)) + '/' + dest
+    path = File.readlink(path)
+    end
+  end
+
+  def absolute_path?( path )
+    if have_drive_letter?
+    then %r<\A([a-z]:)?/> === path
+    else %r<\A/> === path
+    end
+  end
+
+  def have_drive_letter?
+    File::ALT_SEPARATOR ? true : false
+  end
+
   def fu_stream_blksize( *streams )
     streams.each do |s|
       next unless s.respond_to?(:stat)
@@ -750,7 +789,7 @@ module FileUtils
   end
 
   def fu_update_option( args, new )
-    if Hash === args.last
+    if args.last.is_a?(Hash)
       args.last.update new
     else
       args.push new
@@ -836,7 +875,7 @@ module FileUtils
     @fileutils_nowrite = true
 
     FileUtils::OPT_TABLE.each do |name, opts|
-      next unless opts.include? 'noop'
+      next unless opts.include?('noop')
       module_eval(<<-EOS, __FILE__, __LINE__ + 1)
         def #{name}( *args )
           unless defined?(@fileutils_nowrite)

test/fileutils/fileasserts.rb

@@ -14,26 +14,34 @@ module Test
         }
       end
 
-      def assert_file_exist( file )
+      def assert_file_exist( path )
         _wrap_assertion {
-          assert_block("file not exist: #{file}") {
+          assert_block("file not exist: #{path}") {
-            File.exist?(file)
+            File.exist?(path)
           }
         }
       end
 
-      def assert_file_not_exist( file )
+      def assert_file_not_exist( path )
         _wrap_assertion {
-          assert_block("file not exist: #{file}") {
+          assert_block("file not exist: #{path}") {
-            not File.exist?(file)
+            not File.exist?(path)
           }
         }
       end
 
-      def assert_is_directory( file )
+      def assert_directory( path )
         _wrap_assertion {
-          assert_block("is not directory: #{file}") {
+          assert_block("is not directory: #{path}") {
-            File.directory?(file)
+            File.directory?(path)
+          }
+        }
+      end
+
+      def assert_symlink( path )
+        _wrap_assertion {
+          assert_block("is no symlink: #{path}") {
+            File.symlink?(path)
           }
         }
       end

test/fileutils/test_fileutils.rb

@@ -142,6 +142,22 @@ end
       assert_equal a.uid, b.uid
       assert_equal a.gid, b.gid
     end
+
+    # src==dest
+    touch 'tmp/cptmp'
+    assert_raises(ArgumentError) {
+      cp 'tmp/cptmp', 'tmp/cptmp'
+    }
+if have_symlink?
+    File.symlink 'tmp/cptmp', 'tmp/cptmp_symlink'
+    assert_raises(ArgumentError) {
+      cp 'tmp/cptmp', 'tmp/cptmp_symlink'
+    }
+    File.symlink 'tmp/symlink', 'tmp/symlink'
+    assert_raises(Errno::ELOOP) {
+      cp 'tmp/symlink', 'tmp/symlink'
+    }
+end
   end
 
   def test_cp_r
@@ -157,6 +173,22 @@ end
       mv 'tmp/mvsrc', 'tmp/mvdest'
       assert_same_file fname, 'tmp/mvdest'
     end
+
+    # src==dest
+    touch 'tmp/cptmp'
+    assert_raises(ArgumentError) {
+      mv 'tmp/cptmp', 'tmp/cptmp'
+    }
+if have_symlink?
+    File.symlink 'tmp/cptmp', 'tmp/cptmp_symlink'
+    assert_raises(ArgumentError) {
+      mv 'tmp/cptmp', 'tmp/cptmp_symlink'
+    }
+    File.symlink 'tmp/symlink', 'tmp/symlink'
+    assert_raises(Errno::ELOOP) {
+      mv 'tmp/symlink', 'tmp/symlink'
+    }
+end
   end
 
   def test_rm
@@ -249,6 +281,22 @@ end
     TARGETS.each do |fname|
       File.unlink 'tmp/' + File.basename(fname)
     end
+
+    # src==dest
+    touch 'tmp/cptmp'
+    assert_raises(Errno::EEXIST) {
+      ln 'tmp/cptmp', 'tmp/cptmp'
+    }
+if have_symlink?
+    File.symlink 'tmp/cptmp', 'tmp/cptmp_symlink'
+    assert_raises(Errno::EEXIST) {
+      ln 'tmp/cptmp', 'tmp/cptmp_symlink'
+    }
+    File.symlink 'tmp/symlink', 'tmp/symlink'
+    assert_raises(Errno::EEXIST) {
+      ln 'tmp/symlink', 'tmp/symlink'
+    }
+end
   end
 
 if have_symlink?
@@ -259,6 +307,10 @@ if have_symlink?
       assert_equal fname, File.readlink('tmp/lnsdest')
       rm_f 'tmp/lnsdest'
     end
+    assert_nothing_raised {
+      ln_s 'tmp/symlink', 'tmp/symlink'
+    }
+    assert_symlink 'tmp/symlink'
   end
 end
 
@@ -277,15 +329,15 @@ end
   def test_mkdir
     my_rm_rf 'tmpdatadir'
     mkdir 'tmpdatadir'
-    assert_is_directory 'tmpdatadir'
+    assert_directory 'tmpdatadir'
     Dir.rmdir 'tmpdatadir'
 
     mkdir 'tmp/mkdirdest'
-    assert_is_directory 'tmp/mkdirdest'
+    assert_directory 'tmp/mkdirdest'
     Dir.rmdir 'tmp/mkdirdest'
 
     mkdir 'tmp/tmp', :mode => 0700
-    assert_is_directory 'tmp/tmp'
+    assert_directory 'tmp/tmp'
     assert_equal 0700, (File.stat('tmp/tmp').mode & 0777) unless windows?
     Dir.rmdir 'tmp/tmp'
   end
@@ -307,7 +359,7 @@ end
     rm_rf 'tmpdir'
     dirs.each do |d|
       mkdir_p d
-      assert_is_directory d
+      assert_directory d
       assert_file_not_exist "#{d}/a"
       assert_file_not_exist "#{d}/b"
       assert_file_not_exist "#{d}/c"
@@ -315,13 +367,13 @@ end
     end
     dirs.each do |d|
       mkdir_p d
-      assert_is_directory d
+      assert_directory d
     end
     rm_rf 'tmpdir'
 
     mkdir_p 'tmp/tmp/tmp', :mode => 0700
-    assert_is_directory 'tmp/tmp'
+    assert_directory 'tmp/tmp'
-    assert_is_directory 'tmp/tmp/tmp'
+    assert_directory 'tmp/tmp/tmp'
     assert_equal 0700, (File.stat('tmp/tmp').mode & 0777) unless windows?
     assert_equal 0700, (File.stat('tmp/tmp/tmp').mode & 0777) unless windows?
     rm_rf 'tmp/tmp'
@@ -353,6 +405,22 @@ end
 
     File.unlink 'tmp/aaa'
     File.unlink 'tmp/bbb'
+
+    # src==dest
+    touch 'tmp/cptmp'
+    assert_raises(ArgumentError) {
+      install 'tmp/cptmp', 'tmp/cptmp'
+    }
+if have_symlink?
+    File.symlink 'tmp/cptmp', 'tmp/cptmp_symlink'
+    assert_raises(ArgumentError) {
+      install 'tmp/cptmp', 'tmp/cptmp_symlink'
+    }
+    File.symlink 'tmp/symlink', 'tmp/symlink'
+    assert_raises(Errno::ELOOP) {
+      install 'tmp/symlink', 'tmp/symlink'
+    }
+end
   end
 
 end