delete broken symbolic links. PR 41285

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@693071 13f79535-47bb-0310-9956-ffa450edef68

WHATSNEW

@@ -204,6 +204,9 @@ Fixed bugs:
    the link's target.
    Bugzilla Report 41525.
 
+ * <delete file="..."> failed to delete broken symbolic links.
+   Bugzilla Report 41285.
+
 Other changes:
 --------------
 

src/etc/testcases/taskdefs/optional/unix/symlink.xml

@@ -54,8 +54,8 @@
   </target>
 
   <target name="all"
-	      depends="setup, test-single, test-delete, test-record, test-recreate, teardown"/>
-	
+              depends="setup, test-single, test-delete, test-record, test-recreate, teardown"/>
+        
   <!-- test for action = single -->
   <!-- 
     Creates:
@@ -334,6 +334,22 @@
   </target>
 
 
+  <!-- actually tests the symlink methods in FileUtils, but this
+       testfixture already has all the necessary envirnment in place
+       -->
+  <target name="test-fileutils" depends="setup">
+      <mkdir dir="${tdir}/dir1"/>
+      <mkdir dir="${tdir}/dir2"/>
+      <touch file="${tdir}/file1"/>
+      <touch file="${tdir}/file2"/>
+      <symlink link="${tdir}/dir.there" resource="${tdir}/dir1"/>
+      <symlink link="${tdir}/dir.notthere" resource="${tdir}/dir2"/>
+      <symlink link="${tdir}/file.there" resource="${tdir}/file1"/>
+      <symlink link="${tdir}/file.notthere" resource="${tdir}/file2"/>
+      <delete dir="${tdir}/dir2"/>
+      <delete file="${tdir}/file2"/>
+  </target>
+
 <!-- CALL THIS to clean things up afterwards -->
 
   <target name="teardown">

src/main/org/apache/tools/ant/taskdefs/Delete.java

@@ -57,6 +57,7 @@ import org.apache.tools.ant.types.selectors.FilenameSelector;
 import org.apache.tools.ant.types.selectors.MajoritySelector;
 import org.apache.tools.ant.types.selectors.ContainsRegexpSelector;
 import org.apache.tools.ant.types.selectors.modifiedselector.ModifiedSelector;
