Flip logic to try native first

This uses more compatible native stat logic for comparing two files, only falling back on NIO options if native logic cannot be used. Fixes jruby#7151
headius · Feb 13, 2024 · 839b847 · 839b847
1 parent cb722b8
commit 839b847
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 16 deletions.
diff --git a/core/src/main/java/org/jruby/RubyFileTest.java b/core/src/main/java/org/jruby/RubyFileTest.java
@@ -155,31 +155,32 @@ public static IRubyObject grpowned_p(IRubyObject recv, IRubyObject filename) {
     }
 
     @JRubyMethod(name = "identical?", module = true)
-    public static IRubyObject identical_p(IRubyObject recv, IRubyObject filename1, IRubyObject filename2) {
-        Ruby runtime = recv.getRuntime();
+    public static IRubyObject identical_p(ThreadContext context, IRubyObject recv, IRubyObject filename1, IRubyObject filename2) {
+        Ruby runtime = context.runtime;
+
         FileResource file1 = fileResource(filename1);
         FileResource file2 = fileResource(filename2);
 
-        // try NIO2 first to support more platforms
+        // Try posix first
+        if (!Platform.IS_WINDOWS && runtime.getPosix().isNative()) {
+            FileStat stat1 = file1.stat();
+            FileStat stat2 = file2.stat();
+
+            return runtime.newBoolean(stat1 != null && stat2 != null && stat1.isIdentical(stat2));
+        }
+
+        // fallback to NIO2 to support more platforms
         if (file1.exists() && file2.exists()) {
             try {
                 Path canon1 = new File(file1.absolutePath()).getCanonicalFile().toPath();
                 Path canon2 = new File(file2.absolutePath()).getCanonicalFile().toPath();
                 return runtime.newBoolean(Files.isSameFile(canon1, canon2));
             } catch (IOException canonicalizationError) {
-                // fall through to native logic
+                // fall through
             }
         }
 
-        // if we can't use NIO2 and we're on Windows or don't have native posix, just return false?
-        if (Platform.IS_WINDOWS || !runtime.getPosix().isNative()) {
-            return runtime.getFalse();
-        }
-
-        FileStat stat1 = file1.stat();
-        FileStat stat2 = file2.stat();
-
-        return runtime.newBoolean(stat1 != null && stat2 != null && stat1.isIdentical(stat2));
+        return context.fals;
     }
 
     @JRubyMethod(name = "owned?", module = true)
@@ -383,8 +384,8 @@ public static IRubyObject grpowned_p(IRubyObject recv, IRubyObject filename) {
         }
 
         @JRubyMethod(name = "identical?")
-        public static IRubyObject identical_p(IRubyObject recv, IRubyObject filename1, IRubyObject filename2) {
-            return RubyFileTest.identical_p(recv, filename1, filename2);
+        public static IRubyObject identical_p(ThreadContext context, IRubyObject recv, IRubyObject filename1, IRubyObject filename2) {
+            return RubyFileTest.identical_p(context, recv, filename1, filename2);
         }
 
         @JRubyMethod(name = "owned?")
@@ -456,6 +457,11 @@ public static IRubyObject worldReadable(ThreadContext context, IRubyObject recv,
         public static IRubyObject worldWritable(ThreadContext context, IRubyObject recv, IRubyObject filename) {
             return RubyFileTest.worldWritable(context, recv, filename);
         }
+
+        @Deprecated
+        public static IRubyObject identical_p(IRubyObject recv, IRubyObject filename1, IRubyObject filename2) {
+            return RubyFileTest.identical_p(recv, filename1, filename2);
+        }
     }
 
     private static RubyFileStat getRubyFileStat(ThreadContext context, IRubyObject filename) {
@@ -503,4 +509,9 @@ private static void noFileError(IRubyObject filename) {
         throw filename.getRuntime().newErrnoENOENTError("No such file or directory - " +
                 filename.convertToString());
     }
+
+    @Deprecated
+    public static IRubyObject identical_p(IRubyObject recv, IRubyObject filename1, IRubyObject filename2) {
+        return identical_p(recv.getRuntime().getCurrentContext(), recv, filename1, filename2);
+    }
 }
diff --git a/core/src/main/java/org/jruby/RubyKernel.java b/core/src/main/java/org/jruby/RubyKernel.java
@@ -1817,7 +1817,7 @@ private static IRubyObject testCommon(ThreadContext context, IRubyObject recv, i
         case '>': // ?>  | boolean | True if the modification time of file1 is after that of file2
             return context.runtime.newFileStat(arg1.convertToString().toString(), false).mtimeGreaterThan(arg2);
         case '-': // ?-  | boolean | True if file1 and file2 are identical
-            return RubyFileTest.identical_p(recv, arg1, arg2);
+            return RubyFileTest.identical_p(context, recv, arg1, arg2);
         default:
             throw new InternalError("unreachable code reached!");
         }