archive: Include entries for directories

Entries for directories are optional and mostly wasted space in most
archive formats (except as a place to hang ownership and filesystem
permissions), but "git archive" includes them.  Follow suit.

This will make it easier in a later change to include empty
directories as placeholders for missing submodules.

Change-Id: I1810c686bcc9eb4d73498e4d3e763e18787b088a
Signed-off-by: Jonathan Nieder <jrn@google.com>

6 files changed, 143 insertions, 26 deletions

diff --git a/org.eclipse.jgit.archive/resources/org/eclipse/jgit/archive/internal/ArchiveText.properties b/org.eclipse.jgit.archive/resources/org/eclipse/jgit/archive/internal/ArchiveText.properties
new file mode 100644
index 0000000000..fecf99e575
--- /dev/null
+++ b/org.eclipse.jgit.archive/resources/org/eclipse/jgit/archive/internal/ArchiveText.properties
@@ -0,0 +1,2 @@
+pathDoesNotMatchMode=Path {0} does not match mode {1}
+unsupportedMode=Unsupported mode {0}
diff --git a/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/TarFormat.java b/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/TarFormat.java
index 23f4beda14..7e1c9cc907 100644
--- a/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/TarFormat.java
+++ b/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/TarFormat.java
@@ -47,12 +47,14 @@ import java.io.OutputStream;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.text.MessageFormat;
 
 import org.apache.commons.compress.archivers.ArchiveOutputStream;
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
 import org.apache.commons.compress.archivers.tar.TarConstants;
 import org.eclipse.jgit.api.ArchiveCommand;
+import org.eclipse.jgit.archive.internal.ArchiveText;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectLoader;
 
@@ -83,13 +85,29 @@ public class TarFormat implements ArchiveCommand.Format<ArchiveOutputStream> {
 			return;
 		}
 
+		// TarArchiveEntry detects directories by checking
+		// for '/' at the end of the filename.
+		if (path.endsWith("/") && mode != FileMode.TREE)
+			throw new IllegalArgumentException(MessageFormat.format(
+					ArchiveText.get().pathDoesNotMatchMode, path, mode));
+		if (!path.endsWith("/") && mode == FileMode.TREE)
+			path = path + "/";
+
 		final TarArchiveEntry entry = new TarArchiveEntry(path);