+import org.apache.tools.ant.util.FileUtils;
 
 /**
  * Deletes a file or directory, or set of files defined by a fileset.
@@ -112,6 +113,7 @@ public class Delete extends MatchingTask {
     private boolean failonerror = true;
     private boolean deleteOnExit = false;
     private Resources rcs = null;
+    private static FileUtils FILE_UTILS = FileUtils.getFileUtils();
     // CheckStyle:VisibilityModifier ON
 
     /**
@@ -523,6 +525,13 @@ public class Delete extends MatchingTask {
                         handle("Unable to delete file " + file.getAbsolutePath());
                     }
                 }
+            } else if (isDanglingSymlink(file)) {
+                log("Trying to delete file " + file.getAbsolutePath()
+                    + " which looks like a broken symlink.",
+                    quiet ? Project.MSG_VERBOSE : verbosity);
+                if (!delete(file)) {
+                    handle("Unable to delete file " + file.getAbsolutePath());
+                }
             } else {
                 log("Could not find file " + file.getAbsolutePath()
                     + " to delete.", quiet ? Project.MSG_VERBOSE : verbosity);
@@ -530,8 +539,9 @@ public class Delete extends MatchingTask {
         }
 
         // delete the directory
-        if (dir != null && dir.exists() && dir.isDirectory()
+        if (dir != null
             && !usedMatchingTask) {
+            if (dir.exists() && dir.isDirectory()) {
             /*
                If verbosity is MSG_VERBOSE, that mean we are doing
                regular logging (backwards as that sounds).  In that
@@ -543,6 +553,15 @@ public class Delete extends MatchingTask {
                 log("Deleting directory " + dir.getAbsolutePath());
             }
             removeDir(dir);
+            } else if (isDanglingSymlink(dir)) {
+                log("Trying to delete directory " + dir.getAbsolutePath()
+                    + " which looks like a broken symlink.",
+                    quiet ? Project.MSG_VERBOSE : verbosity);
+                if (!delete(dir)) {
+                    handle("Unable to delete directory "
+                           + dir.getAbsolutePath());
+                }
+            }
         }
         Resources resourcesToDelete = new Resources();
         resourcesToDelete.setProject(getProject());
@@ -739,4 +758,16 @@ public class Delete extends MatchingTask {
             }
         }
     }
+
+    private boolean isDanglingSymlink(File f) {
+        try {
+            return FILE_UTILS.isDanglingSymbolicLink(f.getParentFile(),
+                                                     f.getName());
+        } catch (java.io.IOException e) {
+            log("Error while trying to detect " + f.getAbsolutePath()
+                + " as broken symbolic link. " + e.getMessage(),
+                quiet ? Project.MSG_VERBOSE : verbosity);
+            return false;
+        }
+    }
 }

src/main/org/apache/tools/ant/util/FileUtils.java

@@ -1051,6 +1051,47 @@ public class FileUtils {
         return !toTest.getAbsolutePath().equals(toTest.getCanonicalPath());
     }
 
+    /**
+     * Checks whether a given file is a broken symbolic link.
+     *
+     * <p>It doesn't really test for symbolic links but whether Java
+     * reports that the File doesn't exist but its parent's child list
+     * contains it--this may lead to false positives on some
+     * platforms.</p>
+     *
+     * <p>Note that #isSymbolicLink returns false if this method
+     * returns true since Java won't produce a canonical name
+     * different from the abolute one if the link is broken.</p>
+     *
+     * @param parent the parent directory of the file to test
+     * @param name the name of the file to test.
+     *
+     * @return true if the file is a broken symbolic link.
+     * @throws IOException on error.
+     * @since Ant 1.8.0
+     */
+    public boolean isDanglingSymbolicLink(File parent, String name) 
+        throws IOException {
+        File f = null;
+        if (parent == null) {
+            f = new File(name);
+            parent = f.getParentFile();
+            name = f.getName();
+        } else {
+            f = new File(parent, name);
+        }
+        if (!f.exists()) {
+            final String localName = f.getName();
+            String[] c = parent.list(new FilenameFilter() {
+                    public boolean accept(File d, String n) {
+                        return localName.equals(n);
+                    }
+                });
+            return c != null && c.length > 0;
+        }
+        return false;
+    }
+
     /**
      * Removes a leading path from a second path.
      *

src/tests/antunit/taskdefs/delete-test.xml

@@ -67,4 +67,31 @@
 
   </target>
 
+  <target name="checkOs">
+    <condition property="unix">
+      <os family="unix" />
+    </condition>
+  </target>
+
+  <target name="testDanglingSymlinkInDir" if="unix" depends="checkOs,init">
+    <touch file="${output}/foo"/>
+    <symlink link="${existing.dir}/link"
+             resource="${output}/foo"/>
+    <delete file="${output}/foo"/>
+    <delete dir="${existing.dir}"/>
+    <au:assertFileDoesntExist file="${existing.dir}" />
+  </target>
+
+  <target name="testDanglingSymlink" if="unix" depends="checkOs,init">
+    <touch file="${output}/foo"/>
+    <symlink link="${output}/link"
+             resource="${output}/foo"/>
+    <delete file="${output}/foo"/>
+    <delete file="${output}/link"/>
+
+    <!-- since File.exists returns false for dangling links, recreate
+         the file so that assertFileDoesntExist can actually work -->
+    <touch file="${output}/foo"/>
+    <au:assertFileDoesntExist file="${output}/link" />
+  </target>
 </project>

src/tests/junit/org/apache/tools/ant/taskdefs/optional/unix/SymlinkTest.java

@@ -33,6 +33,7 @@ import org.apache.tools.ant.taskdefs.condition.Os;
 
 import org.apache.tools.ant.BuildFileTest;
 import org.apache.tools.ant.Project;
+import org.apache.tools.ant.util.FileUtils;
 
 /**
  * Test cases for the Symlink task. Link creation, link deletion, recording
@@ -181,6 +182,103 @@ public class SymlinkTest extends BuildFileTest {
         }
     }
 
+    public void testFileUtilsMethods() throws Exception {
+        if (supportsSymlinks) {
+            executeTarget("test-fileutils");
+            FileUtils fu = FileUtils.getFileUtils();
+
+            java.io.File f = getProject().resolveFile("test-working/file1");
+            assertTrue(f.exists());
+            assertFalse(f.isDirectory());
+            assertTrue(f.isFile());
+            assertFalse(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isSymbolicLink(f.getParentFile(),
+                                          f.getName()));
+            assertFalse(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                  f.getName()));
+
+            f = getProject().resolveFile("test-working/dir1");
+            assertTrue(f.exists());
+            assertTrue(f.isDirectory());
+            assertFalse(f.isFile());
+            assertFalse(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isSymbolicLink(f.getParentFile(),
+                                          f.getName()));
+            assertFalse(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                  f.getName()));
+
+            f = getProject().resolveFile("test-working/file2");
+            assertFalse(f.exists());
+            assertFalse(f.isDirectory());
+            assertFalse(f.isFile());
+            assertFalse(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isSymbolicLink(f.getParentFile(),
+                                          f.getName()));
+            assertFalse(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                  f.getName()));
+
+            f = getProject().resolveFile("test-working/dir2");
+            assertFalse(f.exists());
+            assertFalse(f.isDirectory());
+            assertFalse(f.isFile());
+            assertFalse(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isSymbolicLink(f.getParentFile(),
+                                          f.getName()));
+            assertFalse(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                  f.getName()));
+
+
+            f = getProject().resolveFile("test-working/file.there");
+            assertTrue(f.exists());
+            assertFalse(f.isDirectory());
+            assertTrue(f.isFile());
+            assertTrue(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertTrue(fu.isSymbolicLink(f.getParentFile(),
+                                         f.getName()));
+            assertFalse(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                  f.getName()));
+
+            f = getProject().resolveFile("test-working/dir.there");
+            assertTrue(f.exists());
+            assertTrue(f.isDirectory());
+            assertFalse(f.isFile());
+            assertTrue(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertTrue(fu.isSymbolicLink(f.getParentFile(),
+                                         f.getName()));
+            assertFalse(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                  f.getName()));
+
+            f = getProject().resolveFile("test-working/file.notthere");
+            assertFalse(f.exists());
+            assertFalse(f.isDirectory());
+            assertFalse(f.isFile());
+            assertFalse(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isSymbolicLink(f.getParentFile(),
+                                          f.getName()));
+            assertTrue(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertTrue(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                 f.getName()));
+
+            f = getProject().resolveFile("test-working/dir.notthere");
+            assertFalse(f.exists());
+            assertFalse(f.isDirectory());
+            assertFalse(f.isFile());
+            assertFalse(fu.isSymbolicLink(null, f.getAbsolutePath()));
+            assertFalse(fu.isSymbolicLink(f.getParentFile(),
+                                          f.getName()));
+            assertTrue(fu.isDanglingSymbolicLink(null, f.getAbsolutePath()));
+            assertTrue(fu.isDanglingSymbolicLink(f.getParentFile(),
+                                                 f.getName()));
+
+        }
+    }
+
     public void tearDown() {
         if (supportsSymlinks) {
             executeTarget("teardown");