Replace useminimalnegotation flag with maxhaves flag

Client will stop sending haves when the number of haves sent reaches maxhaves.

Change-Id: I1e5b1525be4c67f20a81ca24a2770c20eb5c1271
Signed-off-by: Minh Thai <mthai@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java

@@ -77,6 +77,7 @@ public class TestProtocolTest {
 			"+refs/heads/master:refs/heads/master");
 
 	private static final int HAVES_PER_ROUND = 32;
+	private static final int MAX_HAVES = 256;
 
 	private static class User {
 		private final String name;
@@ -187,7 +188,7 @@ public class TestProtocolTest {
 	}
 
 	@Test
-	public void testMinimalNegotiation() throws Exception {
+	public void testMaxHaves() throws Exception {
 		TestProtocol<User> proto = registerDefault();
 		URIish uri = proto.register(new User("user"), remote.getRepository());
 
@@ -200,28 +201,13 @@ public class TestProtocolTest {
 		RevCommit master = remote.branch("master").commit()
 				.add("readme.txt", "unique commit").create();
 
-		TestProtocol.setFetchConfig(new FetchConfig(true, true));
+		TestProtocol.setFetchConfig(new FetchConfig(true, MAX_HAVES));
 		try (Git git = new Git(local.getRepository())) {
 			assertNull(local.getRepository().exactRef("refs/heads/master"));
 			git.fetch().setRemote(uri.toString()).setRefSpecs(MASTER).call();
 			assertEquals(master, local.getRepository()
 					.exactRef("refs/heads/master").getObjectId());
-			assertTrue(havesCount <= 2 * HAVES_PER_ROUND);
+			assertTrue(havesCount <= MAX_HAVES);
-
-			// Update the remote master and add local branches for 5 more rounds
-			// of negotiation, where the local branch commits have newer
-			// timestamps. Negotiation should send 5 rounds for those newer
-			// branches before getting to the round that sends its stale version
-			// of master.
-			master = remote.branch("master").commit().parent(master).create();
-			local.tick(2 * HAVES_PER_ROUND);
-			for (int i = 0; i < 5 * HAVES_PER_ROUND; i++) {
-				local.branch("local-" + i).commit().create();
-			}
-			git.fetch().setRemote(uri.toString()).setRefSpecs(MASTER).call();
-			assertEquals(master, local.getRepository()
-					.exactRef("refs/heads/master").getObjectId());
-			assertEquals(6 * HAVES_PER_ROUND, havesCount);
 		}
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java

@@ -52,7 +52,6 @@ import java.text.MessageFormat;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.Set;
 
 import org.eclipse.jgit.errors.PackProtocolException;
@@ -235,12 +234,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 
 	private boolean noProgress;
 
-	private Set<AnyObjectId> minimalNegotiationSet;
-
 	private String lockMessage;
 
 	private PackLock packLock;
 
+	private int maxHaves;
+
 	/** RPC state, if {@link BasePackConnection#statelessRPC} is true. */
 	private TemporaryBuffer.Heap state;
 
@@ -261,12 +260,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 		if (local != null) {
 			final FetchConfig cfg = getFetchConfig();
 			allowOfsDelta = cfg.allowOfsDelta;
-			if (cfg.minimalNegotiation) {
+			maxHaves = cfg.maxHaves;
-				minimalNegotiationSet = new HashSet<>();
-			}
 		} else {
 			allowOfsDelta = true;
+			maxHaves = Integer.MAX_VALUE;
 		}
+
 		includeTags = transport.getTagOpt() != TagOpt.NO_TAGS;
 		thinPack = transport.isFetchThin();
 		filterBlobLimit = transport.getFilterBlobLimit();
@@ -294,17 +293,16 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 	static class FetchConfig {
 		final boolean allowOfsDelta;
 
-		final boolean minimalNegotiation;
+		final int maxHaves;
 
 		FetchConfig(Config c) {
 			allowOfsDelta = c.getBoolean("repack", "usedeltabaseoffset", true); //$NON-NLS-1$ //$NON-NLS-2$
-			minimalNegotiation = c.getBoolean("fetch", "useminimalnegotiation", //$NON-NLS-1$ //$NON-NLS-2$
+			maxHaves = c.getInt("fetch", "maxhaves", Integer.MAX_VALUE); //$NON-NLS-1$ //$NON-NLS-2$
-					false);
 		}
 
-		FetchConfig(boolean allowOfsDelta, boolean minimalNegotiation) {
+		FetchConfig(boolean allowOfsDelta, int maxHaves) {
 			this.allowOfsDelta = allowOfsDelta;
-			this.minimalNegotiation = minimalNegotiation;
+			this.maxHaves = maxHaves;
 		}
 	}
 
@@ -518,15 +516,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			}
 			line.append('\n');
 			p.writeString(line.toString());
-			if (minimalNegotiationSet != null) {
-				Ref current = local.exactRef(r.getName());
-				if (current != null) {
-					ObjectId o = current.getObjectId();
-					if (o != null && !o.equals(ObjectId.zeroId())) {
-						minimalNegotiationSet.add(o);
-					}
-				}
-			}
 		}
 		if (first) {
 			return false;
@@ -610,9 +599,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 			pckOut.writeString("have " + o.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
 			havesSent++;
 			havesSinceLastContinue++;
-			if (minimalNegotiationSet != null) {
-				minimalNegotiationSet.remove(o);
-			}
 
 			if ((31 & havesSent) != 0) {
 				// We group the have lines into blocks of 32, each marked
@@ -646,16 +632,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 					// pack on the remote side. Keep doing that.
 					//
 					resultsPending--;
-					if (minimalNegotiationSet != null
-							&& minimalNegotiationSet.isEmpty()) {
-						// Minimal negotiation was requested and we sent out our
-						// current reference values for our wants, so terminate
-						// negotiation early.
-						if (statelessRPC) {
-							state.writeTo(out, null);
-						}
-						break SEND_HAVES;
-					}
 					break READ_RESULT;
 
 				case ACK:
@@ -686,14 +662,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 					if (anr == AckNackResult.ACK_READY) {
 						receivedReady = true;
 					}
-					if (minimalNegotiationSet != null && minimalNegotiationSet.isEmpty()) {
-						// Minimal negotiation was requested and we sent out our current reference
-						// values for our wants, so terminate negotiation early.
-						if (statelessRPC) {
-							state.writeTo(out, null);
-						}
-						break SEND_HAVES;
-					}
 					break;
 				}
 
@@ -709,7 +677,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 				state.writeTo(out, null);
 			}
 
-			if (receivedContinue && havesSinceLastContinue > MAX_HAVES) {
+			if (receivedContinue && havesSinceLastContinue > MAX_HAVES
+					|| havesSent >= maxHaves) {
 				// Our history must be really different from the remote's.
 				// We just sent a whole slew of have lines, and it did not
 				// recognize any of them. Avoid sending our entire history