IO-872: PathUtils.directoryAndFileContentEquals with different FileSystems#732
IO-872: PathUtils.directoryAndFileContentEquals with different FileSystems#732dsmiley wants to merge 3 commits intoapache:masterfrom
Conversation
|
@dsmiley Please see the build failures. |
|
I'll get back to this eventually... I can see a Java 8 issue, and a Windows issue. |
|
OK, sounds good. We'll be here 😉 You might want to change the state of this PR to "Draft". |
| } else if (pathA.getFileSystem().getSeparator().equals(pathB.getFileSystem().getSeparator())) { | ||
| // Separators are the same, so we can use toString comparison | ||
| if (!pathA.toString().equals(pathB.toString())) { | ||
| return false; |
There was a problem hiding this comment.
@dsmiley
This doesn't (based on local testing) when you compare two directories like "foo" and "foo/".
There was a problem hiding this comment.
Hm, the binary search in directoryAndFileContentEquals will also need a custom comparator or you get a ProviderMismatchException like:
java.nio.file.ProviderMismatchException
at com.sun.nio.zipfs.ZipPath.checkPath(ZipPath.java:393)
at com.sun.nio.zipfs.ZipPath.compareTo(ZipPath.java:620)
at com.sun.nio.zipfs.ZipPath.compareTo(ZipPath.java:59)
at java.util.Collections.indexedBinarySearch(Collections.java:228)
at java.util.Collections.binarySearch(Collections.java:215)
at org.apache.commons.io.file.PathUtils.directoryAndFileContentEquals(PathUtils.java:711)
at org.apache.commons.io.file.PathUtils.directoryAndFileContentEquals(PathUtils.java:675)
at org.apache.commons.io.file.PathUtilsContentEqualsTest.testDirectoryAndFileContentEqualsDifferentFileSystems(PathUtilsContentEqualsTest.java:115)
at java.lang.reflect.Method.invoke(Method.java:503)
at java.util.ArrayList.forEach(ArrayList.java:1259)
at java.util.ArrayList.forEach(ArrayList.java:1259)
There was a problem hiding this comment.
Hmm. In my (limited) experience looking at Path with say mac/unix, there's never a trailing slash in path.toString(). (Maybe I could be wrong). Maybe this isn't universally true... which sucks because Path is supposed to be an abstraction
There was a problem hiding this comment.
@dsmiley
It's worse than that: Path implementations are OS specific and separators are not normalized such that, for example, on macOS: Paths.get("\\foo").normalize().equals(Paths.get("/foo").normalize()) returns false.
You also need to account for ZipPath, and those end in a '/' for folders...
|
Hello @dsmiley I think we can close this PR based on the state of git master. |
|
Closing: No feedback in months. |
2 participants
IO-872