search

palantir / giraffe

Gracefully Integrated Remote Access For Files and Execution
https://palantir.github.io/giraffe/
Apache License 2.0
51 stars 16 forks source link

MoreFiles.copyRecursive loops infinitely until failure when copying a directory into one of its subdirectories #43

Closed ajlake closed 9 years ago

ajlake commented 9 years ago

Repro: Path a = Paths.get("a") // a is an existing directory with contents Path b = Paths.get("a/b") // b does not exist in a MoreFiles.copyRecursive(a, b)

This loops infinitely until failure because the copy generates more content to copy. It also passes both precondition checks in MoreFiles.copyRecursive because "a/b" does not exist AND "a" does exist.

qwertzguy commented 9 years ago

This is the exception when run locally on linux:

Exception in thread "main" java.nio.file.FileSystemException: /home/gaspardvk/tmp/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob/bob: File name too long
    at sun.nio.fs.UnixException.translateToIOException(UnixException.java:91)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
    at sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:383)
    at java.nio.file.Files.createDirectory(Files.java:628)
    at com.palantir.giraffe.file.MoreFiles$CopyVisitor.preVisitDirectory(MoreFiles.java:304)
    at com.palantir.giraffe.file.MoreFiles$CopyVisitor.preVisitDirectory(MoreFiles.java:286)
    at java.nio.file.FileTreeWalker.walk(FileTreeWalker.java:192)
    at java.nio.file.FileTreeWalker.walk(FileTreeWalker.java:199)
        [...]
bluekeyes commented 9 years ago

Any idea what cp -R a a/b does in this case? I'm curious if this is only a problem with the visitor implementation or if this should be check for all implementations.

bluekeyes commented 9 years ago

cp -R a a/b returns an error: cannot copy a directory, "a", into itself, "a/b"

Looks like we should implement the same check for the file tree version.