1. As suggested by Peter, nixed the attributes outfile & verbose

previously added to Untar, Unjar, Unwar and Unzip, so that these tasks stay
focussed on unarchival and nothing but it.
2. If the Src attribute represents a directory, an exception is thrown, as Stefan
suggested.  Nested Filesets to be used instead.
3. Testcase added to catch this exception.


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

WHATSNEW

@@ -12,6 +12,10 @@ Changes that could break older environments:
 
 * <gzip> will throw an exception if your src attribute points to a directory.
 
+* Unjar, Unzip and Unwar will throw an exception if the Src attribute
+  represents a directory.  Support for nested filesets is provided
+  instead.
+
 Fixed bugs:
 -----------
 
@@ -41,10 +45,9 @@ Fixed bugs:
 
 Other changes:
 --------------
-* Attributes outfile and verbose added to Unjar, Untar, Unwar and Unzip.
-  These tasks now support patternsets to select files from an archive
-  for extraction.  Filesets may be used to select archived files for
-  unarchival.
+* Unjar, Untar, Unwar and Unzip now support patternsets to 
+  select files from an archive for extraction.  Filesets may be 
+  used to select archived files for unarchival.
 
 * Javac task allows debug levels to be spcified.  Debug levels
   will have an effect only when the modern compiler is used.

docs/manual/CoreTasks/unzip.html

@@ -35,7 +35,7 @@ to perform unarchival upon.
   <tr>
     <td valign="top">dest</td>
     <td valign="top">directory where to store the expanded files.</td>
-    <td align="center" valign="top">Yes, if outfile is not specified.</td>
+    <td align="center" valign="top">Yes</td>
   </tr>
   <tr>
     <td valign="top">overwrite</td>
@@ -44,17 +44,6 @@ to perform unarchival upon.
       true).</td>
     <td align="center" valign="top">No</td>
   </tr>
-  <tr>
-    <td valign="top">outfile</td>
-    <td valign="top">List the contents of the archive into this file.</td>
-    <td align="center" valign="top">Yes, if dest is not specified.</td>
-  </tr>
-  <tr>
-    <td valign="top">verbose</td>
-    <td valign="top">Include file details when listing contents of
-    the archive into outfile? Defaults to <I>false</I>.</td>
-    <td align="center" valign="top">No</td>
-  </tr>
 </table>
 <h3>Examples</h3>
 <blockquote>
@@ -70,9 +59,7 @@ to perform unarchival upon.
 <blockquote>
 <p><pre>
 &lt;unzip src=&quot;${tomcat_src}/tools-src.zip&quot;
-       dest=&quot;${tools.home}&quot;
-       outfile=&quot;${outfile}&quot;
-       vebose=&quot;on&quot;&gt;
+       dest=&quot;${tools.home}&quot;&gt;
     &lt;patternset&gt;
         &lt;include name=&quot;**/*.java&quot;/&gt;
         &lt;exclude name=&quot;**/Test*.java&quot;/&gt;
@@ -82,9 +69,7 @@ to perform unarchival upon.
 </blockquote>
 <blockquote>
 <p><pre>
-&lt;unzip dest=&quot;${tools.home}&quot;
-       outfile=&quot;${outfile}&quot;
-       vebose=&quot;on&quot;&gt;
+&lt;unzip dest=&quot;${tools.home}&quot;&gt;
     &lt;patternset&gt;
         &lt;include name=&quot;**/*.java&quot;/&gt;
         &lt;exclude name=&quot;**/Test*.java&quot;/&gt;

src/etc/testcases/taskdefs/untar.xml

@@ -16,4 +16,8 @@
     <untar src="expected/asf-logo.gif.tar" dest="." />
   </target>
 
+  <target name="srcDirTest">
+    <untar src="." dest="." />
+  </target>
+
 </project>

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

@@ -60,15 +60,12 @@ import org.apache.tools.ant.Project;
 import org.apache.tools.ant.types.FileSet;
 import org.apache.tools.ant.types.PatternSet;
 import org.apache.tools.ant.util.FileUtils;
