Fix non-deterministic hash of archives created by ArchiveCommand

Archives created by the ArchiveCommand didn't produce deterministic
archive hashes. For RevCommits RevWalk.parseTree returns the root tree
instead of the RevCommit hence retrieving the commit's timestamp didn't
work. Instead use RevWalk.parseAny and extract the tree manually.

Archive entries store timestamps with 1 second resolution hence we need
to wait longer when creating the same archive twice and compare archive
hashes. Otherwise hash comparison in tests wouldn't fail without this
patch.

Bug: 548312
Change-Id: I437d515de51cf68265584d28a8446cebe6341b79
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/META-INF/MANIFEST.MF

@@ -10,8 +10,15 @@ Bundle-ActivationPolicy: lazy
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
 Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)",
  com.jcraft.jsch;version="[0.1.54,0.2.0)",
+ org.apache.commons.compress.archivers;version="[1.15.0,2.0)",
+ org.apache.commons.compress.archivers.tar;version="[1.15.0,2.0)",
+ org.apache.commons.compress.archivers.zip;version="[1.15.0,2.0)",
+ org.apache.commons.compress.compressors.bzip2;version="[1.15.0,2.0)",
+ org.apache.commons.compress.compressors.gzip;version="[1.15.0,2.0)",
+ org.apache.commons.compress.compressors.xz;version="[1.15.0,2.0)",
  org.eclipse.jgit.api;version="[5.1.9,5.2.0)",
  org.eclipse.jgit.api.errors;version="[5.1.9,5.2.0)",
+ org.eclipse.jgit.archive;version="[5.1.9,5.2.0)",
  org.eclipse.jgit.attributes;version="[5.1.9,5.2.0)",
  org.eclipse.jgit.awtui;version="[5.1.9,5.2.0)",
  org.eclipse.jgit.blame;version="[5.1.9,5.2.0)",
@@ -55,6 +62,7 @@ Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)",
  org.eclipse.jgit.util;version="[5.1.9,5.2.0)",
  org.eclipse.jgit.util.io;version="[5.1.9,5.2.0)",
  org.eclipse.jgit.util.sha1;version="[5.1.9,5.2.0)",
+ org.tukaani.xz;version="[1.6.0,2.0)",
  org.junit;version="[4.12,5.0.0)",
  org.junit.experimental.theories;version="[4.12,5.0.0)",
  org.junit.rules;version="[4.12,5.0.0)",

org.eclipse.jgit.test/pom.xml

@@ -112,6 +112,12 @@
       <artifactId>org.eclipse.jgit.pgm</artifactId>
       <version>${project.version}</version>
     </dependency>
+
+    <dependency>
+      <groupId>org.tukaani</groupId>
+      <artifactId>xz</artifactId>
+      <optional>true</optional>
+    </dependency>
   </dependencies>
 
   <profiles>

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ArchiveCommandTest.java

@@ -46,20 +46,44 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
 import java.beans.Statement;
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.compress.archivers.ArchiveEntry;
+import org.apache.commons.compress.archivers.ArchiveInputStream;
+import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
+import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream;
+import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream;
+import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
+import org.apache.commons.compress.compressors.xz.XZCompressorInputStream;
+import org.eclipse.jgit.api.errors.AbortedByHookException;
+import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
 import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.api.errors.NoFilepatternException;
+import org.eclipse.jgit.api.errors.NoHeadException;
+import org.eclipse.jgit.api.errors.NoMessageException;
+import org.eclipse.jgit.api.errors.UnmergedPathsException;
+import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
+import org.eclipse.jgit.archive.ArchiveFormats;
+import org.eclipse.jgit.errors.AmbiguousObjectException;
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.StringUtils;
 import org.junit.After;
 import org.junit.Before;
