Restore <move>'s ability to replace existing files and/or write

into existing directories.
PR: 31031


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

src/etc/testcases/taskdefs/move.xml

@@ -59,38 +59,129 @@
     </move>
   </target>
 
-  <target name="testCompleteDirectoryMoveFileToFile">
-    <mkdir dir="A"/>
-    <touch file="A/1"/>
-    <move file="A" tofile="E" />
-    <fail message="E/1 not available">
+  <target name="testMoveFileAndFileset">
+    <mkdir dir="A" />
+    <touch>
+      <filelist dir="A" files="1,2,3" />
+    </touch>
+    <move todir="E" file="A/1">
+      <fileset dir="A" includes="2,3" />
+    </move>
+    <fail message="A unavailable">
       <condition>
         <not>
-          <available file="E/1" type="file" />
+          <available file="A" type="dir" />
         </not>
       </condition>
     </fail>
-    <fail message="A remains">
+    <fail message="A/1 not moved">
+      <condition>
+        <or>
+          <available file="A/1" type="file" />
+          <not>
+            <available file="E/1" type="file" />
+          </not>
+        </or>
+      </condition>
+    </fail>
+    <fail message="A/2 not moved">
+      <condition>
+        <or>
+          <available file="A/2" type="file" />
+          <not>
+            <available file="E/2" type="file" />
+          </not>
+        </or>
+      </condition>
+    </fail>
+    <fail message="A/3 not moved">
+      <condition>
+        <or>
+          <available file="A/3" type="file" />
+          <not>
+            <available file="E/3" type="file" />
+          </not>
+        </or>
+      </condition>
+    </fail>
+  </target>
+
+  <macrodef name="verifymove">
+    <attribute name="newfile" />
+    <attribute name="olddir" />
+    <sequential>
+        <fail message="@{newfile} not available">
+          <condition>
+            <not>
+              <available file="@{newfile}" type="file" />
+            </not>
+          </condition>
+        </fail>
+        <fail message="@{olddir} remains">
+          <condition>
+            <available file="@{olddir}" type="dir" />
+          </condition>
+        </fail>
+    </sequential>
+  </macrodef>
+
+  <target name="testCompleteDirectoryMoveToExistingDir">
+    <mkdir dir="A" />
+    <touch file="A/1" />
+    <mkdir dir="E" />
+    <touch file="E/2" />
+    <move todir="E">
+      <fileset dir="A" />
+    </move>
+    <verifymove newfile="E/1" olddir="A" />
+    <fail message="E/2 unavailable">
       <condition>
-        <available file="A" type="dir" />
+        <not>
+          <available file="E/2" type="file" />
+        </not>
       </condition>
     </fail>
   </target>
 
+  <target name="testCompleteDirectoryMoveFileToFile">
+    <mkdir dir="A"/>
+    <touch file="A/1"/>
+    <move file="A" tofile="E" />
+    <verifymove newfile="E/1" olddir="A" />
+  </target>
+
   <target name="testCompleteDirectoryMoveFileToDir">
     <mkdir dir="A"/>
     <touch file="A/1"/>
     <move file="A" todir="E" />
-    <fail message="E/A/1 not available">
+    <verifymove newfile="E/A/1" olddir="A" />
+  </target>
+
+  <target name="testCompleteDirectoryMoveFileAndFileset">
+    <mkdir dir="A/1" />
+    <touch file="A/2" />
+    <move file="A/1" todir="E">
+      <fileset dir="A" includes="2" />
+    </move>
+    <fail message="A unavailable">
       <condition>
         <not>
-          <available file="E/A/1" type="file" />
+          <available file="A" type="dir" />
         </not>
       </condition>
     </fail>
-    <fail message="A remains">
+    <fail message="E/1 unavailable">
       <condition>
-        <available file="A" type="dir" />
+        <not>
+          <available file="E/1" type="dir" />
+        </not>
+      </condition>
+    </fail>
+    <fail message="E/2 unavailable">
+      <condition>
+        <not>
+          <available file="E/2" type="file" />
+        </not>
       </condition>
     </fail>
   </target>
@@ -98,8 +189,8 @@
   <target name="testCompleteDirectoryMoveFileToExistingFile">
     <mkdir dir="A"/>
     <touch file="A/1"/>
-    <touch file="B"/>
-    <move file="A" tofile="B" />
+    <touch file="E"/>
+    <move file="A" tofile="E" />
   </target>
 
   <target name="testCompleteDirectoryMoveFileToExistingDir">
@@ -107,6 +198,7 @@
     <touch file="A/1"/>
     <mkdir dir="E"/>
     <move file="A" tofile="E" />
+    <verifymove newfile="E/1" olddir="A" />
   </target>
 
   <target name="testCompleteDirectoryMoveFileToDirWithExistingFile">
@@ -123,6 +215,7 @@
     <mkdir dir="E"/>
     <mkdir dir="E/A"/>
     <move file="A" todir="E" />
