Pete, those changes look good to me. I do have a few suggested fixes to the new
code. I created two levels of changes. The first batch (ZipDiff1.txt) contains
just bugfixes, which I think should be non-controversial. The second batch
(*Diff2.txt) changes the behavior, adding some functionality that I think is
useful. It also reorganizes the code somewhat, removing most of the code from
Jar and War. I think these are all good changes, but I'll leave it up to you to
decide which batch to take.
Enjoy!
Alex
[BATCH 1]
- execute() now checks for (locFileSets.size() == 0) too, so if your Zip task
contains only prefixedfilesets, it won't throw an exception.
- If your build file included a <prefixedfileset> that didn't contain a
"prefix" attribute, the directory "" would get added to the archive, which is
OK with WinZip, but the "jar" tool chokes on it. Now "" never gets added.
- All the parent directories of the prefix directory were not being added to
the archive. E.g. if the prefix was "x/y/z", then directory "x/y/z" was added,
but not "x/y" or "x". Don't know why this is important, but other parts of
Zip.java seem to think it is. Now all parent directories are added.
- Fixed a comment to say "prefixedfileset" instead of "relocatablefileset"
(must have been a previous name).
- Fixed the exception handling in execute(). Previously, if for example your
<war> element didn't have a "webxml" element, you'd get this exception:
Problem creating war: ZIP file must have at least one entry (and the archive is
probably corrupt but I could not delete it)
java.util.zip.ZipException: ZIP file must have at least one entry
That's because close() in the finally clause was throwing a second exception,
and the real problem (the BuildException that happened before that) was getting
lost. It now swallows the close() exception (when necessary), and displays the
real problem instead ("webxml attribute is required").
[BATCH 2]
- PrefixedFileSet now has an additional attribute "fullpath", which specifies
the full path in which to save the file specified by the fileset. For example,
fullpath="META-INF/MANIFEST.MF". This means that the name of the file can be
changed when it goes in the archive, not just the path. This attribute is only
valid on a fileset that represents a single file. (As I mentioned in an earlier
e-mail, I think I prefer "archivedir" and "archivefile" to "prefix" and
"fullpath", but I've left them as-is for now.)
- A big benefit of this change is that it now allows Jar, War, and other
potential subclasses, to just be special cases of Zip. Most of the code in Jar
and War goes away - they just perform some setup, turing the "manifest"
attribute (for example) into a PrefixedFileSet with
fullpath="META-INF/MANIFEST.MF". Other subclasses such as Ear can be added with
a minimum of code, but the nice thing is that they're not required anymore -
you can get the same functionality just by setting up your <zip> task, rather
than having to write an Ear task.
- I changed the name of the add method, so that you now use regular <fileset>
elements instead of <prefixedfileset> elements. I think this is better, but it
can easily be changed.
Index: Zip.java
===================================================================
RCS file:
/home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/Zip.java,v
retrieving revision 1.20
diff -u -r1.20 Zip.java
--- Zip.java 2000/12/07 12:21:12 1.20
+++ Zip.java 2000/12/08 16:46:20
@@ -150,9 +150,9 @@
}
public void execute() throws BuildException {
- if (baseDir == null && filesets.size() == 0 &&
"zip".equals(archiveType))
+ if (baseDir == null && filesets.size() == 0 && locFileSets.size() == 0
&& "zip".equals(archiveType))
throw new BuildException( "basedir attribute must be set, or at
least " +
- "one fileset must be given!" );
+ "one fileset or prefixedfileset must be
given!" );
if (zipFile == null) {
throw new BuildException("You must specify the " + archiveType + "
file to create!");
@@ -178,6 +178,7 @@
log("Building "+ archiveType +": "+ zipFile.getAbsolutePath());
try {
+ boolean success = false;
ZipOutputStream zOut = new ZipOutputStream(new
FileOutputStream(zipFile));
try {
if (doCompress) {
@@ -191,9 +192,24 @@
for (int j = 0; j < dssSize; j++) {
addFiles(scanners[j], zOut, "");
+
+ success = true;
}
} finally {
- zOut.close ();
+ // Close the output stream.
+ try {
+ if (zOut != null)
+ zOut.close ();
+ } catch(IOException ex) {
+ // If we're in this finally clause because of an
exception, we don't
+ // really care if there's an exception when closing the
stream. E.g. if it
+ // throws "ZIP file must have at least one entry", because
an exception happened
+ // before we added any files, then we must swallow this
exception. Otherwise,
+ // the error that's reported will be the close() error,
which is not the real
+ // cause of the problem.
+ if (success)
+ throw ex;
+ }
}
} catch (IOException ioe) {
String msg = "Problem creating " + archiveType + ": " +
ioe.getMessage();
@@ -473,7 +489,7 @@
}
/**
- * Iterate over the given Vector of relocatablefilesets and add
+ * Iterate over the given Vector of prefixedfilesets and add
* all files to the ZipOutputStream using the given prefix.
*/
protected void addPrefixedFiles(Vector v, ZipOutputStream zOut)
@@ -486,8 +502,11 @@
&& !prefix.endsWith("/")
&& !prefix.endsWith("\\")) {
prefix += "/";
+ }
+ if (prefix.length() > 0) {
+ addParentDirs(null, prefix, zOut, "");
+ zipDir(null, zOut, prefix);
}
- zipDir(null, zOut, prefix);
addFiles(ds, zOut, prefix);
}
}
Index: Jar.java
===================================================================
RCS file:
/home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/Jar.java,v
retrieving revision 1.9
diff -u -r1.9 Jar.java
--- Jar.java 2000/11/28 13:59:00 1.9
+++ Jar.java 2000/12/08 16:34:41
@@ -68,35 +68,37 @@
public class Jar extends Zip {
private File manifest;
-
+ private boolean manifestAdded;
+
public Jar() {
super();
archiveType = "jar";
emptyBehavior = "create";
}
- public void execute() {
- if (manifest != null && !manifest.exists())
- throw new BuildException("Manifest file: " + manifest + " does not
exists.");
- super.execute();
- }
-
public void setJarfile(File jarFile) {
super.setZipfile(jarFile);
}
public void setManifest(File manifestFile) {
manifest = manifestFile;
+ if (!manifest.exists())
+ throw new BuildException("Manifest file: " + manifest + " does not
exist.");
+
+ // Create a PrefixedFileSet for this file, and pass it up.
+ PrefixedFileSet fs = new PrefixedFileSet();
+ fs.setDir(new File(manifest.getParent()));
+ fs.setIncludes(manifest.getName());
+ fs.setFullpath("META-INF/MANIFEST.MF");
+ super.addFileset(fs);
}
+
protected void initZipOutputStream(ZipOutputStream zOut)
throws IOException, BuildException
{
- // add manifest first
- if (manifest != null) {
- zipDir(new File(manifest.getParent()), zOut, "META-INF/");
- super.zipFile(manifest, zOut, "META-INF/MANIFEST.MF");
- } else {
+ // If no manifest is specified, add the default one.
+ if (manifest == null) {
String s = "/org/apache/tools/ant/defaultManifest.mf";
InputStream in = this.getClass().getResourceAsStream(s);
if ( in == null )
@@ -104,53 +106,27 @@
zipDir(null, zOut, "META-INF/");
zipFile(in, zOut, "META-INF/MANIFEST.MF",
System.currentTimeMillis());
}
- }
- protected boolean isUpToDate(FileScanner[] scanners, File zipFile) throws
BuildException
- {
- File[] files = grabFiles(scanners);
-
- if (manifest != null) {
- // just add the manifest file to the mix
-
- DirectoryScanner ds = new DirectoryScanner();
- ds.setBasedir(new File(manifest.getParent()));
- ds.setIncludes(new String[] {manifest.getName()});
- ds.scan();
-
- FileScanner[] myScanners = new FileScanner[scanners.length+1];
- System.arraycopy(scanners, 0, myScanners, 0, scanners.length);
- myScanners[scanners.length] = ds;
-
- boolean retval = super.isUpToDate(myScanners, zipFile);
- if (!retval && files.length == 0) {
- log("Note: creating empty "+archiveType+" archive " + zipFile,
- Project.MSG_INFO);
- }
- return retval;
-
- } else if (emptyBehavior.equals("create") && files.length == 0) {
-
- log("Note: creating empty "+archiveType+" archive " + zipFile,
- Project.MSG_INFO);
- return false;
-
- } else {
- // all other cases are handled correctly by Zip's method
- return super.isUpToDate(scanners, zipFile);
- }
+ super.initZipOutputStream(zOut);
}
protected void zipFile(File file, ZipOutputStream zOut, String vPath)
throws IOException
{
- // We already added a META-INF/MANIFEST.MF
- if (!vPath.equalsIgnoreCase("META-INF/MANIFEST.MF")) {
- super.zipFile(file, zOut, vPath);
+ // If the file being added is META-INF/MANIFEST.MF, we warn if it's
not the
+ // one specified in the "manifest" attribute - or if it's being added
twice,
+ // meaning the same file is specified by the "manifeset" attribute and
in
+ // a <fileset> element.
+ if (vPath.equalsIgnoreCase("META-INF/MANIFEST.MF")) {
+ if (manifest == null || !manifest.equals(file) || manifestAdded) {
+ log("Warning: selected "+archiveType+" files include a
META-INF/MANIFEST.MF which will be ignored " +
+ "(please use manifest attribute to "+archiveType+" task)",
Project.MSG_WARN);
+ } else {
+ super.zipFile(file, zOut, vPath);
+ manifestAdded = true;
+ }
} else {
- log("Warning: selected "+archiveType+" files include a
META-INF/MANIFEST.MF which will be ignored " +
- "(please use manifest attribute to "+archiveType+" task)",
Project.MSG_WARN);
+ super.zipFile(file, zOut, vPath);
}
}
-
}
Index: War.java
===================================================================
RCS file:
/home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/War.java,v
retrieving revision 1.5
diff -u -r1.5 War.java
--- War.java 2000/12/07 12:00:54 1.5
+++ War.java 2000/12/08 16:34:43
@@ -69,11 +69,8 @@
public class War extends Jar {
private File deploymentDescriptor;
+ private boolean descriptorAdded;
- private Vector libFileSets = new Vector();
- private Vector classesFileSets = new Vector();
- private Vector webInfFileSets = new Vector();
-
public War() {
super();
archiveType = "war";
@@ -85,85 +82,64 @@
}
public void setWebxml(File descr) {
- deploymentDescriptor = descr;
- }
-
- public void addLib(FileSet fs) {
- libFileSets.addElement(fs);
+ deploymentDescriptor = descr;
+ if (!deploymentDescriptor.exists())
+ throw new BuildException("Deployment descriptor: " +
deploymentDescriptor + " does not exist.");
+
+ // Create a PrefixedFileSet for this file, and pass it up.
+ PrefixedFileSet fs = new PrefixedFileSet();
+ fs.setDir(new File(deploymentDescriptor.getParent()));
+ fs.setIncludes(deploymentDescriptor.getName());
+ fs.setFullpath("WEB-INF/web.xml");
+ super.addFileset(fs);
+ }
+
+ public void addLib(PrefixedFileSet fs) {
+ // We just set the prefix for this fileset, and pass it up.
+ fs.setPrefix("WEB-INF/lib/");
+ super.addFileset(fs);
+ }
+
+ public void addClasses(PrefixedFileSet fs) {
+ // We just set the prefix for this fileset, and pass it up.
+ fs.setPrefix("WEB-INF/classes/");
+ super.addFileset(fs);
+ }
+
+ public void addWebinf(PrefixedFileSet fs) {
+ // We just set the prefix for this fileset, and pass it up.
+ fs.setPrefix("WEB-INF/");
+ super.addFileset(fs);
}
- public void addClasses(FileSet fs) {
- classesFileSets.addElement(fs);
- }
-
- public void addWebinf(FileSet fs) {
- webInfFileSets.addElement(fs);
- }
-
- /**
- * Add the deployment descriptor as well as all files added the
- * special way of nested lib, classes or webinf filesets.
- */
protected void initZipOutputStream(ZipOutputStream zOut)
throws IOException, BuildException
{
- // add deployment descriptor first
- if (deploymentDescriptor != null) {
- zipDir(new File(deploymentDescriptor.getParent()), zOut,
- "WEB-INF/");
- super.zipFile(deploymentDescriptor, zOut, "WEB-INF/web.xml");
- } else {
- throw new BuildException("webxml attribute is required", location);
- }
-
- addFiles(libFileSets, zOut, "WEB-INF/lib/");
- addFiles(classesFileSets, zOut, "WEB-INF/classes/");
- addFiles(webInfFileSets, zOut, "WEB-INF/");
-
- super.initZipOutputStream(zOut);
- }
-
- protected boolean isUpToDate(FileScanner[] scanners, File zipFile) throws
BuildException
- {
+ // If no webxml file is specified, it's an error.
if (deploymentDescriptor == null) {
throw new BuildException("webxml attribute is required", location);
}
-
- // just add some Scanners for our filesets and web.xml and let
- // Jar/Zip do the rest of the work
-
- FileScanner[] myScanners = new FileScanner[scanners.length
- + 1 // web.xml
- + libFileSets.size()
- + classesFileSets.size()
- + webInfFileSets.size()];
-
- System.arraycopy(scanners, 0, myScanners, 0, scanners.length);
-
- DirectoryScanner ds = new DirectoryScanner();
- ds.setBasedir(new File(deploymentDescriptor.getParent()));
- ds.setIncludes(new String[] {deploymentDescriptor.getName()});
- ds.scan();
- myScanners[scanners.length] = ds;
- addScanners(myScanners, scanners.length+1, libFileSets);
- addScanners(myScanners, scanners.length+1+libFileSets.size(),
- classesFileSets);
- addScanners(myScanners,
scanners.length+1+libFileSets.size()+classesFileSets.size(),
- webInfFileSets);
-
- return super.isUpToDate(myScanners, zipFile);
+ super.initZipOutputStream(zOut);
}
protected void zipFile(File file, ZipOutputStream zOut, String vPath)
throws IOException
{
- // We already added a WEB-INF/web.xml
- if (!vPath.equalsIgnoreCase("WEB-INF/web.xml")) {
- super.zipFile(file, zOut, vPath);
+ // If the file being added is WEB-INF/web.xml, we warn if it's not the
+ // one specified in the "webxml" attribute - or if it's being added
twice,
+ // meaning the same file is specified by the "webxml" attribute and in
+ // a <fileset> element.
+ if (vPath.equalsIgnoreCase("WEB-INF/web.xml")) {
+ if (deploymentDescriptor == null ||
!deploymentDescriptor.equals(file) || descriptorAdded) {
+ log("Warning: selected "+archiveType+" files include a
WEB-INF/web.xml which will be ignored " +
+ "(please use webxml attribute to "+archiveType+" task)",
Project.MSG_WARN);
+ } else {
+ super.zipFile(file, zOut, vPath);
+ descriptorAdded = true;
+ }
} else {
- log("Warning: selected "+archiveType+" files include a
WEB-INF/web.xml which will be ignored " +
- "(please use webxml attribute to "+archiveType+" task)",
Project.MSG_WARN);
+ super.zipFile(file, zOut, vPath);
}
}
-}
+}
\ No newline at end of file
Index: Zip.java
===================================================================
RCS file:
/home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/Zip.java,v
retrieving revision 1.20
diff -u -r1.20 Zip.java
--- Zip.java 2000/12/07 12:21:12 1.20
+++ Zip.java 2000/12/08 16:47:55
@@ -82,8 +82,7 @@
protected String emptyBehavior = "skip";
private Vector filesets = new Vector ();
private Hashtable addedDirs = new Hashtable();
- private Vector locFileSets = new Vector();
-
+
/**
* This is the name/location of where to
* create the .zip file.
@@ -110,26 +109,32 @@
/**
* Adds a set of files (nested fileset attribute).
*/
- public void addFileset(FileSet set) {
+ public void addFileset(PrefixedFileSet set) {
filesets.addElement(set);
}
/**
* FileSet with an additional prefix attribute to specify the
* location we want to move the files to (inside the archive).
+ * Or, if this FileSet represents only a single file, then the
+ * fullpath attribute can be set, which specifies the full path
+ * that the file should have when it is placed in the archive.
*/
public static class PrefixedFileSet extends FileSet {
private String prefix = "";
+ private String fullpath = "";
public void setPrefix(String loc) {
prefix = loc;
}
public String getPrefix() {return prefix;}
- }
- public void addPrefixedFileSet(PrefixedFileSet fs) {
- locFileSets.addElement(fs);
+ public void setFullpath(String loc) {
+ fullpath = loc;
+ }
+
+ public String getFullpath() {return fullpath;}
}
/**
@@ -158,6 +163,7 @@
throw new BuildException("You must specify the " + archiveType + "
file to create!");
}
+ // Create the scanners to pass to isUpToDate().
Vector dss = new Vector ();
if (baseDir != null)
dss.addElement(getDirectoryScanner(baseDir));
@@ -166,11 +172,9 @@
dss.addElement (fs.getDirectoryScanner(project));
}
int dssSize = dss.size();
- FileScanner[] scanners = new FileScanner[dssSize + locFileSets.size()];
+ FileScanner[] scanners = new FileScanner[dssSize];
dss.copyInto(scanners);
- addScanners(scanners, dssSize, locFileSets);
-
// quick exit if the target is up to date
// can also handle empty archives
if (isUpToDate(scanners, zipFile)) return;
@@ -178,6 +182,7 @@
log("Building "+ archiveType +": "+ zipFile.getAbsolutePath());
try {
+ boolean success = false;
ZipOutputStream zOut = new ZipOutputStream(new
FileOutputStream(zipFile));
try {
if (doCompress) {
@@ -187,13 +192,27 @@
}
initZipOutputStream(zOut);
- addPrefixedFiles(locFileSets, zOut);
-
- for (int j = 0; j < dssSize; j++) {
- addFiles(scanners[j], zOut, "");
- }
+ // Add the implicit fileset to the archive.
+ if (baseDir != null)
+ addFiles(getDirectoryScanner(baseDir), zOut, "", "");
+ // Add the explicit filesets to the archive.
+ addFiles(filesets, zOut);
+ success = true;
} finally {
- zOut.close ();
+ // Close the output stream.
+ try {
+ if (zOut != null)
+ zOut.close ();
+ } catch(IOException ex) {
+ // If we're in this finally clause because of an
exception, we don't
+ // really care if there's an exception when closing the
stream. E.g. if it
+ // throws "ZIP file must have at least one entry", because
an exception happened
+ // before we added any files, then we must swallow this
exception. Otherwise,
+ // the error that's reported will be the close() error,
which is not the real
+ // cause of the problem.
+ if (success)
+ throw ex;
+ }
}
} catch (IOException ioe) {
String msg = "Problem creating " + archiveType + ": " +
ioe.getMessage();
@@ -208,29 +227,22 @@
}
/**
- * Add a DirectoryScanner for each FileSet included in fileSets to scanners
- * starting with index startIndex.
- */
- protected void addScanners(FileScanner[] scanners, int startIndex,
- Vector fileSets) {
- for (int i=0; i<fileSets.size(); i++) {
- FileSet fs = (FileSet) fileSets.elementAt(i);
- scanners[startIndex+i] = fs.getDirectoryScanner(project);
- }
- }
-
- /**
* Add all files of the given FileScanner to the ZipOutputStream
* prependig the given prefix to each filename.
*
* <p>Ensure parent directories have been added as well.
*/
protected void addFiles(FileScanner scanner, ZipOutputStream zOut,
- String prefix) throws IOException {
+ String prefix, String fullpath) throws IOException
{
+ if (prefix.length() > 0 && fullpath.length() > 0)
+ throw new BuildException("Both prefix and fullpath attributes may
not be set on the same fileset.");
+
File thisBaseDir = scanner.getBasedir();
// directories that matched include patterns
String[] dirs = scanner.getIncludedDirectories();
+ if (dirs.length > 0 && fullpath.length() > 0)
+ throw new BuildException("fullpath attribute may only be specified
for filesets that specify a single file.");
for (int i = 0; i < dirs.length; i++) {
String name = dirs[i].replace(File.separatorChar,'/');
if (!name.endsWith("/")) {
@@ -241,11 +253,23 @@
// files that matched include patterns
String[] files = scanner.getIncludedFiles();
+ if (files.length > 1 && fullpath.length() > 0)
+ throw new BuildException("fullpath attribute may only be specified
for filesets that specify a single file.");
for (int i = 0; i < files.length; i++) {
File f = new File(thisBaseDir, files[i]);
- String name = files[i].replace(File.separatorChar,'/');
- addParentDirs(thisBaseDir, name, zOut, prefix);
- zipFile(f, zOut, prefix+name);
+ if (fullpath.length() > 0)
+ {
+ // Add this file at the specified location.
+ addParentDirs(null, fullpath, zOut, "");
+ zipFile(f, zOut, fullpath);
+ }
+ else
+ {
+ // Add this file with the specified prefix.
+ String name = files[i].replace(File.separatorChar,'/');
+ addParentDirs(thisBaseDir, name, zOut, prefix);
+ zipFile(f, zOut, prefix+name);
+ }
}
}
@@ -459,27 +483,15 @@
}
}
- /**
- * Iterate over the given Vector of filesets and add all files to the
- * ZipOutputStream using the given prefix.
- */
- protected void addFiles(Vector v, ZipOutputStream zOut, String prefix)
- throws IOException {
- for (int i=0; i<v.size(); i++) {
- FileSet fs = (FileSet) v.elementAt(i);
- DirectoryScanner ds = fs.getDirectoryScanner(project);
- addFiles(ds, zOut, prefix);
- }
- }
-
/**
- * Iterate over the given Vector of relocatablefilesets and add
+ * Iterate over the given Vector of prefixedfilesets and add
* all files to the ZipOutputStream using the given prefix.
*/
- protected void addPrefixedFiles(Vector v, ZipOutputStream zOut)
+ protected void addFiles(Vector filesets, ZipOutputStream zOut)
throws IOException {
- for (int i=0; i<v.size(); i++) {
- PrefixedFileSet fs = (PrefixedFileSet) v.elementAt(i);
+ // Add each fileset in the Vector.
+ for (int i = 0; i<filesets.size(); i++) {
+ PrefixedFileSet fs = (PrefixedFileSet) filesets.elementAt(i);
DirectoryScanner ds = fs.getDirectoryScanner(project);
String prefix = fs.getPrefix();
if (prefix.length() > 0
@@ -487,8 +499,17 @@
&& !prefix.endsWith("\\")) {
prefix += "/";
}
- zipDir(null, zOut, prefix);
- addFiles(ds, zOut, prefix);
+ String fullpath = fs.getFullpath();
+ // Need to manually add either fullpath's parent directory, or
+ // the prefix directory, to the archive.
+ if (prefix.length() > 0) {
+ addParentDirs(null, prefix, zOut, "");
+ zipDir(null, zOut, prefix);
+ } else if (fullpath.length() > 0) {
+ addParentDirs(null, fullpath, zOut, "");
+ }
+ // Add the fileset.
+ addFiles(ds, zOut, prefix, fullpath);
}
}
}
