diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 4d6ebaf8a8c..334f967600e 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -281,6 +281,9 @@ Release 0.23.2 - UNRELEASED HADOOP-8175. Add -p option to mkdir in FsShell. (Daryn Sharp via szetszwo) + HADOOP-8176. Disambiguate the destination of FsShell copies (Daryn Sharp + via bobby) + Release 0.23.1 - 2012-02-17 INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java index 82c4391f36c..a18b6e17368 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java @@ -200,6 +200,8 @@ abstract class CommandWithDestination extends FsCommand { // a child path if dst is a dir; after recursion, it's always a dir if ((getDepth() > 0) || (dst.exists && dst.stat.isDirectory())) { target = dst.getPathDataForChild(src); + } else if (dst.representsDirectory()) { // see if path looks like a dir + target = dst.getPathDataForChild(src); } else { target = dst; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java index cb481c8e0ee..1e3dfd2c4a2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java @@ -188,12 +188,21 @@ public class PathData implements Comparable { * @throws IOException upon unexpected error */ public boolean parentExists() throws IOException { + return representsDirectory() + ? fs.exists(path) : fs.exists(path.getParent()); + } + + /** + * Check if the path represents a directory as determined by the basename + * being "." or "..", or the path ending with a directory separator + * @return boolean if this represents a directory + */ + public boolean representsDirectory() { String uriPath = uri.getPath(); String name = uriPath.substring(uriPath.lastIndexOf("/")+1); // Path will munch off the chars that indicate a dir, so there's no way // to perform this test except by examining the raw basename we maintain - return (name.isEmpty() || name.equals(".") || name.equals("..")) - ? fs.exists(path) : fs.exists(path.getParent()); + return (name.isEmpty() || name.equals(".") || name.equals("..")); } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java index 1ba1f915552..5b4f585c19d 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java @@ -225,6 +225,54 @@ public class TestFsShellCopy { assertEquals(exitCode, gotExit); } + @Test + public void testRepresentsDir() throws Exception { + Path subdirDstPath = new Path(dstPath, srcPath.getName()); + String argv[] = null; + lfs.delete(dstPath, true); + assertFalse(lfs.exists(dstPath)); + + argv = new String[]{ "-put", srcPath.toString(), dstPath.toString() }; + assertEquals(0, shell.run(argv)); + assertTrue(lfs.exists(dstPath) && lfs.isFile(dstPath)); + + lfs.delete(dstPath, true); + assertFalse(lfs.exists(dstPath)); + + // since dst path looks like a dir, it should not copy the file and + // rename it to what looks like a directory + lfs.delete(dstPath, true); // make copy fail + for (String suffix : new String[]{ "/", "/." } ) { + argv = new String[]{ + "-put", srcPath.toString(), dstPath.toString()+suffix }; + assertEquals(1, shell.run(argv)); + assertFalse(lfs.exists(dstPath)); + assertFalse(lfs.exists(subdirDstPath)); + } + + // since dst path looks like a dir, it should not copy the file and + // rename it to what looks like a directory + for (String suffix : new String[]{ "/", "/." } ) { + // empty out the directory and create to make copy succeed + lfs.delete(dstPath, true); + lfs.mkdirs(dstPath); + argv = new String[]{ + "-put", srcPath.toString(), dstPath.toString()+suffix }; + assertEquals(0, shell.run(argv)); + assertTrue(lfs.exists(subdirDstPath)); + assertTrue(lfs.isFile(subdirDstPath)); + } + + // ensure .. is interpreted as a dir + String dotdotDst = dstPath+"/foo/.."; + lfs.delete(dstPath, true); + lfs.mkdirs(new Path(dstPath, "foo")); + argv = new String[]{ "-put", srcPath.toString(), dotdotDst }; + assertEquals(0, shell.run(argv)); + assertTrue(lfs.exists(subdirDstPath)); + assertTrue(lfs.isFile(subdirDstPath)); + } + @Test public void testCopyMerge() throws Exception { Path root = new Path(testRootDir, "TestMerge");