-		if (mode == FileMode.REGULAR_FILE ||
-		    mode == FileMode.EXECUTABLE_FILE) {
+		if (mode == FileMode.TREE) {
+			out.putArchiveEntry(entry);
+			out.closeArchiveEntry();
+			return;
+		}
+
+		if (mode == FileMode.REGULAR_FILE) {
+			// ok
+		} else if (mode == FileMode.EXECUTABLE_FILE) {
 			entry.setMode(mode.getBits());
 		} else {
-			// TODO(jrn): Let the caller know the tree contained
-			// an entry with unsupported mode (e.g., a submodule).
+			// Unsupported mode (e.g., GITLINK).
+			throw new IllegalArgumentException(MessageFormat.format(
+					ArchiveText.get().unsupportedMode, mode));
 		}
 		entry.setSize(loader.getSize());
 		out.putArchiveEntry(entry);
diff --git a/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/ZipFormat.java b/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/ZipFormat.java
index 00c962bc98..1a3765ec19 100644
--- a/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/ZipFormat.java
+++ b/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/ZipFormat.java
@@ -44,6 +44,7 @@ package org.eclipse.jgit.archive;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -52,6 +53,7 @@ import org.apache.commons.compress.archivers.ArchiveOutputStream;
 import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
 import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
 import org.eclipse.jgit.api.ArchiveCommand;
+import org.eclipse.jgit.archive.internal.ArchiveText;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectLoader;
 
@@ -69,7 +71,20 @@ public class ZipFormat implements ArchiveCommand.Format<ArchiveOutputStream> {
 	public void putEntry(ArchiveOutputStream out,
 			String path, FileMode mode, ObjectLoader loader)
 			throws IOException {
+		// ZipArchiveEntry detects directories by checking
+		// for '/' at the end of the filename.
+		if (path.endsWith("/") && mode != FileMode.TREE)
+			throw new IllegalArgumentException(MessageFormat.format(
+					ArchiveText.get().pathDoesNotMatchMode, path, mode));
+		if (!path.endsWith("/") && mode == FileMode.TREE)
+			path = path + "/";
+
 		final ZipArchiveEntry entry = new ZipArchiveEntry(path);
+		if (mode == FileMode.TREE) {
+			out.putArchiveEntry(entry);
+			out.closeArchiveEntry();
+			return;
+		}
 
 		if (mode == FileMode.REGULAR_FILE) {
 			// ok
@@ -77,8 +92,9 @@ public class ZipFormat implements ArchiveCommand.Format<ArchiveOutputStream> {
 				|| mode == FileMode.SYMLINK) {
 			entry.setUnixMode(mode.getBits());
 		} else {
-			// TODO(jrn): Let the caller know the tree contained
-			// an entry with unsupported mode (e.g., a submodule).
+			// Unsupported mode (e.g., GITLINK).
+			throw new IllegalArgumentException(MessageFormat.format(
+					ArchiveText.get().unsupportedMode, mode));
 		}
 		entry.setSize(loader.getSize());
 		out.putArchiveEntry(entry);
diff --git a/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/internal/ArchiveText.java b/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/internal/ArchiveText.java
new file mode 100644
index 0000000000..edadf1c81d
--- /dev/null
+++ b/org.eclipse.jgit.archive/src/org/eclipse/jgit/archive/internal/ArchiveText.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2013, Google Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.archive.internal;
+
+import org.eclipse.jgit.nls.NLS;
+import org.eclipse.jgit.nls.TranslationBundle;
+
+/**
+ * Translation bundle for archivers
+ */
+public class ArchiveText extends TranslationBundle {
+	/**
+	 * @return an instance of this translation bundle
+	 */
+	public static ArchiveText get() {
+		return NLS.getBundleFor(ArchiveText.class);
+	}
+
+	// @formatter:off
+	/***/ public String pathDoesNotMatchMode;
+	/***/ public String unsupportedMode;
+}
diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java
index 816094aef9..30f875ca52 100644
--- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java
+++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java
@@ -43,6 +43,7 @@
 package org.eclipse.jgit.pgm;
 
 import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.junit.Assume.assumeNoException;
 
@@ -304,7 +305,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 
 		final byte[] result = CLIGitCommand.rawExecute( //
 				"git archive --format=zip master", db);
-		String[] expect = { "a", "b.c", "b0c", "b/a", "b/b", "c" };
+		String[] expect = { "a", "b.c", "b0c", "b/", "b/a", "b/b", "c" };
 		String[] actual = listZipEntries(result);
 
 		Arrays.sort(expect);
@@ -330,7 +331,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 
 		final byte[] result = CLIGitCommand.rawExecute( //
 				"git archive --format=tar master", db);
-		String[] expect = { "a", "b.c", "b0c", "b/a", "b/b", "c" };
+		String[] expect = { "a", "b.c", "b0c", "b/", "b/a", "b/b", "c" };
 		String[] actual = listTarEntries(result);
 
 		Arrays.sort(expect);
@@ -351,7 +352,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		commitBazAndFooSlashBar();
 		byte[] result = CLIGitCommand.rawExecute(
 				"git archive --prefix=x/ --format=zip master", db);
-		String[] expect = { "x/baz", "x/foo/bar" };
+		String[] expect = { "x/baz", "x/foo/", "x/foo/bar" };
 		String[] actual = listZipEntries(result);
 
 		Arrays.sort(expect);
@@ -364,7 +365,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		commitBazAndFooSlashBar();
 		byte[] result = CLIGitCommand.rawExecute(
 				"git archive --prefix=x/ --format=tar master", db);
-		String[] expect = { "x/baz", "x/foo/bar" };
+		String[] expect = { "x/baz", "x/foo/", "x/foo/bar" };
 		String[] actual = listTarEntries(result);
 
 		Arrays.sort(expect);
@@ -410,7 +411,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		commitBazAndFooSlashBar();
 		byte[] result = CLIGitCommand.rawExecute(
 				"git archive --prefix=my- --format=zip master", db);
-		String[] expect = { "my-baz", "my-foo/bar" };
+		String[] expect = { "my-baz", "my-foo/", "my-foo/bar" };
 		String[] actual = listZipEntries(result);
 
 		Arrays.sort(expect);
@@ -423,7 +424,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		commitBazAndFooSlashBar();
 		final byte[] result = CLIGitCommand.rawExecute( //
 				"git archive --prefix=my- --format=tar master", db);
-		String[] expect = { "my-baz", "my-foo/bar" };
+		String[] expect = { "my-baz", "my-foo/", "my-foo/bar" };
 		String[] actual = listTarEntries(result);
 
 		Arrays.sort(expect);
@@ -436,9 +437,11 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		writeTrashFile("plain", "a file with content");
 		writeTrashFile("executable", "an executable file");
 		writeTrashFile("symlink", "plain");
+		writeTrashFile("dir/content", "clutter in a subdir");
 		git.add().addFilepattern("plain").call();
 		git.add().addFilepattern("executable").call();
 		git.add().addFilepattern("symlink").call();
+		git.add().addFilepattern("dir").call();
 
 		DirCache cache = db.lockDirCache();
 		cache.getEntry("executable").setFileMode(FileMode.EXECUTABLE_FILE);
@@ -455,6 +458,7 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		assertContainsEntryWithMode("zip-with-modes.zip", "-rw-", "plain");
 		assertContainsEntryWithMode("zip-with-modes.zip", "-rwx", "executable");
 		assertContainsEntryWithMode("zip-with-modes.zip", "l", "symlink");
+		assertContainsEntryWithMode("zip-with-modes.zip", "-rw-", "dir/");
 	}
 
 	@Test
@@ -462,9 +466,11 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		writeTrashFile("plain", "a file with content");
 		writeTrashFile("executable", "an executable file");
 		writeTrashFile("symlink", "plain");
+		writeTrashFile("dir/content", "clutter in a subdir");
 		git.add().addFilepattern("plain").call();
 		git.add().addFilepattern("executable").call();
 		git.add().addFilepattern("symlink").call();
+		git.add().addFilepattern("dir").call();
 
 		DirCache cache = db.lockDirCache();
 		cache.getEntry("executable").setFileMode(FileMode.EXECUTABLE_FILE);
@@ -481,35 +487,46 @@ public class ArchiveTest extends CLIRepositoryTestCase {
 		assertTarContainsEntry("with-modes.tar", "-rw-r--r--", "plain");
 		assertTarContainsEntry("with-modes.tar", "-rwxr-xr-x", "executable");
 		assertTarContainsEntry("with-modes.tar", "l", "symlink -> plain");
+		assertTarContainsEntry("with-modes.tar", "drwxr-xr-x", "dir/");
 	}
 
 	@Test
 	public void testArchiveWithLongFilename() throws Exception {
-		String filename = "1234567890";
-		for (int i = 0; i < 20; i++)
-			filename = filename + "/1234567890";
+		String filename = "";
+		final List<String> l = new ArrayList<String>();
+		for (int i = 0; i < 20; i++) {
+			filename = filename + "1234567890/";
+			l.add(filename);
+		}
+		filename = filename + "1234567890";
+		l.add(filename);
 		writeTrashFile(filename, "file with long path");
 		git.add().addFilepattern("1234567890").call();
 		git.commit().setMessage("file with long name").call();
 
 		final byte[] result = CLIGitCommand.rawExecute( //
 				"git archive --format=zip HEAD", db);
-		assertArrayEquals(new String[] { filename },
+		assertArrayEquals(l.toArray(new String[l.size()]),
 				listZipEntries(result));
 	}
 
 	@Test
 	public void testTarWithLongFilename() throws Exception {
-		String filename = "1234567890";
-		for (int i = 0; i < 20; i++)
-			filename = filename + "/1234567890";
+		String filename = "";
+		final List<String> l = new ArrayList<String>();
+		for (int i = 0; i < 20; i++) {
+			filename = filename + "1234567890/";
+			l.add(filename);
+		}
+		filename = filename + "1234567890";
+		l.add(filename);
 		writeTrashFile(filename, "file with long path");
 		git.add().addFilepattern("1234567890").call();
 		git.commit().setMessage("file with long name").call();
 
 		final byte[] result = CLIGitCommand.rawExecute( //
 				"git archive --format=tar HEAD", db);
-		assertArrayEquals(new String[] { filename },
+		assertArrayEquals(l.toArray(new String[l.size()]),
 				listTarEntries(result));
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java
index c17330e6f1..2e6b50a6a5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java
@@ -138,11 +138,13 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
 		 *            archive object from createArchiveOutputStream
 		 * @param path
 		 *            full filename relative to the root of the archive
+		 *            (with trailing '/' for directories)
 		 * @param mode
 		 *            mode (for example FileMode.REGULAR_FILE or
 		 *            FileMode.SYMLINK)
 		 * @param loader
-		 *            blob object with data for this entry
+		 *            blob object with data for this entry (null for
+		 *            directories)
 		 * @throws IOException
 		 *            thrown by the underlying output stream for I/O errors
 		 */
@@ -275,16 +277,15 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
 				final RevWalk rw = new RevWalk(walk.getObjectReader());
 
 				walk.reset(rw.parseTree(tree));
-				walk.setRecursive(true);
 				while (walk.next()) {
 					final String name = pfx + walk.getPathString();
 					final FileMode mode = walk.getFileMode(0);
 
-					if (mode == FileMode.TREE)
-						// ZIP entries for directories are optional.
-						// Leave them out, mimicking "git archive".
+					if (walk.isSubtree()) {
+						fmt.putEntry(outa, name + "/", mode, null);
+						walk.enterSubtree();
 						continue;
-
+					}
 					walk.getObjectId(idBuf, 0);
 					fmt.putEntry(outa, name, mode, reader.open(idBuf));
 				}