bug 274181 - jars don't have directory entriesv20090428-2350

4 files changed, 61 insertions, 17 deletions

diff --git a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java
index 5533b6515..6498ccbed 100644
--- a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java
+++ b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/FileUtils.java
@@ -12,6 +12,7 @@ package org.eclipse.equinox.internal.p2.core.helpers;
 
 import java.io.*;
 import java.util.*;
+import java.util.jar.JarFile;
 import java.util.zip.*;
 import org.eclipse.core.runtime.*;
 import org.eclipse.osgi.util.NLS;
@@ -243,10 +244,11 @@ public class FileUtils {
 		FileOutputStream fileOutput = new FileOutputStream(destinationArchive);
 		ZipOutputStream output = new ZipOutputStream(fileOutput);
 		HashSet exclusionSet = exclusions == null ? new HashSet() : new HashSet(Arrays.asList(exclusions));
+		HashSet directoryEntries = new HashSet();
 		try {
 			for (int i = 0; i < inclusions.length; i++) {
 				pathComputer.reset();
-				zip(output, inclusions[i], exclusionSet, pathComputer);
+				zip(output, inclusions[i], exclusionSet, pathComputer, directoryEntries);
 			}
 		} finally {
 			try {
@@ -272,27 +274,32 @@ public class FileUtils {
 	 * @throws IOException
 	 */
 	public static void zip(ZipOutputStream output, File source, Set exclusions, IPathComputer pathComputer) throws IOException {
+		zip(output, source, exclusions, pathComputer, new HashSet());
+	}
+
+	public static void zip(ZipOutputStream output, File source, Set exclusions, IPathComputer pathComputer, Set directoryEntries) throws IOException {
 		if (exclusions.contains(source))
 			return;
 		if (source.isDirectory()) //if the file path is a URL then isDir and isFile are both false
-			zipDir(output, source, exclusions, pathComputer);
+			zipDir(output, source, exclusions, pathComputer, directoryEntries);
 		else
-			zipFile(output, source, pathComputer);
+			zipFile(output, source, pathComputer, directoryEntries);
 	}
 
-	/*
-	 * Zip the contents of the given directory into the zip file represented by
-	 * the given zip stream. Prepend the given prefix to the file paths.
-	 */
-	private static void zipDir(ZipOutputStream output, File source, Set exclusions, IPathComputer pathComputer) throws IOException {
-		File[] files = source.listFiles();
-		if (files.length == 0) {
+	private static void zipDirectoryEntry(ZipOutputStream output, IPath entry, long time, Set directoryEntries) throws IOException {
+		entry = entry.addTrailingSeparator();
+		if (!directoryEntries.contains(entry)) {
+			//make sure parent entries are in the zip
+			if (entry.segmentCount() > 1)
+				zipDirectoryEntry(output, entry.removeLastSegments(1), time, directoryEntries);
+
 			try {
-				ZipEntry dirEntry = new ZipEntry(pathComputer.computePath(source).toString() + "/"); //$NON-NLS-1$
-				dirEntry.setTime(source.lastModified());
+				ZipEntry dirEntry = new ZipEntry(entry.toString());
+				dirEntry.setTime(time);
 				output.putNextEntry(dirEntry);
+				directoryEntries.add(entry);
 			} catch (ZipException ze) {
-				//TODO: something about duplicate entries
+				//duplicate entries shouldn't happen because we checked the set
 			} finally {
 				try {
 					output.closeEntry();
@@ -301,6 +308,17 @@ public class FileUtils {
 				}
 			}
 		}
+	}
+
+	/*
+	 * Zip the contents of the given directory into the zip file represented by
+	 * the given zip stream. Prepend the given prefix to the file paths.
+	 */
+	private static void zipDir(ZipOutputStream output, File source, Set exclusions, IPathComputer pathComputer, Set directoryEntries) throws IOException {
+		File[] files = source.listFiles();
+		if (files.length == 0) {
+			zipDirectoryEntry(output, pathComputer.computePath(source), source.lastModified(), directoryEntries);
+		}
 
 		// Different OSs return files in a different order.  This affects the creation
 		// the dynamic path computer.  To address this, we sort the files such that
@@ -327,19 +345,31 @@ public class FileUtils {
 		});
 
 		for (int i = 0; i < files.length; i++)
-			zip(output, files[i], exclusions, pathComputer);
+			zip(output, files[i], exclusions, pathComputer, directoryEntries);
 	}
 
 	/*
 	 * Add the given file to the zip file represented by the specified stream.
 	 * Prepend the given prefix to the path of the file.
 	 */
-	private static void zipFile(ZipOutputStream output, File source, IPathComputer pathComputer) throws IOException {
-		InputStream input = new FileInputStream(source);
+	private static void zipFile(ZipOutputStream output, File source, IPathComputer pathComputer, Set directoryEntries) throws IOException {
+		boolean isManifest = false; //manifest files are special
+		InputStream input = new BufferedInputStream(new FileInputStream(source));
 		try {
 			IPath entryPath = pathComputer.computePath(source);
+			if (entryPath.isAbsolute())
+				throw new IOException(Messages.Util_Absolute_Entry);
 			if (entryPath.segmentCount() == 0)
-				throw new IOException("Cannot have an empty zip entry."); //$NON-NLS-1$
+				throw new IOException(Messages.Util_Empty_Zip_Entry);
+
+			//make sure parent directory entries are in the zip
+			if (entryPath.segmentCount() > 1) {
+				//manifest files should be first, add their directory entry afterwards
+				isManifest = JarFile.MANIFEST_NAME.equals(entryPath.toString());
+				if (!isManifest)
+					zipDirectoryEntry(output, entryPath.removeLastSegments(1), source.lastModified(), directoryEntries);
+			}
+
 			ZipEntry zipEntry = new ZipEntry(entryPath.toString());
 			zipEntry.setTime(source.lastModified());
 			output.putNextEntry(zipEntry);
@@ -358,6 +388,10 @@ public class FileUtils {
 				// ignore
 			}
 		}
+
+		if (isManifest) {
+			zipDirectoryEntry(output, new Path("META-INF"), source.lastModified(), directoryEntries); //$NON-NLS-1$
+		}
 	}
 
 	/**
diff --git a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/Messages.java b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/Messages.java
index 8bf01043a..418d4fe92 100644
--- a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/Messages.java
+++ b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/Messages.java
@@ -27,5 +27,7 @@ class Messages extends NLS {
 
 	public static String Util_Invalid_Zip_File_Format;
 	public static String Util_Error_Unzipping;
+	public static String Util_Empty_Zip_Entry;
+	public static String Util_Absolute_Entry;
 
 }
diff --git a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/messages.properties b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/messages.properties
index f3cafadff..006f18ce7 100644
--- a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/messages.properties
+++ b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/messages.properties
@@ -11,3 +11,6 @@
 
 Util_Invalid_Zip_File_Format=Invalid zip file format
 Util_Error_Unzipping=Error unzipping {0}: {1}
+
+Util_Empty_Zip_Entry=Cannot have an empty zip entry.
+Util_Absolute_Entry=Zip entries cannot be absolute.
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java
index e1478ee64..0a1b2862c 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/core/FileUtilsTest.java
@@ -108,6 +108,7 @@ public class FileUtilsTest extends AbstractProvisioningTest {
 		assertTrue("2.2", archive.length() > 0);
 		assertExists("2.3", archive, "a.txt");
 		assertExists("2.4", archive, "b/b.txt");
+		assertExists("2.5", archive, "b/");
 	}
 
 	public void testZipDynamicPathComputer() {
@@ -142,7 +143,9 @@ public class FileUtilsTest extends AbstractProvisioningTest {
 		assertTrue("3.1", archive.exists());
 		assertTrue("3.2", archive.length() > 0);
 		assertExists("3.3", archive, "a2/a.txt");
+		assertExists("3.3.1", archive, "a2/");
 		assertExists("3.4", archive, "b/b.txt");
+		assertExists("3.4.1", archive, "b/");
 
 		archive = new File(temp, getUniqueString() + ".zip");
 		File[] input = new File[] {getTestData("4.0", "testData/core/x/y"), getTestData("4.0", "testData/core/z")};
@@ -154,7 +157,9 @@ public class FileUtilsTest extends AbstractProvisioningTest {
 		assertTrue("4.1", archive.exists());
 		assertTrue("4.2", archive.length() > 0);
 		assertExists("4.3", archive, "features/feature.txt");
+		assertExists("4.3.1", archive, "features/");
 		assertExists("4.4", archive, "plugins/bundle.txt");
+		assertExists("4.4.1", archive, "plugins/");
 	}
 
 	public void testZipParentPrefixComputer() {