Merge "BasePackFetchConnection: support negotiationTip feature"
This commit is contained in:
commit
a7f8f66523
|
@ -50,6 +50,12 @@ For details on native git options see also the official [git config documentatio
|
||||||
| `core.trustPackedRefsStat` | `unset` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the packed-refs file. If `never` JGit will ignore the file attributes of the packed-refs file and always read it. If `always` JGit will trust the file attributes of the packed-refs file and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, except that the packed-refs file is opened and closed before its file attributes are considered. An open/close of the packed-refs file is known to refresh its file attributes, at least on some NFS clients. If `unset`, JGit will use the behavior described in `trustFolderStat`. |
|
| `core.trustPackedRefsStat` | `unset` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the packed-refs file. If `never` JGit will ignore the file attributes of the packed-refs file and always read it. If `always` JGit will trust the file attributes of the packed-refs file and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, except that the packed-refs file is opened and closed before its file attributes are considered. An open/close of the packed-refs file is known to refresh its file attributes, at least on some NFS clients. If `unset`, JGit will use the behavior described in `trustFolderStat`. |
|
||||||
| `core.worktree` | Root directory of the working tree if it is not the parent directory of the `.git` directory | ✅ | The path to the root of the working tree. |
|
| `core.worktree` | Root directory of the working tree if it is not the parent directory of the `.git` directory | ✅ | The path to the root of the working tree. |
|
||||||
|
|
||||||
|
## __fetch__ options
|
||||||
|
|
||||||
|
| option | default | git option | description |
|
||||||
|
|---------|---------|------------|-------------|
|
||||||
|
| `fetch.useNegotiationTip` | `false` | ✅ | When enabled it restricts the client negotiation on unrelated branches i.e. only send haves for the refs that the client is interested in fetching. |
|
||||||
|
|
||||||
## __gc__ options
|
## __gc__ options
|
||||||
|
|
||||||
| option | default | git option | description |
|
| option | default | git option | description |
|
||||||
|
|
|
@ -22,9 +22,12 @@
|
||||||
import java.util.Collection;
|
import java.util.Collection;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
|
import java.util.HashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
|
import java.util.Set;
|
||||||
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
import java.util.function.Consumer;
|
import java.util.function.Consumer;
|
||||||
|
|
||||||
import org.eclipse.jgit.dircache.DirCache;
|
import org.eclipse.jgit.dircache.DirCache;
|
||||||
|
@ -53,6 +56,7 @@
|
||||||
import org.eclipse.jgit.revwalk.RevTag;
|
import org.eclipse.jgit.revwalk.RevTag;
|
||||||
import org.eclipse.jgit.revwalk.RevTree;
|
import org.eclipse.jgit.revwalk.RevTree;
|
||||||
import org.eclipse.jgit.storage.pack.PackStatistics;
|
import org.eclipse.jgit.storage.pack.PackStatistics;
|
||||||
|
import org.eclipse.jgit.transport.BasePackFetchConnection.FetchConfig;
|
||||||
import org.eclipse.jgit.transport.UploadPack.RequestPolicy;
|
import org.eclipse.jgit.transport.UploadPack.RequestPolicy;
|
||||||
import org.eclipse.jgit.util.io.NullOutputStream;
|
import org.eclipse.jgit.util.io.NullOutputStream;
|
||||||
import org.junit.After;
|
import org.junit.After;
|
||||||
|
@ -63,6 +67,8 @@
|
||||||
* Tests for server upload-pack utilities.
|
* Tests for server upload-pack utilities.
|
||||||
*/
|
*/
|
||||||
public class UploadPackTest {
|
public class UploadPackTest {
|
||||||
|
private static final int MAX_HAVES = 64;
|
||||||
|
|
||||||
private URIish uri;
|
private URIish uri;
|
||||||
|
|
||||||
private TestProtocol<Object> testProtocol;
|
private TestProtocol<Object> testProtocol;
|
||||||
|
@ -87,6 +93,7 @@ public void setUp() throws Exception {
|
||||||
|
|
||||||
@After
|
@After
|
||||||
public void tearDown() {
|
public void tearDown() {
|
||||||
|
TestProtocol.setFetchConfig(null);
|
||||||
Transport.unregister(testProtocol);
|
Transport.unregister(testProtocol);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2158,6 +2165,231 @@ public void testV2FetchWantRef() throws Exception {
|
||||||
assertFalse(client.getObjectDatabase().has(three.toObjectId()));
|
assertFalse(client.getObjectDatabase().has(three.toObjectId()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* <pre>
|
||||||
|
* remote:
|
||||||
|
* foo <- foofoo <-- branchFoo
|
||||||
|
* bar <- barbar <-- branchBar
|
||||||
|
*
|
||||||
|
* client:
|
||||||
|
* foo <-- branchFoo
|
||||||
|
* bar <-- branchBar
|
||||||
|
*
|
||||||
|
* fetch(branchFoo) should send exactly 1 have (i.e. foo) from branchFoo
|
||||||
|
* </pre>
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
public void testNegotiationTip() throws Exception {
|
||||||
|
RevCommit fooParent = remote.commit().message("foo").create();
|
||||||
|
RevCommit fooChild = remote.commit().message("foofoo").parent(fooParent)
|
||||||
|
.create();
|
||||||
|
RevCommit barParent = remote.commit().message("bar").create();
|
||||||
|
RevCommit barChild = remote.commit().message("barbar").parent(barParent)
|
||||||
|
.create();
|
||||||
|
|
||||||
|
// Remote has branchFoo at fooChild and branchBar at barChild
|
||||||
|
remote.update("branchFoo", fooChild);
|
||||||
|
remote.update("branchBar", barChild);
|
||||||
|
|
||||||
|
AtomicReference<UploadPack> uploadPack = new AtomicReference<>();
|
||||||
|
CountHavesPreUploadHook countHavesHook = new CountHavesPreUploadHook();
|
||||||
|
RevCommit localFooParent = null;
|
||||||
|
|
||||||
|
// Client has lagging branchFoo at fooParent and branchBar at barParent
|
||||||
|
try (TestRepository<InMemoryRepository> clientRepo = new TestRepository<>(
|
||||||
|
client)) {
|
||||||
|
localFooParent = clientRepo.commit().message("foo")
|
||||||
|
.create();
|
||||||
|
RevCommit localBarParent = clientRepo.commit().message("bar")
|
||||||
|
.create();
|
||||||
|
|
||||||
|
clientRepo.update("branchFoo", localFooParent);
|
||||||
|
clientRepo.update("branchBar", localBarParent);
|
||||||
|
|
||||||
|
testProtocol = new TestProtocol<>((Object req, Repository db) -> {
|
||||||
|
UploadPack up = new UploadPack(db);
|
||||||
|
up.setPreUploadHook(countHavesHook);
|
||||||
|
uploadPack.set(up);
|
||||||
|
return up;
|
||||||
|
}, null);
|
||||||
|
|
||||||
|
uri = testProtocol.register(ctx, server);
|
||||||
|
|
||||||
|
TestProtocol.setFetchConfig(new FetchConfig(true, MAX_HAVES, true));
|
||||||
|
try (Transport tn = testProtocol.open(uri,
|
||||||
|
clientRepo.getRepository(), "server")) {
|
||||||
|
|
||||||
|
tn.fetch(NullProgressMonitor.INSTANCE, Collections
|
||||||
|
.singletonList(new RefSpec("refs/heads/branchFoo")),
|
||||||
|
"branchFoo");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
assertTrue(client.getObjectDatabase().has(fooParent.toObjectId()));
|
||||||
|
assertTrue(client.getObjectDatabase().has(fooChild.toObjectId()));
|
||||||
|
assertFalse(client.getObjectDatabase().has(barChild.toObjectId()));
|
||||||
|
|
||||||
|
assertEquals(1, uploadPack.get().getStatistics().getHaves());
|
||||||
|
assertTrue(countHavesHook.havesSentDuringNegotiation
|
||||||
|
.contains(localFooParent.toObjectId()));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Remote has 2 branches branchFoo and branchBar, each of them have 128 (2 x
|
||||||
|
* MAX_HAVES) commits each. Local has both the branches with lagging
|
||||||
|
* commits. Only 64 (1 x MAX_HAVES) from each branch and lagging 64.
|
||||||
|
* fetch(branchFoo) should send all 64 (MAX_HAVES) commits on branchFoo as
|
||||||
|
* haves and none from branchBar.
|
||||||
|
*
|
||||||
|
* Visual representation of the same:
|
||||||
|
*
|
||||||
|
* <pre>
|
||||||
|
* remote:
|
||||||
|
* parent
|
||||||
|
* / \
|
||||||
|
* branchFoo-0 branchBar-0
|
||||||
|
* branchFoo-1 branchBar-1
|
||||||
|
* ... ...
|
||||||
|
* ... ...
|
||||||
|
* branchFoo-128 branchBar-128
|
||||||
|
* ^-- branchFoo ^--branchBar
|
||||||
|
*
|
||||||
|
* local:
|
||||||
|
* parent
|
||||||
|
* / \
|
||||||
|
* branchFoo-0 branchBar-0
|
||||||
|
* branchFoo-1 branchBar-1
|
||||||
|
* ... ...
|
||||||
|
* ... ...
|
||||||
|
* branchFoo-64 branchBar-64
|
||||||
|
* ^-- branchFoo ^--branchBar
|
||||||
|
*
|
||||||
|
* fetch(branchFoo) should send all 64 (MAX_HAVES) commits on branchFoo as haves
|
||||||
|
* </pre>
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
public void testNegotiationTipWithLongerHistoryThanMaxHaves()
|
||||||
|
throws Exception {
|
||||||
|
Set<RevCommit> remoteFooCommits = new HashSet<>();
|
||||||
|
Set<RevCommit> remoteBarCommits = new HashSet<>();
|
||||||
|
|
||||||
|
RevCommit parent = remote.commit().message("branchFoo-0").create();
|
||||||
|
RevCommit parentFoo = parent;
|
||||||
|
remoteFooCommits.add(parentFoo);
|
||||||
|
for (int i = 1; i < 2 * MAX_HAVES; i++) {
|
||||||
|
RevCommit child = remote.commit()
|
||||||
|
.message("branchFoo-" + Integer.toString(i))
|
||||||
|
.parent(parentFoo)
|
||||||
|
.create();
|
||||||
|
parentFoo = child;
|
||||||
|
remoteFooCommits.add(parentFoo);
|
||||||
|
|
||||||
|
}
|
||||||
|
remote.update("branchFoo", parentFoo);
|
||||||
|
|
||||||
|
RevCommit parentBar = parent;
|
||||||
|
remoteBarCommits.add(parentBar);
|
||||||
|
for (int i = 1; i < 2 * MAX_HAVES; i++) {
|
||||||
|
RevCommit child = remote.commit()
|
||||||
|
.message("branchBar-" + Integer.toString(i))
|
||||||
|
.parent(parentBar)
|
||||||
|
.create();
|
||||||
|
parentBar = child;
|
||||||
|
remoteBarCommits.add(parentBar);
|
||||||
|
}
|
||||||
|
remote.update("branchBar", parentBar);
|
||||||
|
|
||||||
|
AtomicReference<UploadPack> uploadPack = new AtomicReference<>();
|
||||||
|
CountHavesPreUploadHook countHavesHook = new CountHavesPreUploadHook();
|
||||||
|
Set<ObjectId> localFooCommits = new HashSet<>();
|
||||||
|
|
||||||
|
try (TestRepository<InMemoryRepository> clientRepo = new TestRepository<>(
|
||||||
|
client)) {
|
||||||
|
RevCommit localParent = clientRepo.commit().message("branchBar-0")
|
||||||
|
.create();
|
||||||
|
RevCommit localParentFoo = localParent;
|
||||||
|
localFooCommits.add(localParentFoo);
|
||||||
|
for (int i = 1; i < 1 * MAX_HAVES; i++) {
|
||||||
|
RevCommit child = clientRepo.commit()
|
||||||
|
.message("branchFoo-" + Integer.toString(i))
|
||||||
|
.parent(localParentFoo).create();
|
||||||
|
localParentFoo = child;
|
||||||
|
localFooCommits.add(localParentFoo);
|
||||||
|
}
|
||||||
|
clientRepo.update("branchFoo", localParentFoo);
|
||||||
|
|
||||||
|
RevCommit localParentBar = localParent;
|
||||||
|
for (int i = 1; i < 1 * MAX_HAVES; i++) {
|
||||||
|
RevCommit child = clientRepo.commit()
|
||||||
|
.message("branchBar-" + Integer.toString(i))
|
||||||
|
.parent(localParentBar).create();
|
||||||
|
localParentBar = child;
|
||||||
|
}
|
||||||
|
clientRepo.update("branchBar", localParentBar);
|
||||||
|
|
||||||
|
testProtocol = new TestProtocol<>((Object req, Repository db) -> {
|
||||||
|
UploadPack up = new UploadPack(db);
|
||||||
|
up.setPreUploadHook(countHavesHook);
|
||||||
|
uploadPack.set(up);
|
||||||
|
return up;
|
||||||
|
}, null);
|
||||||
|
|
||||||
|
uri = testProtocol.register(ctx, server);
|
||||||
|
TestProtocol.setFetchConfig(new FetchConfig(true, MAX_HAVES, true));
|
||||||
|
try (Transport tn = testProtocol.open(uri,
|
||||||
|
clientRepo.getRepository(), "server")) {
|
||||||
|
|
||||||
|
tn.fetch(NullProgressMonitor.INSTANCE, Collections
|
||||||
|
.singletonList(new RefSpec("refs/heads/branchFoo")),
|
||||||
|
"branchFoo");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for (RevCommit c : remoteFooCommits) {
|
||||||
|
assertTrue(c.toObjectId() + "",
|
||||||
|
client.getObjectDatabase().has(c.toObjectId()));
|
||||||
|
}
|
||||||
|
remoteBarCommits.remove(parent);
|
||||||
|
for (RevCommit c : remoteBarCommits) {
|
||||||
|
assertFalse(client.getObjectDatabase().has(c.toObjectId()));
|
||||||
|
}
|
||||||
|
|
||||||
|
assertEquals(MAX_HAVES, uploadPack.get().getStatistics().getHaves());
|
||||||
|
// Verify that all the haves that were sent during negotiation are local
|
||||||
|
// commits from branchFoo
|
||||||
|
for (Object id : countHavesHook.havesSentDuringNegotiation) {
|
||||||
|
assertTrue(localFooCommits.contains(id));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static class CountHavesPreUploadHook implements PreUploadHook {
|
||||||
|
Set<ObjectId> havesSentDuringNegotiation = new HashSet<>();
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void onSendPack(UploadPack unusedUploadPack,
|
||||||
|
Collection<? extends ObjectId> unusedWants,
|
||||||
|
Collection<? extends ObjectId> haves)
|
||||||
|
throws ServiceMayNotContinueException {
|
||||||
|
// record haves
|
||||||
|
havesSentDuringNegotiation.addAll(haves);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void onEndNegotiateRound(UploadPack unusedUploadPack,
|
||||||
|
Collection<? extends ObjectId> unusedWants, int unusedCntCommon,
|
||||||
|
int unusedCntNotFound, boolean unusedReady)
|
||||||
|
throws ServiceMayNotContinueException {
|
||||||
|
// Do nothing.
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void onBeginNegotiateRound(UploadPack unusedUploadPack,
|
||||||
|
Collection<? extends ObjectId> unusedWants,
|
||||||
|
int unusedCntOffered) throws ServiceMayNotContinueException {
|
||||||
|
// Do nothing.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testV2FetchBadWantRef() throws Exception {
|
public void testV2FetchBadWantRef() throws Exception {
|
||||||
RevCommit one = remote.commit().message("1").create();
|
RevCommit one = remote.commit().message("1").create();
|
||||||
|
|
|
@ -37,6 +37,7 @@
|
||||||
import java.util.LinkedHashSet;
|
import java.util.LinkedHashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
import org.eclipse.jgit.errors.PackProtocolException;
|
import org.eclipse.jgit.errors.PackProtocolException;
|
||||||
import org.eclipse.jgit.errors.RemoteRepositoryException;
|
import org.eclipse.jgit.errors.RemoteRepositoryException;
|
||||||
|
@ -215,6 +216,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
|
||||||
|
|
||||||
private boolean allowOfsDelta;
|
private boolean allowOfsDelta;
|
||||||
|
|
||||||
|
private boolean useNegotiationTip;
|
||||||
|
|
||||||
private boolean noDone;
|
private boolean noDone;
|
||||||
|
|
||||||
private boolean noProgress;
|
private boolean noProgress;
|
||||||
|
@ -259,9 +262,11 @@ public BasePackFetchConnection(PackTransport packTransport) {
|
||||||
final FetchConfig cfg = getFetchConfig();
|
final FetchConfig cfg = getFetchConfig();
|
||||||
allowOfsDelta = cfg.allowOfsDelta;
|
allowOfsDelta = cfg.allowOfsDelta;
|
||||||
maxHaves = cfg.maxHaves;
|
maxHaves = cfg.maxHaves;
|
||||||
|
useNegotiationTip = cfg.useNegotiationTip;
|
||||||
} else {
|
} else {
|
||||||
allowOfsDelta = true;
|
allowOfsDelta = true;
|
||||||
maxHaves = Integer.MAX_VALUE;
|
maxHaves = Integer.MAX_VALUE;
|
||||||
|
useNegotiationTip = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
includeTags = transport.getTagOpt() != TagOpt.NO_TAGS;
|
includeTags = transport.getTagOpt() != TagOpt.NO_TAGS;
|
||||||
|
@ -297,14 +302,35 @@ static class FetchConfig {
|
||||||
|
|
||||||
final int maxHaves;
|
final int maxHaves;
|
||||||
|
|
||||||
|
final boolean useNegotiationTip;
|
||||||
|
|
||||||
FetchConfig(Config c) {
|
FetchConfig(Config c) {
|
||||||
allowOfsDelta = c.getBoolean("repack", "usedeltabaseoffset", true); //$NON-NLS-1$ //$NON-NLS-2$
|
allowOfsDelta = c.getBoolean("repack", "usedeltabaseoffset", true); //$NON-NLS-1$ //$NON-NLS-2$
|
||||||
maxHaves = c.getInt("fetch", "maxhaves", Integer.MAX_VALUE); //$NON-NLS-1$ //$NON-NLS-2$
|
maxHaves = c.getInt("fetch", "maxhaves", Integer.MAX_VALUE); //$NON-NLS-1$ //$NON-NLS-2$
|
||||||
|
useNegotiationTip = c.getBoolean("fetch", "usenegotiationtip", //$NON-NLS-1$ //$NON-NLS-2$
|
||||||
|
false);
|
||||||
}
|
}
|
||||||
|
|
||||||
FetchConfig(boolean allowOfsDelta, int maxHaves) {
|
FetchConfig(boolean allowOfsDelta, int maxHaves) {
|
||||||
|
this(allowOfsDelta, maxHaves, false);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @param allowOfsDelta
|
||||||
|
* when true optimizes the pack size by deltafying base
|
||||||
|
* object
|
||||||
|
* @param maxHaves
|
||||||
|
* max haves to be sent per negotiation
|
||||||
|
* @param useNegotiationTip
|
||||||
|
* if true uses the wanted refs instead of all refs as source
|
||||||
|
* of the "have" list to send.
|
||||||
|
* @since 6.6
|
||||||
|
*/
|
||||||
|
FetchConfig(boolean allowOfsDelta, int maxHaves,
|
||||||
|
boolean useNegotiationTip) {
|
||||||
this.allowOfsDelta = allowOfsDelta;
|
this.allowOfsDelta = allowOfsDelta;
|
||||||
this.maxHaves = maxHaves;
|
this.maxHaves = maxHaves;
|
||||||
|
this.useNegotiationTip = useNegotiationTip;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -384,7 +410,7 @@ protected void doFetch(final ProgressMonitor monitor,
|
||||||
noProgress = monitor == NullProgressMonitor.INSTANCE;
|
noProgress = monitor == NullProgressMonitor.INSTANCE;
|
||||||
|
|
||||||
markRefsAdvertised();
|
markRefsAdvertised();
|
||||||
markReachable(have, maxTimeWanted(want));
|
markReachable(want, have, maxTimeWanted(want));
|
||||||
|
|
||||||
if (TransferConfig.ProtocolVersion.V2
|
if (TransferConfig.ProtocolVersion.V2
|
||||||
.equals(getProtocolVersion())) {
|
.equals(getProtocolVersion())) {
|
||||||
|
@ -662,9 +688,17 @@ private int maxTimeWanted(Collection<Ref> wants) {
|
||||||
return maxTime;
|
return maxTime;
|
||||||
}
|
}
|
||||||
|
|
||||||
private void markReachable(Set<ObjectId> have, int maxTime)
|
private void markReachable(Collection<Ref> want, Set<ObjectId> have,
|
||||||
|
int maxTime)
|
||||||
throws IOException {
|
throws IOException {
|
||||||
|
Set<String> wantRefs = want.stream().map(Ref::getName)
|
||||||
|
.collect(Collectors.toSet());
|
||||||
|
|
||||||
for (Ref r : local.getRefDatabase().getRefs()) {
|
for (Ref r : local.getRefDatabase().getRefs()) {
|
||||||
|
if (useNegotiationTip && !wantRefs.contains(r.getName())) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
ObjectId id = r.getPeeledObjectId();
|
ObjectId id = r.getPeeledObjectId();
|
||||||
if (id == null)
|
if (id == null)
|
||||||
id = r.getObjectId();
|
id = r.getObjectId();
|
||||||
|
|
Loading…
Reference in New Issue