fix PackWriter excluded objects handling

PackWriter supports excluding objects from being written to the pack.
You may specify a PackIndex which lists all those objects which should
not go into the new pack. This feature was broken because not all
commits have been checked whether they should be excluded or not. For
other object types the exclude algorithm worked. This commit adds the
missing check.

Change-Id: Id0047098393641ccba784c58b8325175c22fcece
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/PackWriterTest.java

@@ -63,10 +63,14 @@ import java.util.Set;
 
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.junit.JGitTestUtil;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.SampleDataRepositoryTestCase;
+import org.eclipse.jgit.revwalk.RevBlob;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.file.PackIndex.MutableEntry;
@@ -447,6 +451,68 @@ public class PackWriterTest extends SampleDataRepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void testExclude() throws Exception {
+		FileRepository repo = createBareRepository();
+
+		TestRepository<FileRepository> testRepo = new TestRepository<FileRepository>(
+				repo);
+		BranchBuilder bb = testRepo.branch("refs/heads/master");
+		RevBlob contentA = testRepo.blob("A");
+		RevCommit c1 = bb.commit().add("f", contentA).create();
+		testRepo.getRevWalk().parseHeaders(c1);
+		PackIndex pf1 = writePack(repo, Collections.singleton(c1),
+				Collections.<PackIndex> emptySet());
+		assertContent(
+				pf1,
+				Arrays.asList(c1.getId(), c1.getTree().getId(),
+						contentA.getId()));
+		RevBlob contentB = testRepo.blob("B");
+		RevCommit c2 = bb.commit().add("f", contentB).create();
+		testRepo.getRevWalk().parseHeaders(c2);
+		PackIndex pf2 = writePack(repo, Collections.singleton(c2),
+				Collections.singleton(pf1));
+		assertContent(
+				pf2,
+				Arrays.asList(c2.getId(), c2.getTree().getId(),
+						contentB.getId()));
+	}
+
+	private void assertContent(PackIndex pi, List<ObjectId> expected) {
+		assertEquals("Pack index has wrong size.", expected.size(),
+				pi.getObjectCount());
+		for (int i = 0; i < pi.getObjectCount(); i++)
+			assertTrue(
+					"Pack index didn't contain the expected id "
+							+ pi.getObjectId(i),
+					expected.contains(pi.getObjectId(i)));
+	}
+
+	private PackIndex writePack(FileRepository repo,
+			Set<? extends ObjectId> want, Set<PackIndex> excludeObjects)
+			throws IOException {
+		PackWriter pw = new PackWriter(repo);
+		pw.setDeltaBaseAsOffset(true);
+		pw.setReuseDeltaCommits(false);
+		for (PackIndex idx : excludeObjects)
+			pw.excludeObjects(idx);
+		pw.preparePack(NullProgressMonitor.INSTANCE, want,
+				Collections.<ObjectId> emptySet());
+		String id = pw.computeName().getName();
+		File packdir = new File(repo.getObjectsDirectory(), "pack");
+		File packFile = new File(packdir, "pack-" + id + ".pack");
+		FileOutputStream packOS = new FileOutputStream(packFile);
+		pw.writePack(NullProgressMonitor.INSTANCE,
+				NullProgressMonitor.INSTANCE, packOS);
+		packOS.close();
+		File idxFile = new File(packdir, "pack-" + id + ".idx");
+		FileOutputStream idxOS = new FileOutputStream(idxFile);
+		pw.writeIndex(idxOS);
+		idxOS.close();
+		pw.release();
+		return PackIndex.open(idxFile);
+	}
+
 	// TODO: testWritePackDeltasCycle()
 	// TODO: testWritePackDeltasDepth()
 

org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java

@@ -1660,7 +1660,8 @@ public class PackWriter {
 
 				for (int i = 0; i < cmit.getParentCount(); i++) {
 					RevCommit p = cmit.getParent(i);
-					if (!p.has(added) && !p.has(RevFlag.UNINTERESTING)) {
+					if (!p.has(added) && !p.has(RevFlag.UNINTERESTING)
+							&& !exclude(p)) {
 						p.add(added);
 						addObject(p, 0);
 						commitCnt++;