Shallow fetch: avoid sending unneeded blobs

When doing an incremental fetch from JGit, "have" commits are marked
as "uninteresting". In a non-shallow fetch, when the RevWalk hits an
"uninteresting" commit it marks the commit's corresponding tree as
uninteresting. That has the effect of dropping those trees and all the
trees and blobs they reference out of the thin pack returned to the
client.

However, shallow fetches use a DepthWalk to limit the RevWalk, which
nearly always causes the RevWalk to terminate before encountering the
"have" commits. As a result the pack created for the incremental fetch
never encounters "uninteresting" tree objects and thus includes
duplicate objects that it knows the client already has.

Change-Id: I7b1f7c3b0d83e04d34cd2fa676f1ad4fec904c05
Signed-off-by: Terry Parker <tparker@google.com>

1 files changed, 18 insertions, 2 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
index 525f9aecc7..4c0fc47f67 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java
@@ -1653,6 +1653,8 @@ public class PackWriter implements AutoCloseable {
 		List<RevObject> haveObjs = new ArrayList<RevObject>(haveEst);
 		List<RevTag> wantTags = new ArrayList<RevTag>(want.size());
 
+		// Retrieve the RevWalk's versions of "want" and "have" objects to
+		// maintain any flags previously set in the RevWalk.
 		AsyncRevObjectQueue q = walker.parseAny(all, true);
 		try {
 			for (;;) {
@@ -1695,11 +1697,25 @@ public class PackWriter implements AutoCloseable {
 
 		if (walker instanceof DepthWalk.ObjectWalk) {
 			DepthWalk.ObjectWalk depthWalk = (DepthWalk.ObjectWalk) walker;
-			for (RevObject obj : wantObjs)
+			for (RevObject obj : wantObjs) {
 				depthWalk.markRoot(obj);
+			}
+			// Mark the tree objects associated with "have" commits as
+			// uninteresting to avoid writing redundant blobs. A normal RevWalk
+			// lazily propagates the "uninteresting" state from a commit to its
+			// tree during the walk, but DepthWalks can terminate early so
+			// preemptively propagate that state here.
+			for (RevObject obj : haveObjs) {
+				if (obj instanceof RevCommit) {
+					RevTree t = ((RevCommit) obj).getTree();
+					depthWalk.markUninteresting(t);
+				}
+			}
+
 			if (unshallowObjects != null) {
-				for (ObjectId id : unshallowObjects)
+				for (ObjectId id : unshallowObjects) {
 					depthWalk.markUnshallow(walker.parseAny(id));
+				}
 			}
 		} else {
 			for (RevObject obj : wantObjs)