+    <verifymove newfile="E/A/1" olddir="A" />
   </target>
 
   <target name="cleanup"> 

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

@@ -73,9 +73,9 @@ public class Copy extends Task {
     protected Hashtable completeDirMap = new Hashtable();
 
     protected Mapper mapperElement = null;
+    protected FileUtils fileUtils;
     private Vector filterChains = new Vector();
     private Vector filterSets = new Vector();
-    private FileUtils fileUtils;
     private String inputEncoding = null;
     private String outputEncoding = null;
     private long granularity = 0;

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

@@ -20,8 +20,10 @@ package org.apache.tools.ant.taskdefs;
 import java.io.File;
 import java.io.IOException;
 import java.util.Enumeration;
-import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.DirectoryScanner;
+import org.apache.tools.ant.types.FileSet;
 import org.apache.tools.ant.types.FilterSet;
 import org.apache.tools.ant.types.FilterSetCollection;
 
@@ -61,55 +63,23 @@ public class Move extends Copy {
         setOverwrite(true);
     }
 
-    /**
-     * Performs the move operation.
-     */
-    public void execute() throws BuildException {
+    // inherit doc
+    protected void validateAttributes() throws BuildException {
         if (file != null && file.isDirectory()) {
-            if (destFile != null && destDir != null) {
-                throw new BuildException("Only one of tofile and todir "
-                                         + "may be set.");
+            if ((destFile != null && destDir != null)
+                || (destFile == null && destDir == null)){
+                throw new BuildException("One and only one of tofile and todir "
+                                         + "must be set.");
             }
+            destFile = (destFile == null)
+                ? new File(destDir, file.getName()) : destFile;
+            destDir = (destDir == null)
+                ? fileUtils.getParentFile(destFile) : destDir;
 
-            if (destFile == null && destDir == null) {
-                throw new BuildException("One of tofile or todir must be set.");
-            }
-
-            destFile = (destFile != null)
-                ? destFile : new File(destDir, file.getName());
-
-            try {
-                boolean renamed = false;
-                log("Moving directory " + file
-                    + " to " + destFile, Project.MSG_INFO);
-                try {
-                    renamed =
-                        renameFile(file, destFile, filtering, forceOverwrite);
-                } catch (IOException eyeOhEx) {
-                    throw new BuildException(eyeOhEx.getMessage());
-                }
-                if (!renamed) {
-                    StringBuffer buf = new StringBuffer(
-                        "Failed to move directory ").append(
-                        file.getAbsolutePath());
-
-                    if ((getFilterChains() != null && getFilterChains().size() > 0)
-                        || (getFilterSets() != null && getFilterSets().size() > 0)
-                        || filtering) {
-                        buf.append(
-                            "; use a fileset to move directories with filtering");
-                    }
-                    throw new BuildException(buf.append('.').toString());
-                }
-            } catch (BuildException e) {
-                if (!failonerror) {
-                    log("Warning: " + e.getMessage(), Project.MSG_ERR);
-                } else {
-                    throw e;
-                }
-            }
+            completeDirMap.put(file, destFile);
+            file = null;
         } else {
-            super.execute();
+            super.validateAttributes();
         }
     }
 
@@ -128,21 +98,35 @@ public class Move extends Copy {
             while (e.hasMoreElements()) {
                 File fromDir = (File) e.nextElement();
                 File toDir = (File) completeDirMap.get(fromDir);
+                boolean renamed = false;
                 try {
                     log("Attempting to rename dir: " + fromDir
                         + " to " + toDir, verbosity);
-                    renameFile(fromDir, toDir, filtering, forceOverwrite);
+                    renamed =
+                        renameFile(fromDir, toDir, filtering, forceOverwrite);
                 } catch (IOException ioe) {
                     String msg = "Failed to rename dir " + fromDir
                         + " to " + toDir
                         + " due to " + ioe.getMessage();
                     throw new BuildException(msg, ioe, getLocation());
                 }
+                if (!renamed) {
+                    FileSet fs = new FileSet();
+                    fs.setProject(getProject());
+                    fs.setDir(fromDir);
+                    addFileset(fs);
+                    DirectoryScanner ds = fs.getDirectoryScanner(getProject());
+                    String[] files = ds.getIncludedFiles();
+                    String[] dirs = ds.getIncludedDirectories();
+                    scan(fromDir, toDir, files, dirs);
+                }
             }
         }
-        if (fileCopyMap.size() > 0) {   // files to move
-            log("Moving " + fileCopyMap.size() + " files to "
-                + destDir.getAbsolutePath());
+        int moveCount = fileCopyMap.size();
+        if (moveCount > 0) {   // files to move
+            log("Moving " + moveCount + " file"
+                + ((moveCount == 1) ? "" : "s")
+                + " to " + destDir.getAbsolutePath());
 
             Enumeration e = fileCopyMap.keys();
             while (e.hasMoreElements()) {
@@ -325,6 +309,15 @@ public class Move extends Copy {
      * @param d the directory to delete
      */
     protected void deleteDir(File d) {
+        deleteDir(d, false);
+    }
+
+    /**
+     * Go and delete the directory tree.
+     * @param d the directory to delete
+     * @param deleteFiles whether to delete files
+     */
+    protected void deleteDir(File d, boolean deleteFiles) {
         String[] list = d.list();
         if (list == null) {
             return;
@@ -335,6 +328,9 @@ public class Move extends Copy {
             File f = new File(d, s);
             if (f.isDirectory()) {
                 deleteDir(f);
+            } else if (deleteFiles && !(f.delete())) {
+                throw new BuildException("Unable to delete file "
+                                         + f.getAbsolutePath());
             } else {
                 throw new BuildException("UNEXPECTED ERROR - The file "
                                          + f.getAbsolutePath()
@@ -370,34 +366,19 @@ public class Move extends Copy {
                                  boolean filtering, boolean overwrite)
         throws IOException, BuildException {
 
-        boolean renamed = true;
-        if ((getFilterSets() != null && getFilterSets().size() > 0)
-            || (getFilterChains() != null && getFilterChains().size() > 0)) {
-            renamed = false;
-        } else {
-            if (!filtering) {
-                // ensure that parent dir of dest file exists!
-                File parent = destFile.getParentFile();
-                if (parent != null && !parent.exists()) {
-                    parent.mkdirs();
-                }
-
-                if (destFile.exists()) {
-                    if (sourceFile.isDirectory()) {
-                     throw new BuildException(
-                        new StringBuffer("Cannot replace ").append(
-                        ((destFile.isFile()) ? "file " : "directory ")).append(
-                        destFile).append(" with directory ").append(
-                        sourceFile).toString());
-                    } else if (destFile.isFile() && !destFile.delete()) {
-                        throw new BuildException("Unable to remove existing "
-                                                 + "file " + destFile);
-                    }
-                }
-                renamed = sourceFile.renameTo(destFile);
-            } else {
-                renamed = false;
+        boolean renamed = false;
+        if ((getFilterSets().size() + getFilterChains().size() == 0)
+            && !(filtering || destFile.isDirectory())) {
+            // ensure that parent dir of dest file exists!
+            File parent = destFile.getParentFile();
+            if (parent != null && !parent.exists()) {
+                parent.mkdirs();
+            }
+            if (destFile.isFile() && !destFile.delete()) {
+                throw new BuildException("Unable to remove existing "
+                                         + "file " + destFile);
             }
+            renamed = sourceFile.renameTo(destFile);
         }
         return renamed;
     }

src/testcases/org/apache/tools/ant/taskdefs/MoveTest.java

@@ -89,6 +89,14 @@ public class MoveTest extends BuildFileTest {
         assertTrue(!getProject().resolveFile("A").exists());
     }
 
+    public void testMoveFileAndFileset() {
+        executeTarget("testMoveFileAndFileset");
+    }
+
+    public void testCompleteDirectoryMoveToExistingDir() {
+        executeTarget("testCompleteDirectoryMoveToExistingDir");
+    }
+
     public void testCompleteDirectoryMoveFileToFile() {
         executeTarget("testCompleteDirectoryMoveFileToFile");
     }
@@ -97,24 +105,24 @@ public class MoveTest extends BuildFileTest {
         executeTarget("testCompleteDirectoryMoveFileToDir");
     }
 
+    public void testCompleteDirectoryMoveFileAndFileset() {
+        executeTarget("testCompleteDirectoryMoveFileAndFileset");
+    }
+
     public void testCompleteDirectoryMoveFileToExistingFile() {
-        expectBuildExceptionContaining("testCompleteDirectoryMoveFileToExistingFile",
-                                       "", "Cannot replace file");
+        executeTarget("testCompleteDirectoryMoveFileToExistingFile");
     }
 
     public void testCompleteDirectoryMoveFileToExistingDir() {
-        expectBuildExceptionContaining("testCompleteDirectoryMoveFileToExistingDir",
-                                       "", "Cannot replace directory");
+        executeTarget("testCompleteDirectoryMoveFileToExistingDir");
     }
 
     public void testCompleteDirectoryMoveFileToDirWithExistingFile() {
-        expectBuildExceptionContaining("testCompleteDirectoryMoveFileToDirWithExistingFile",
-                                       "", "Cannot replace file");
+        executeTarget("testCompleteDirectoryMoveFileToDirWithExistingFile");
     }
 
     public void testCompleteDirectoryMoveFileToDirWithExistingDir() {
-        expectBuildExceptionContaining("testCompleteDirectoryMoveFileToDirWithExistingDir",
-                                       "", "Cannot replace directory");
+        executeTarget("testCompleteDirectoryMoveFileToDirWithExistingDir");
     }
 
 }