Merge pull request #5168 from jruby/fu_mv_exdev

Remove hack for FileUtils.mv due to incorrect error raised from File.rename
jruby · May 13, 2018 · be4a079 · be4a079
2 parents 30e9fa4 + da420c9
commit be4a079
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 11 deletions.
diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -3814,6 +3814,10 @@ public RaiseException newErrnoEMSGSIZEError() {
         return newRaiseException(getErrno().getClass("EMSGSIZE"), null);
     }
 
+    public RaiseException newErrnoEXDEVError(String message) {
+        return newRaiseException(getErrno().getClass("EXDEV"), message);
+    }
+
     public RaiseException newIndexError(String message) {
         return newRaiseException(getIndexError(), message);
     }

diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java
@@ -65,6 +65,7 @@
 import java.net.URL;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
+import java.nio.file.FileStore;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.Path;
@@ -1083,6 +1084,19 @@ public static IRubyObject rename(ThreadContext context, IRubyObject recv, IRubyO
             newFile.delete();
         }
 
+        // Check if source file and dest parent are on same filesystem or raise EXDEV
+        Path oldPath = Paths.get(oldFile.toURI());
+        Path destPath = Paths.get(dest.getAbsolutePath());
+        try {
+            FileStore oldStore = Files.getFileStore(oldPath);
+            FileStore destStore = Files.getFileStore(destPath.getParent());
+            if (!oldStore.equals(destStore)) {
+                throw runtime.newErrnoEXDEVError("(" + oldFile + ", " + dest + ")");
+            }
+        } catch (IOException ioe) {
+            throw Helpers.newIOErrorFromException(runtime, ioe);
+        }
+
         if (oldFile.renameTo(dest)) { // try to rename one more time
             return RubyFixnum.zero(runtime);
         }

diff --git a/core/src/main/java/org/jruby/runtime/backtrace/TraceType.java b/core/src/main/java/org/jruby/runtime/backtrace/TraceType.java
@@ -402,7 +402,7 @@ protected static String printBacktraceJRuby(RubyException exception, boolean con
         String message;
         try {
             message = exception.callMethod(context, "message").toString();
-        } catch (org.jruby.exceptions.Exception _) {
+        } catch (org.jruby.exceptions.Exception unused) {
             message = exception.message(context).toString();
         }
         if (exception.getMetaClass() == runtime.getRuntimeError() && message.length() == 0) {

diff --git a/lib/ruby/stdlib/fileutils.rb b/lib/ruby/stdlib/fileutils.rb
@@ -469,7 +469,7 @@ def mv(src, dest, force: nil, noop: nil, verbose: nil, secure: nil)
         end
         begin
           File.rename s, d
-        rescue *MV_RESCUES
+        rescue EXDEV
           copy_entry s, d, true
           if secure
             remove_entry_secure s, force
@@ -484,15 +484,6 @@ def mv(src, dest, force: nil, noop: nil, verbose: nil, secure: nil)
   end
   module_function :mv
 
-  # JRuby raises EACCES because JDK reports errors differently
-  MV_RESCUES = begin
-    if RUBY_ENGINE == 'jruby'
-      [Errno::EXDEV, Errno::EACCES]
-    else
-      [Errno::EXDEV]
-    end
-  end
-
   alias move mv
   module_function :move