diff --git a/CHANGES.txt b/CHANGES.txt index f132f891f4..306ee00460 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -142,6 +142,9 @@ Trunk (unreleased changes) HADOOP-6670. Use the UserGroupInformation's Subject as the criteria for equals and hashCode. (Owen O'Malley and Kan Zhang via ddas) + HADOOP-6536. Fixes FileUtil.fullyDelete() not to delete the contents of + the sym-linked directory. (Ravi Gummadi via amareshwari) + Release 0.21.0 - Unreleased INCOMPATIBLE CHANGES diff --git a/src/java/org/apache/hadoop/fs/FileUtil.java b/src/java/org/apache/hadoop/fs/FileUtil.java index d553750458..5492284b3d 100644 --- a/src/java/org/apache/hadoop/fs/FileUtil.java +++ b/src/java/org/apache/hadoop/fs/FileUtil.java @@ -79,8 +79,22 @@ public static Path[] stat2Paths(FileStatus[] stats, Path path) { /** * Delete a directory and all its contents. If * we return false, the directory may be partially-deleted. + * (1) If dir is symlink to a file, the symlink is deleted. The file pointed + * to by the symlink is not deleted. + * (2) If dir is symlink to a directory, symlink is deleted. The directory + * pointed to by symlink is not deleted. + * (3) If dir is a normal file, it is deleted. + * (4) If dir is a normal directory, then dir and all its contents recursively + * are deleted. */ public static boolean fullyDelete(File dir) throws IOException { + if (dir.delete()) { + // dir is (a) normal file, (b) symlink to a file, (c) empty directory or + // (d) symlink to a directory + return true; + } + + // handle nonempty directory deletion if (!fullyDeleteContents(dir)) { return false; } @@ -90,6 +104,8 @@ public static boolean fullyDelete(File dir) throws IOException { /** * Delete the contents of a directory, not the directory itself. If * we return false, the directory may be partially-deleted. + * If dir is a symlink to a directory, all the contents of the actual + * directory pointed to by dir will be deleted. */ public static boolean fullyDeleteContents(File dir) throws IOException { boolean deletionSucceeded = true; @@ -97,13 +113,13 @@ public static boolean fullyDeleteContents(File dir) throws IOException { if (contents != null) { for (int i = 0; i < contents.length; i++) { if (contents[i].isFile()) { - if (!contents[i].delete()) { + if (!contents[i].delete()) {// normal file or symlink to another file deletionSucceeded = false; continue; // continue deletion of other files/dirs under dir } } else { - //try deleting the directory - // this might be a symlink + // Either directory or symlink to another directory. + // Try deleting the directory as this might be a symlink boolean b = false; b = contents[i].delete(); if (b){ diff --git a/src/test/core/org/apache/hadoop/fs/TestFileUtil.java b/src/test/core/org/apache/hadoop/fs/TestFileUtil.java index 886ef81fd6..ccb237c117 100644 --- a/src/test/core/org/apache/hadoop/fs/TestFileUtil.java +++ b/src/test/core/org/apache/hadoop/fs/TestFileUtil.java @@ -95,6 +95,69 @@ public void testFullyDelete() throws IOException { validateTmpDir(); } + /** + * Tests if fullyDelete deletes + * (a) symlink to file only and not the file pointed to by symlink. + * (b) symlink to dir only and not the dir pointed to by symlink. + * @throws IOException + */ + @Test + public void testFullyDeleteSymlinks() throws IOException { + setupDirs(); + + File link = new File(del, LINK); + Assert.assertEquals(5, del.list().length); + // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not + // delete contents of tmp. See setupDirs for details. + boolean ret = FileUtil.fullyDelete(link); + Assert.assertTrue(ret); + Assert.assertFalse(link.exists()); + Assert.assertEquals(4, del.list().length); + validateTmpDir(); + + File linkDir = new File(del, "tmpDir"); + // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not + // delete contents of tmp. See setupDirs for details. + ret = FileUtil.fullyDelete(linkDir); + Assert.assertTrue(ret); + Assert.assertFalse(linkDir.exists()); + Assert.assertEquals(3, del.list().length); + validateTmpDir(); + } + + /** + * Tests if fullyDelete deletes + * (a) dangling symlink to file properly + * (b) dangling symlink to directory properly + * @throws IOException + */ + @Test + public void testFullyDeleteDanglingSymlinks() throws IOException { + setupDirs(); + // delete the directory tmp to make tmpDir a dangling link to dir tmp and + // to make y as a dangling link to file tmp/x + boolean ret = FileUtil.fullyDelete(tmp); + Assert.assertTrue(ret); + Assert.assertFalse(tmp.exists()); + + // dangling symlink to file + File link = new File(del, LINK); + Assert.assertEquals(5, del.list().length); + // Even though 'y' is dangling symlink to file tmp/x, fullyDelete(y) + // should delete 'y' properly. + ret = FileUtil.fullyDelete(link); + Assert.assertTrue(ret); + Assert.assertEquals(4, del.list().length); + + // dangling symlink to directory + File linkDir = new File(del, "tmpDir"); + // Even though tmpDir is dangling symlink to tmp, fullyDelete(tmpDir) should + // delete tmpDir properly. + ret = FileUtil.fullyDelete(linkDir); + Assert.assertTrue(ret); + Assert.assertEquals(3, del.list().length); + } + @Test public void testFullyDeleteContents() throws IOException { setupDirs();