-import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FileNotFoundException;
-import java.io.FileWriter;
 import java.io.InputStream;
 import java.io.IOException;
-import java.io.PrintWriter;
 import java.util.Date;
 import java.util.Vector;
 import java.util.zip.ZipInputStream;
@@ -82,16 +79,11 @@ import java.util.zip.ZipEntry;
  * @author <a href="mailto:umagesh@rediffmail.com">Magesh Umasankar</a>
  */
 public class Expand extends MatchingTask {
-    protected File dest;
-    protected File source; // req
-    protected File outFile;
-    protected boolean overwrite = true;
-    protected boolean verbose;
-    protected PrintWriter pw = null;
-    protected BufferedWriter bw = null;
-    protected FileWriter fw = null;
-    protected Vector patternsets = new Vector();
-    protected Vector filesets = new Vector();
+    private File dest; //req
+    private File source; // req
+    private boolean overwrite = true;
+    private Vector patternsets = new Vector();
+    private Vector filesets = new Vector();
 
     /**
      * Do the work.
@@ -107,58 +99,22 @@ public class Expand extends MatchingTask {
             throw new BuildException("src attribute and/or filesets must be specified");
         }
 
-        if (dest == null && outFile == null) {
+        if (dest == null) {
             throw new BuildException(
-                "Dest and/or the OutFile attribute " +
-                "must be specified");
+                "Dest attribute must be specified");
         }
 
-        if (dest != null && dest.exists() && !dest.isDirectory()) {
+        if (dest.exists() && !dest.isDirectory()) {
             throw new BuildException("Dest must be a directory.", location);
         }
 
-        if (verbose && outFile == null) {
-            throw new BuildException(
-                "Verbose can be set only when OutFile is " +
-                "specified");
-        }
-
         FileUtils fileUtils = FileUtils.newFileUtils();
 
-        try {
-            if (outFile != null) {
-                if (outFile.isDirectory()) {
-                    throw new BuildException("Outfile " + outFile
-                        + " must not be a directory.");
-                }
-                if (!outFile.exists()) {
-                    File parent = new File(outFile.getParent());
-                    if (!parent.exists()) {
-                        if (!parent.mkdirs()) {
-                            throw new BuildException("Unable to create "
-                                + outFile);
-                        }
-                    }
-                }
-                fw = new FileWriter(outFile);
-                bw = new BufferedWriter(fw);
-                pw = new PrintWriter(bw, true);
-            }
-        } catch (IOException ioe) {
-            throw new BuildException(ioe.getMessage(), location);
-        }
         if (source != null) {
             if (source.isDirectory()) {
-                // get all the files in the descriptor directory
-                DirectoryScanner ds = super.getDirectoryScanner(source);
-
-                String[] files = ds.getIncludedFiles();
-                for (int i = 0; i < files.length; ++i) {
-                    File file = new File(source, files[i]);
-                    expandFile(fileUtils, file, dest);
-                }
-            }
-            else {
+                throw new BuildException("Src must not be a directory." +
+                    " Use nested filesets instead.", location);
+            } else {
                 expandFile(fileUtils, source, dest);
             }
         }
@@ -175,21 +131,6 @@ public class Expand extends MatchingTask {
                 }
             }
         }
-        if (pw != null) {
-            pw.close();
-        }
-        if (bw != null) {
-            try {
-                bw.close();
-            } catch (IOException ioe1) {}
-        }
-        if (fw != null) {
-            try {
-                fw.close();
-            } catch (IOException ioe1) {
-                //Oh, well!  We did our best
-            }
-        }
     }
 
     /*
@@ -204,15 +145,12 @@ public class Expand extends MatchingTask {
 
             while ((ze = zis.getNextEntry()) != null) {
                 extractFile(fileUtils, srcF, dir, zis,
-                            ze.getName(), ze.getSize(),
+                            ze.getName(),
                             new Date(ze.getTime()),
                             ze.isDirectory());
             }
 
-            if (dest != null) {
-                log("expand complete", Project.MSG_VERBOSE );
-            }
-
+            log("expand complete", Project.MSG_VERBOSE );
         } catch (IOException ioe) {
             throw new BuildException("Error while expanding " + srcF.getPath(), ioe);
         } finally {
@@ -227,21 +165,9 @@ public class Expand extends MatchingTask {
 
     protected void extractFile(FileUtils fileUtils, File srcF, File dir,
                                InputStream compressedInputStream,
-                               String entryName, long entrySize,
+                               String entryName,
                                Date entryDate, boolean isDirectory)
                                throws IOException {
-        extractFile(fileUtils, srcF, dir, compressedInputStream,
-                    entryName, entrySize, entryDate, isDirectory,
-                    null, null);
-
-    }
-
-    protected void extractFile(FileUtils fileUtils, File srcF, File dir,
-                               InputStream compressedInputStream,
-                               String entryName, long entrySize,
-                               Date entryDate, boolean isDirectory,
-                               String modeStr, String userGroup)
-                               throws IOException {
 
         if (patternsets != null && patternsets.size() > 0) {
             String name = entryName;
@@ -275,81 +201,50 @@ public class Expand extends MatchingTask {
             }
         }
 
-        if (dest != null) {
-            log("Expanding: " + srcF + " into " + dir, Project.MSG_INFO);
-        }
+        log("Expanding: " + srcF + " into " + dir, Project.MSG_INFO);
+        File f = fileUtils.resolveFile(dir, entryName);
+        try {
+            if (!overwrite && f.exists()
+                && f.lastModified() >= entryDate.getTime()) {
+                log("Skipping " + f + " as it is up-to-date",
+                    Project.MSG_DEBUG);
+                return;
+            }
 
-        if (outFile != null) {
-            if (verbose) {
-                StringBuffer sb = new StringBuffer();
-                if (modeStr != null) {
-                    sb.append(modeStr);
-                    sb.append(' ');
-                }
-                if (userGroup != null) {
-                    sb.append(userGroup);
-                    sb.append(' ');
-                }
-                String s = Long.toString(entrySize);
-                int len = s.length();
-                for(int i = 6 - len; i > 0; i--) {
-                    sb.append(' ');
-                }
-                sb.append(s)
-                  .append(' ')
-                  .append(entryDate.toString());
-                sb.append(' ')
-                  .append(entryName);
-                pw.println(sb);
+            log("expanding " + entryName + " to "+ f,
+                Project.MSG_VERBOSE);
+            // create intermediary directories - sometimes zip don't add them
+            File dirF= fileUtils.getParentFile(f);
+            dirF.mkdirs();
+
+            if (isDirectory) {
+                f.mkdirs();
             } else {
-                pw.println(entryName);
-            }
-        }
-        if (dest != null) {
-            File f = fileUtils.resolveFile(dir, entryName);
-            try {
-                if (!overwrite && f.exists()
-                    && f.lastModified() >= entryDate.getTime()) {
-                    log("Skipping " + f + " as it is up-to-date",
-                        Project.MSG_DEBUG);
-                    return;
-                }
+                byte[] buffer = new byte[1024];
+                int length = 0;
+                FileOutputStream fos = null;
+                try {
+                    fos = new FileOutputStream(f);
 
-                log("expanding " + entryName + " to "+ f,
-                    Project.MSG_VERBOSE);
-                // create intermediary directories - sometimes zip don't add them
-                File dirF= fileUtils.getParentFile(f);
-                dirF.mkdirs();
+                    while ((length =
+                            compressedInputStream.read(buffer)) >= 0) {
+                        fos.write(buffer, 0, length);
+                    }
 
-                if (isDirectory) {
-                    f.mkdirs();
-                } else {
-                    byte[] buffer = new byte[1024];
-                    int length = 0;
-                    FileOutputStream fos = null;
-                    try {
-                        fos = new FileOutputStream(f);
-                        
-                        while ((length =
-                                compressedInputStream.read(buffer)) >= 0) {
-                            fos.write(buffer, 0, length);
-                        }
-                        
-                        fos.close();
-                        fos = null;
-                    } finally {
-                        if (fos != null) {
-                            try {
-                                fos.close();
-                            } catch (IOException e) {}
-                        }
+                    fos.close();
+                    fos = null;
+                } finally {
+                    if (fos != null) {
+                        try {
+                            fos.close();
+                        } catch (IOException e) {}
                     }
                 }
-                    
-                fileUtils.setFileLastModified(f, entryDate.getTime());
-            } catch( FileNotFoundException ex ) {
-                log("Unable to expand to file " + f.getPath(), Project.MSG_WARN);
             }
+
+            fileUtils.setFileLastModified(f, entryDate.getTime());
+        } catch( FileNotFoundException ex ) {
+            log("Unable to expand to file " + f.getPath(), Project.MSG_WARN);
         }
 
     }
@@ -381,23 +276,6 @@ public class Expand extends MatchingTask {
         overwrite = b;
     }
 
-    /**
-     * Set the output file to be used to store the list of the
-     * archive's contents.
-     *
-     * @param outFile the output file to be used.
-     */
-    public void setOutfile(File outFile) {
-        this.outFile = outFile;
-    }
-
-    /**
-     * Set the verbose mode for the contents-list file.
-     */
-    public void setVerbose(boolean verbose) {
-        this.verbose = verbose;
-    }
-
     /**
      * Add a patternset
      */

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

@@ -73,50 +73,21 @@ import java.util.Date;
  * @author <a href="mailto:umagesh@rediffmail.com">Magesh Umasankar</a>
  */
 public class Untar extends Expand {
-    private final static int S_IFMT   = 0170000;
-    private final static int S_IFSOCK = 0140000;
-    private final static int S_IFLNK  = 0120000;
-    private final static int S_IFREG  = 0100000;
-    private final static int S_IFBLK  = 0060000;
-    private final static int S_IFDIR  = 0040000;
-    private final static int S_IFCHR  = 0020000;
-    private final static int S_IFIFO  = 0010000;
-    private final static int S_ISUID  = 0004000;
-    private final static int S_ISGID  = 0002000;
-    private final static int S_ISVTX  = 0001000;
-    private final static int S_IRWXU  = 00700;
-    private final static int S_IRUSR  = 00400;
-    private final static int S_IWUSR  = 00200;
-    private final static int S_IXUSR  = 00100;
-    private final static int S_IRWXG  = 00070;
-    private final static int S_IRGRP  = 00040;
-    private final static int S_IWGRP  = 00020;
-    private final static int S_IXGRP  = 00010;
-    private final static int S_IRWXO  = 00007;
-    private final static int S_IROTH  = 00004;
-    private final static int S_IWOTH  = 00002;
-    private final static int S_IXOTH  = 00001;
 
     protected void expandFile(FileUtils fileUtils, File srcF, File dir) {
         TarInputStream tis = null;
         try {
-            if (dest != null) {
-                log("Expanding: " + srcF + " into " + dir, Project.MSG_INFO);
-            }
+            log("Expanding: " + srcF + " into " + dir, Project.MSG_INFO);
 
             tis = new TarInputStream(new FileInputStream(srcF));
             TarEntry te = null;
 
             while ((te = tis.getNextEntry()) != null) {
                 extractFile(fileUtils, srcF, dir, tis,
-                            te.getName(), te.getSize(),
-                            te.getModTime(), te.isDirectory(),
-                            mode2str(te.getMode()),
-                            te.getUserId() + "/" + te.getGroupId());
-            }
-            if (dest != null) {
-                log("expand complete", Project.MSG_VERBOSE );
+                            te.getName(),
+                            te.getModTime(), te.isDirectory());
             }
+            log("expand complete", Project.MSG_VERBOSE );
 
         } catch (IOException ioe) {
             throw new BuildException("Error while expanding " + srcF.getPath(),
@@ -130,56 +101,4 @@ public class Untar extends Expand {
             }
         }
     }
-
-    private String mode2str(int mode) {
-        StringBuffer sb = new StringBuffer("----------");
-        if ((mode & S_IFREG ) == 0) {
-            if ((mode & S_IFDIR ) != 0) {
-                sb.setCharAt(0, 'd');
-            } else if ((mode & S_IFLNK)  != 0) {
-                sb.setCharAt(0, 'l');
-            } else if ((mode & S_IFIFO)  != 0) {
-                sb.setCharAt(0, 'p');
-            } else if ((mode & S_IFCHR)  != 0) {
-                sb.setCharAt(0, 'c');
-            } else if ((mode & S_IFBLK)  != 0) {
-                sb.setCharAt(0, 'b');
-            } else if ((mode & S_IFSOCK) != 0) {
-                sb.setCharAt(0, 's');
-            } else if ((mode & S_IFIFO)  != 0) {
-                sb.setCharAt(0, 'p');
-            }
-        }
-
-        if ((mode & S_IRUSR ) != 0) {
-            sb.setCharAt(1, 'r');
-        }
-        if ((mode & S_IWUSR ) != 0) {
-            sb.setCharAt(2, 'w');
-        }
-        if ((mode & S_IXUSR ) != 0) {
-            sb.setCharAt(3, 'x');
-        }
-
-        if ((mode & S_IRGRP ) != 0) {
-            sb.setCharAt(4, 'r');
-        }
-        if ((mode & S_IWGRP ) != 0) {
-            sb.setCharAt(5, 'w');
-        }
-        if ((mode & S_IXGRP ) != 0) {
-            sb.setCharAt(6, 'x');
-        }
-
-        if ((mode & S_IROTH ) != 0) {
-            sb.setCharAt(7, 'r');
-        }
-        if ((mode & S_IWOTH ) != 0) {
-            sb.setCharAt(8, 'w');
-        }
-        if ((mode & S_IXOTH ) != 0) {
-            sb.setCharAt(9, 'x');
-        }
-        return new String(sb);
-    }
 }

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

@@ -52,7 +52,7 @@
  * <http://www.apache.org/>.
  */
 package org.apache.tools.ant.taskdefs;
- 
+
 import java.io.File;
 import org.apache.tools.ant.BuildFileTest;
 import org.apache.tools.ant.util.FileUtils;
@@ -61,13 +61,13 @@ import org.apache.tools.ant.util.FileUtils;
  * @author <a href="mailto:stefan.bodewig@epost.de">Stefan Bodewig</a>
  * @version $Revision$
  */
-public class UntarTest extends BuildFileTest { 
-    
-    public UntarTest(String name) { 
+public class UntarTest extends BuildFileTest {
+
+    public UntarTest(String name) {
         super(name);
-    }    
-    
-    public void setUp() { 
+    }
+
+    public void setUp() {
         configureProject("src/etc/testcases/taskdefs/untar.xml");
     }
 
@@ -81,12 +81,16 @@ public class UntarTest extends BuildFileTest {
         assertTrue(fileUtils.contentEquals(project.resolveFile("../asf-logo.gif"),
                                            project.resolveFile("asf-logo.gif")));
     }
-    
+
     public void testTestTarTask() throws java.io.IOException {
         FileUtils fileUtils = FileUtils.newFileUtils();
         executeTarget("testTarTask");
         assertTrue(fileUtils.contentEquals(project.resolveFile("../asf-logo.gif"),
                                            project.resolveFile("asf-logo.gif")));
     }
-    
+
+    public void testSrcDirTest() throws java.io.IOException {
+        FileUtils fileUtils = FileUtils.newFileUtils();
+        expectBuildException("srcDirTest", "Src cannot be a directory.");
+    }
 }