From 9387288a8612eb9e9478c5ecfe4b1351a0ed4717 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 16 Jun 2019 23:58:06 +0200 Subject: [PATCH] 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 --- org.eclipse.jgit.test/META-INF/MANIFEST.MF | 8 + org.eclipse.jgit.test/pom.xml | 6 + .../eclipse/jgit/api/ArchiveCommandTest.java | 191 +++++++++++++++++- .../org/eclipse/jgit/api/ArchiveCommand.java | 35 +++- 4 files changed, 226 insertions(+), 14 deletions(-) diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF index 69ea99b4f..f469173aa 100644 --- a/org.eclipse.jgit.test/META-INF/MANIFEST.MF +++ b/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)", diff --git a/org.eclipse.jgit.test/pom.xml b/org.eclipse.jgit.test/pom.xml index f42eb54ed..3a887efb8 100644 --- a/org.eclipse.jgit.test/pom.xml +++ b/org.eclipse.jgit.test/pom.xml @@ -112,6 +112,12 @@ org.eclipse.jgit.pgm ${project.version} + + + org.tukaani + xz + true + diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ArchiveCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ArchiveCommandTest.java index 4883bcacc..fbec024a8 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ArchiveCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ArchiveCommandTest.java @@ -46,20 +46,44 @@ 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 @@ 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 void archiveByDirectoryPath() throws GitAPIException, IOException { } } + @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 { 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 27bb5a90b..3f7306bf3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java @@ -56,13 +56,18 @@ 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 @@ private OutputStream writeArchive(Format fmt) { 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 @@ private OutputStream writeArchive(Format fmt) { 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 ArchiveCommand setPaths(String... paths) { 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; + } + }