Author: bobby Date: Thu Dec 20 16:19:48 2012 New Revision: 1424566 URL: http://svn.apache.org/viewvc?rev=1424566&view=rev Log: HADOOP-9105. FsShell -moveFromLocal erroneously fails (daryn via bobby)
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/MoveCommands.java hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1424566&r1=1424565&r2=1424566&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Thu Dec 20 16:19:48 2012 @@ -1224,6 +1224,8 @@ Release 0.23.6 - UNRELEASED HADOOP-9038. unit-tests for AllocatorPerContext.PathIterator (Ivan A. Veselovsky via bobby) + HADOOP-9105. FsShell -moveFromLocal erroneously fails (daryn via bobby) + Release 0.23.5 - UNRELEASED INCOMPATIBLE CHANGES Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java?rev=1424566&r1=1424565&r2=1424566&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java Thu Dec 20 16:19:48 2012 @@ -311,6 +311,7 @@ abstract public class Command extends Co if (recursive && item.stat.isDirectory()) { recursePath(item); } + postProcessPath(item); } catch (IOException e) { displayError(e); } @@ -330,6 +331,15 @@ abstract public class Command extends Co } /** + * Hook for commands to implement an operation to be applied on each + * path for the command after being processed successfully + * @param item a {@link PathData} object + * @throws IOException if anything goes wrong... + */ + protected void postProcessPath(PathData item) throws IOException { + } + + /** * Gets the directory listing for a path and invokes * {@link #processPaths(PathData, PathData...)} * @param item {@link PathData} for directory to recurse into Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/MoveCommands.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/MoveCommands.java?rev=1424566&r1=1424565&r2=1424566&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/MoveCommands.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/MoveCommands.java Thu Dec 20 16:19:48 2012 @@ -24,6 +24,7 @@ import java.util.LinkedList; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.fs.PathIOException; +import org.apache.hadoop.fs.PathExistsException; import org.apache.hadoop.fs.shell.CopyCommands.CopyFromLocal; /** Various commands for moving files */ @@ -49,7 +50,21 @@ class MoveCommands { @Override protected void processPath(PathData src, PathData target) throws IOException { - target.fs.moveFromLocalFile(src.path, target.path); + // unlike copy, don't merge existing dirs during move + if (target.exists && target.stat.isDirectory()) { + throw new PathExistsException(target.toString()); + } + super.processPath(src, target); + } + + @Override + protected void postProcessPath(PathData src) throws IOException { + if (!src.fs.delete(src.path, false)) { + // we have no way to know the actual error... + PathIOException e = new PathIOException(src.toString()); + e.setOperation("remove"); + throw e; + } } } @@ -95,4 +110,4 @@ class MoveCommands { } } } -} \ No newline at end of file +} Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java?rev=1424566&r1=1424565&r2=1424566&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java Thu Dec 20 16:19:48 2012 @@ -357,6 +357,66 @@ public class TestFsShellCopy { assertEquals(0, exit); assertEquals("f1\ndf1\ndf2\ndf3\nf2\n", readFile("out")); } + + + @Test + public void testMoveFileFromLocal() throws Exception { + Path testRoot = new Path(testRootDir, "testPutFile"); + lfs.delete(testRoot, true); + lfs.mkdirs(testRoot); + + Path target = new Path(testRoot, "target"); + Path srcFile = new Path(testRoot, new Path("srcFile")); + lfs.createNewFile(srcFile); + + int exit = shell.run(new String[]{ + "-moveFromLocal", srcFile.toString(), target.toString() }); + assertEquals(0, exit); + assertFalse(lfs.exists(srcFile)); + assertTrue(lfs.exists(target)); + assertTrue(lfs.isFile(target)); + } + + @Test + public void testMoveDirFromLocal() throws Exception { + Path testRoot = new Path(testRootDir, "testPutDir"); + lfs.delete(testRoot, true); + lfs.mkdirs(testRoot); + + Path srcDir = new Path(testRoot, "srcDir"); + lfs.mkdirs(srcDir); + Path targetDir = new Path(testRoot, "target"); + + int exit = shell.run(new String[]{ + "-moveFromLocal", srcDir.toString(), targetDir.toString() }); + assertEquals(0, exit); + assertFalse(lfs.exists(srcDir)); + assertTrue(lfs.exists(targetDir)); + } + + @Test + public void testMoveDirFromLocalDestExists() throws Exception { + Path testRoot = new Path(testRootDir, "testPutDir"); + lfs.delete(testRoot, true); + lfs.mkdirs(testRoot); + + Path srcDir = new Path(testRoot, "srcDir"); + lfs.mkdirs(srcDir); + Path targetDir = new Path(testRoot, "target"); + lfs.mkdirs(targetDir); + + int exit = shell.run(new String[]{ + "-moveFromLocal", srcDir.toString(), targetDir.toString() }); + assertEquals(0, exit); + assertFalse(lfs.exists(srcDir)); + assertTrue(lfs.exists(new Path(targetDir, srcDir.getName()))); + + lfs.mkdirs(srcDir); + exit = shell.run(new String[]{ + "-moveFromLocal", srcDir.toString(), targetDir.toString() }); + assertEquals(1, exit); + assertTrue(lfs.exists(srcDir)); + } private void createFile(Path ... paths) throws IOException { for (Path path : paths) {