@@ -67,9 +91,14 @@ import org.junit.Test;
 
 public class ArchiveCommandTest extends RepositoryTestCase {
 
+	// archives store timestamp with 1 second resolution
+	private static final int WAIT = 2000;
 	private static final String UNEXPECTED_ARCHIVE_SIZE  = "Unexpected archive size";
 	private static final String UNEXPECTED_FILE_CONTENTS = "Unexpected file contents";
 	private static final String UNEXPECTED_TREE_CONTENTS = "Unexpected tree contents";
+	private static final String UNEXPECTED_LAST_MODIFIED =
+			"Unexpected lastModified mocked by MockSystemReader, truncated to 1 second";
+	private static final String UNEXPECTED_DIFFERENT_HASH = "Unexpected different hash";
 
 	private MockFormat format = null;
 
@@ -77,25 +106,20 @@ public class ArchiveCommandTest extends RepositoryTestCase {
 	public void setup() {
 		format = new MockFormat();
 		ArchiveCommand.registerFormat(format.SUFFIXES.get(0), format);
+		ArchiveFormats.registerAll();
 	}
 
 	@Override
 	@After
 	public void tearDown() {
 		ArchiveCommand.unregisterFormat(format.SUFFIXES.get(0));
+		ArchiveFormats.unregisterAll();
 	}
 
 	@Test
 	public void archiveHeadAllFiles() throws IOException, GitAPIException {
 		try (Git git = new Git(db)) {
-			writeTrashFile("file_1.txt", "content_1_1");
-			git.add().addFilepattern("file_1.txt").call();
-			git.commit().setMessage("create file").call();
-
-			writeTrashFile("file_1.txt", "content_1_2");
-			writeTrashFile("file_2.txt", "content_2_2");
-			git.add().addFilepattern(".").call();
-			git.commit().setMessage("updated file").call();
+			createTestContent(git);
 
 			git.archive().setOutputStream(new MockOutputStream())
 					.setFormat(format.SUFFIXES.get(0))
@@ -190,6 +214,157 @@ public class ArchiveCommandTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void archiveHeadAllFilesTarTimestamps() throws Exception {
+		try (Git git = new Git(db)) {
+			createTestContent(git);
+			String fmt = "tar";
+			File archive = new File(getTemporaryDirectory(),
+					"archive." + format);
+			archive(git, archive, fmt);
+			ObjectId hash1 = ObjectId.fromRaw(IO.readFully(archive));
+
+			try (InputStream fi = Files.newInputStream(archive.toPath());
+					InputStream bi = new BufferedInputStream(fi);
+					ArchiveInputStream o = new TarArchiveInputStream(bi)) {
+				assertEntries(o);
+			}
+
+			Thread.sleep(WAIT);
+			archive(git, archive, fmt);
+			assertEquals(UNEXPECTED_DIFFERENT_HASH, hash1,
+					ObjectId.fromRaw(IO.readFully(archive)));
+		}
+	}
+
+	@Test
+	public void archiveHeadAllFilesTgzTimestamps() throws Exception {
+		try (Git git = new Git(db)) {
+			createTestContent(git);
+			String fmt = "tgz";
+			File archive = new File(getTemporaryDirectory(),
+					"archive." + fmt);
+			archive(git, archive, fmt);
+			ObjectId hash1 = ObjectId.fromRaw(IO.readFully(archive));
+
+			try (InputStream fi = Files.newInputStream(archive.toPath());
+					InputStream bi = new BufferedInputStream(fi);
+					InputStream gzi = new GzipCompressorInputStream(bi);
+					ArchiveInputStream o = new TarArchiveInputStream(gzi)) {
+				assertEntries(o);
+			}
+
+			Thread.sleep(WAIT);
+			archive(git, archive, fmt);
+			assertEquals(UNEXPECTED_DIFFERENT_HASH, hash1,
+					ObjectId.fromRaw(IO.readFully(archive)));
+		}
+	}
+
+	@Test
+	public void archiveHeadAllFilesTbz2Timestamps() throws Exception {
+		try (Git git = new Git(db)) {
+			createTestContent(git);
+			String fmt = "tbz2";
+			File archive = new File(getTemporaryDirectory(),
+					"archive." + fmt);
+			archive(git, archive, fmt);
+			ObjectId hash1 = ObjectId.fromRaw(IO.readFully(archive));
+
+			try (InputStream fi = Files.newInputStream(archive.toPath());
+					InputStream bi = new BufferedInputStream(fi);
+					InputStream gzi = new BZip2CompressorInputStream(bi);
+					ArchiveInputStream o = new TarArchiveInputStream(gzi)) {
+				assertEntries(o);
+			}
+
+			Thread.sleep(WAIT);
+			archive(git, archive, fmt);
+			assertEquals(UNEXPECTED_DIFFERENT_HASH, hash1,
+					ObjectId.fromRaw(IO.readFully(archive)));
+		}
+	}
+
+	@Test
+	public void archiveHeadAllFilesTxzTimestamps() throws Exception {
+		try (Git git = new Git(db)) {
+			createTestContent(git);
+			String fmt = "txz";
+			File archive = new File(getTemporaryDirectory(), "archive." + fmt);
+			archive(git, archive, fmt);
+			ObjectId hash1 = ObjectId.fromRaw(IO.readFully(archive));
+
+			try (InputStream fi = Files.newInputStream(archive.toPath());
+					InputStream bi = new BufferedInputStream(fi);
+					InputStream gzi = new XZCompressorInputStream(bi);
+					ArchiveInputStream o = new TarArchiveInputStream(gzi)) {
+				assertEntries(o);
+			}
+
+			Thread.sleep(WAIT);
+			archive(git, archive, fmt);
+			assertEquals(UNEXPECTED_DIFFERENT_HASH, hash1,
+					ObjectId.fromRaw(IO.readFully(archive)));
+		}
+	}
+
+	@Test
+	public void archiveHeadAllFilesZipTimestamps() throws Exception {
+		try (Git git = new Git(db)) {
+			createTestContent(git);
+			String fmt = "zip";
+			File archive = new File(getTemporaryDirectory(), "archive." + fmt);
+			archive(git, archive, fmt);
+			ObjectId hash1 = ObjectId.fromRaw(IO.readFully(archive));
+
+			try (InputStream fi = Files.newInputStream(archive.toPath());
+					InputStream bi = new BufferedInputStream(fi);
+					ArchiveInputStream o = new ZipArchiveInputStream(bi)) {
+				assertEntries(o);
+			}
+
+			Thread.sleep(WAIT);
+			archive(git, archive, fmt);
+			assertEquals(UNEXPECTED_DIFFERENT_HASH, hash1,
+					ObjectId.fromRaw(IO.readFully(archive)));
+		}
+	}
+
+	private void createTestContent(Git git) throws IOException, GitAPIException,
+			NoFilepatternException, NoHeadException, NoMessageException,
+			UnmergedPathsException, ConcurrentRefUpdateException,
+			WrongRepositoryStateException, AbortedByHookException {
+		writeTrashFile("file_1.txt", "content_1_1");
+		git.add().addFilepattern("file_1.txt").call();
+		git.commit().setMessage("create file").call();
+
+		writeTrashFile("file_1.txt", "content_1_2");
+		writeTrashFile("file_2.txt", "content_2_2");
+		git.add().addFilepattern(".").call();
+		git.commit().setMessage("updated file").call();
+	}
+
+	private static void archive(Git git, File archive, String fmt)
+			throws GitAPIException,
+			FileNotFoundException, AmbiguousObjectException,
+			IncorrectObjectTypeException, IOException {
+		git.archive().setOutputStream(new FileOutputStream(archive))
+				.setFormat(fmt)
+				.setTree(git.getRepository().resolve("HEAD")).call();
+	}
+
+	private static void assertEntries(ArchiveInputStream o) throws IOException {
+		ArchiveEntry e;
+		int n = 0;
+		while ((e = o.getNextEntry()) != null) {
+			n++;
+			assertEquals(UNEXPECTED_LAST_MODIFIED,
+					(1250379778668L / 1000L) * 1000L,
+					e.getLastModifiedDate().getTime());
+		}
+		assertEquals(UNEXPECTED_ARCHIVE_SIZE, 2, n);
+	}
+
 	private static class MockFormat
 			implements ArchiveCommand.Format<MockOutputStream> {
 

org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java

@@ -56,13 +56,18 @@ import java.util.concurrent.ConcurrentMap;
 
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.MutableObjectId;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
+import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
@@ -375,13 +380,15 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
 				MutableObjectId idBuf = new MutableObjectId();
 				ObjectReader reader = walk.getObjectReader();
 
-				walk.reset(rw.parseTree(tree));
-				if (!paths.isEmpty())
+				RevObject o = rw.peel(rw.parseAny(tree));
+				walk.reset(getTree(o));
+				if (!paths.isEmpty()) {
 					walk.setFilter(PathFilterGroup.createFromStrings(paths));
+				}
 
 				// Put base directory into archive
 				if (pfx.endsWith("/")) { //$NON-NLS-1$
-					fmt.putEntry(outa, tree, pfx.replaceAll("[/]+$", "/"), //$NON-NLS-1$ //$NON-NLS-2$
+					fmt.putEntry(outa, o, pfx.replaceAll("[/]+$", "/"), //$NON-NLS-1$ //$NON-NLS-2$
 							FileMode.TREE, null);
 				}
 
@@ -392,17 +399,18 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
 					if (walk.isSubtree())
 						walk.enterSubtree();
 
-					if (mode == FileMode.GITLINK)
+					if (mode == FileMode.GITLINK) {
 						// TODO(jrn): Take a callback to recurse
 						// into submodules.
 						mode = FileMode.TREE;
+					}
 
 					if (mode == FileMode.TREE) {
-						fmt.putEntry(outa, tree, name + "/", mode, null); //$NON-NLS-1$
+						fmt.putEntry(outa, o, name + "/", mode, null); //$NON-NLS-1$
 						continue;
 					}
 					walk.getObjectId(idBuf, 0);
-					fmt.putEntry(outa, tree, name, mode, reader.open(idBuf));
+					fmt.putEntry(outa, o, name, mode, reader.open(idBuf));
 				}
 				return out;
 			} finally {
@@ -534,4 +542,19 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
 		this.paths = Arrays.asList(paths);
 		return this;
 	}
+
+	private RevTree getTree(RevObject o)
+			throws IncorrectObjectTypeException {
+		final RevTree t;
+		if (o instanceof RevCommit) {
+			t = ((RevCommit) o).getTree();
+		} else if (!(o instanceof RevTree)) {
+			throw new IncorrectObjectTypeException(tree.toObjectId(),
+					Constants.TYPE_TREE);
+		} else {
+			t = (RevTree) o;
+		}
+		return t;
+	}
+
 }