diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 2447514b346..5b912567b21 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -166,6 +166,9 @@ Release 0.23.2 - UNRELEASED HADOOP-8050. Deadlock in metrics. (Kihwal Lee via mattf) + HADOOP-8131. FsShell put doesn't correctly handle a non-existent dir + (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 8a0c91a6da5..82c4391f36c 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 @@ -112,10 +112,12 @@ abstract class CommandWithDestination extends FsCommand { if (!dst.stat.isDirectory()) { throw new PathIsNotDirectoryException(dst.toString()); } - } else { - if (dst.exists && !dst.stat.isDirectory() && !overwrite) { + } else if (dst.exists) { + if (!dst.stat.isDirectory() && !overwrite) { throw new PathExistsException(dst.toString()); } + } else if (!dst.parentExists()) { + throw new PathNotFoundException(dst.toString()); } super.processArguments(args); } 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 826f20685e3..850193c2883 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 @@ -181,6 +181,20 @@ public class PathData { return tmpFile; } + /** + * Test if the parent directory exists + * @return boolean indicating parent exists + * @throws IOException upon unexpected error + */ + public boolean parentExists() throws IOException { + 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()); + } + /** * Returns a list of PathData objects of the items contained in the given * directory. 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 997f9fa6ef1..796cec3d828 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 @@ -38,8 +38,10 @@ public class TestFsShellCopy { conf = new Configuration(); shell = new FsShell(conf); lfs = FileSystem.getLocal(conf); - testRootDir = new Path( - System.getProperty("test.build.data","test/build/data"), "testShellCopy"); + testRootDir = lfs.makeQualified(new Path( + System.getProperty("test.build.data","test/build/data"), + "testShellCopy")); + lfs.mkdirs(testRootDir); srcPath = new Path(testRootDir, "srcFile"); dstPath = new Path(testRootDir, "dstFile"); @@ -94,4 +96,138 @@ public class TestFsShellCopy { private void shellRun(int n, String ... args) throws Exception { assertEquals(n, shell.run(args)); } + + @Test + public void testCopyFileFromLocal() throws Exception { + Path testRoot = new Path(testRootDir, "testPutFile"); + lfs.delete(testRoot, true); + lfs.mkdirs(testRoot); + + Path targetDir = new Path(testRoot, "target"); + Path filePath = new Path(testRoot, new Path("srcFile")); + lfs.create(filePath).close(); + checkPut(filePath, targetDir); + } + + @Test + public void testCopyDirFromLocal() throws Exception { + Path testRoot = new Path(testRootDir, "testPutDir"); + lfs.delete(testRoot, true); + lfs.mkdirs(testRoot); + + Path targetDir = new Path(testRoot, "target"); + Path dirPath = new Path(testRoot, new Path("srcDir")); + lfs.mkdirs(dirPath); + lfs.create(new Path(dirPath, "srcFile")).close(); + checkPut(dirPath, targetDir); + } + + private void checkPut(Path srcPath, Path targetDir) + throws Exception { + lfs.delete(targetDir, true); + lfs.mkdirs(targetDir); + lfs.setWorkingDirectory(targetDir); + + final Path dstPath = new Path("path"); + final Path childPath = new Path(dstPath, "childPath"); + lfs.setWorkingDirectory(targetDir); + + // copy to new file, then again + prepPut(dstPath, false, false); + checkPut(0, srcPath, dstPath); + if (lfs.isFile(srcPath)) { + checkPut(1, srcPath, dstPath); + } else { // directory works because it copies into the dir + // clear contents so the check won't think there are extra paths + prepPut(dstPath, true, true); + checkPut(0, srcPath, dstPath); + } + + // copy to non-existent subdir + prepPut(childPath, false, false); + checkPut(1, srcPath, dstPath); + + // copy into dir, then with another name + prepPut(dstPath, true, true); + checkPut(0, srcPath, dstPath); + prepPut(childPath, true, true); + checkPut(0, srcPath, childPath); + + // try to put to pwd with existing dir + prepPut(targetDir, true, true); + checkPut(0, srcPath, null); + prepPut(targetDir, true, true); + checkPut(0, srcPath, new Path(".")); + + // try to put to pwd with non-existent cwd + prepPut(dstPath, false, true); + lfs.setWorkingDirectory(dstPath); + checkPut(1, srcPath, null); + prepPut(dstPath, false, true); + checkPut(1, srcPath, new Path(".")); + } + + private void prepPut(Path dst, boolean create, + boolean isDir) throws IOException { + lfs.delete(dst, true); + assertFalse(lfs.exists(dst)); + if (create) { + if (isDir) { + lfs.mkdirs(dst); + assertTrue(lfs.isDirectory(dst)); + } else { + lfs.mkdirs(new Path(dst.getName())); + lfs.create(dst).close(); + assertTrue(lfs.isFile(dst)); + } + } + } + + private void checkPut(int exitCode, Path src, Path dest) throws Exception { + String argv[] = null; + if (dest != null) { + argv = new String[]{ "-put", src.toString(), pathAsString(dest) }; + } else { + argv = new String[]{ "-put", src.toString() }; + dest = new Path(Path.CUR_DIR); + } + + Path target; + if (lfs.exists(dest)) { + if (lfs.isDirectory(dest)) { + target = new Path(pathAsString(dest), src.getName()); + } else { + target = dest; + } + } else { + target = new Path(lfs.getWorkingDirectory(), dest); + } + boolean targetExists = lfs.exists(target); + Path parent = lfs.makeQualified(target).getParent(); + + System.out.println("COPY src["+src.getName()+"] -> ["+dest+"] as ["+target+"]"); + String lsArgv[] = new String[]{ "-ls", "-R", pathAsString(parent) }; + shell.run(lsArgv); + + int gotExit = shell.run(argv); + + System.out.println("copy exit:"+gotExit); + lsArgv = new String[]{ "-ls", "-R", pathAsString(parent) }; + shell.run(lsArgv); + + if (exitCode == 0) { + assertTrue(lfs.exists(target)); + assertTrue(lfs.isFile(src) == lfs.isFile(target)); + assertEquals(1, lfs.listStatus(lfs.makeQualified(target).getParent()).length); + } else { + assertEquals(targetExists, lfs.exists(target)); + } + assertEquals(exitCode, gotExit); + } + + // path handles "." rather oddly + private String pathAsString(Path p) { + String s = (p == null) ? Path.CUR_DIR : p.toString(); + return s.isEmpty() ? Path.CUR_DIR : s; + } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java index fdef2e73410..e05d00a894a 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java @@ -280,11 +280,15 @@ public class TestFsShellReturnCode { System.setErr(out); final String results; try { + Path tdir = new Path(TEST_ROOT_DIR, "notNullCopy"); + fileSys.delete(tdir, true); + fileSys.mkdirs(tdir); String[] args = new String[3]; args[0] = "-get"; - args[1] = "/invalidPath"; - args[2] = "/test/tmp"; + args[1] = tdir+"/invalidSrc"; + args[2] = tdir+"/invalidDst"; assertTrue("file exists", !fileSys.exists(new Path(args[1]))); + assertTrue("file exists", !fileSys.exists(new Path(args[2]))); int run = shell.run(args); results = bytes.toString(); assertEquals("Return code should be 1", 1, run);