diff options
4 files changed, 101 insertions, 17 deletions
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java index 1ceb0965a3..2b9e81f1d3 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java @@ -104,6 +104,7 @@ class UploadPackServlet extends HttpServlet { ServiceNotEnabledException, ServiceNotAuthorizedException { UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER); try { + up.setBiDirectionalPipe(false); up.sendAdvertisedRefs(pck); } finally { up.getRevWalk().release(); diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java index c3590a44fb..cd127cdeaf 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java @@ -292,8 +292,6 @@ public class SmartClientSmartServerTest extends HttpTestCase { .getRequestHeader(HDR_CONTENT_LENGTH)); assertNull("not chunked", service .getRequestHeader(HDR_TRANSFER_ENCODING)); - assertNull("no compression (too small)", service - .getRequestHeader(HDR_CONTENT_ENCODING)); assertEquals(200, service.getStatus()); assertEquals("application/x-git-upload-pack-result", service @@ -301,7 +299,70 @@ public class SmartClientSmartServerTest extends HttpTestCase { } @Test - public void testFetchUpdateExisting() throws Exception { + public void testFetch_FewLocalCommits() throws Exception { + // Bootstrap by doing the clone. + // + TestRepository dst = createTestRepository(); + Transport t = Transport.open(dst.getRepository(), remoteURI); + try { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + } finally { + t.close(); + } + assertEquals(B, dst.getRepository().getRef(master).getObjectId()); + List<AccessEvent> cloneRequests = getRequests(); + + // Only create a few new commits. + TestRepository.BranchBuilder b = dst.branch(master); + for (int i = 0; i < 4; i++) + b.commit().tick(3600 /* 1 hour */).message("c" + i).create(); + + // Create a new commit on the remote. + // + b = new TestRepository(remoteRepository).branch(master); + RevCommit Z = b.commit().message("Z").create(); + + // Now incrementally update. + // + t = Transport.open(dst.getRepository(), remoteURI); + try { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + } finally { + t.close(); + } + assertEquals(Z, dst.getRepository().getRef(master).getObjectId()); + + List<AccessEvent> requests = getRequests(); + requests.removeAll(cloneRequests); + assertEquals(2, requests.size()); + + AccessEvent info = requests.get(0); + assertEquals("GET", info.getMethod()); + assertEquals(join(remoteURI, "info/refs"), info.getPath()); + assertEquals(1, info.getParameters().size()); + assertEquals("git-upload-pack", info.getParameter("service")); + assertEquals(200, info.getStatus()); + assertEquals("application/x-git-upload-pack-advertisement", + info.getResponseHeader(HDR_CONTENT_TYPE)); + + // We should have needed one request to perform the fetch. + // + AccessEvent service = requests.get(1); + assertEquals("POST", service.getMethod()); + assertEquals(join(remoteURI, "git-upload-pack"), service.getPath()); + assertEquals(0, service.getParameters().size()); + assertNotNull("has content-length", + service.getRequestHeader(HDR_CONTENT_LENGTH)); + assertNull("not chunked", + service.getRequestHeader(HDR_TRANSFER_ENCODING)); + + assertEquals(200, service.getStatus()); + assertEquals("application/x-git-upload-pack-result", + service.getResponseHeader(HDR_CONTENT_TYPE)); + } + + @Test + public void testFetch_TooManyLocalCommits() throws Exception { // Bootstrap by doing the clone. // TestRepository dst = createTestRepository(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java index 4ae26a3e56..67bedaca11 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java @@ -138,6 +138,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection static final String OPTION_NO_PROGRESS = "no-progress"; + static final String OPTION_NO_DONE = "no-done"; + static enum MultiAck { OFF, CONTINUE, DETAILED; } @@ -169,6 +171,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection private boolean allowOfsDelta; + private boolean noDone; + private String lockMessage; private PackLock packLock; @@ -408,9 +412,11 @@ public abstract class BasePackFetchConnection extends BasePackConnection if (allowOfsDelta) wantCapability(line, OPTION_OFS_DELTA); - if (wantCapability(line, OPTION_MULTI_ACK_DETAILED)) + if (wantCapability(line, OPTION_MULTI_ACK_DETAILED)) { multiAck = MultiAck.DETAILED; - else if (wantCapability(line, OPTION_MULTI_ACK)) + if (statelessRPC) + noDone = wantCapability(line, OPTION_NO_DONE); + } else if (wantCapability(line, OPTION_MULTI_ACK)) multiAck = MultiAck.CONTINUE; else multiAck = MultiAck.OFF; @@ -441,13 +447,13 @@ public abstract class BasePackFetchConnection extends BasePackConnection int havesSinceLastContinue = 0; boolean receivedContinue = false; boolean receivedAck = false; - boolean negotiate = true; + boolean receivedReady = false; if (statelessRPC) state.writeTo(out, null); negotiateBegin(); - SEND_HAVES: while (negotiate) { + SEND_HAVES: while (!receivedReady) { final RevCommit c = walk.next(); if (c == null) break SEND_HAVES; @@ -514,7 +520,7 @@ public abstract class BasePackFetchConnection extends BasePackConnection receivedContinue = true; havesSinceLastContinue = 0; if (anr == AckNackResult.ACK_READY) - negotiate = false; + receivedReady = true; break; } @@ -540,12 +546,14 @@ public abstract class BasePackFetchConnection extends BasePackConnection if (monitor.isCancelled()) throw new CancelledException(); - // When statelessRPC is true we should always leave SEND_HAVES - // loop above while in the middle of a request. This allows us - // to just write done immediately. - // - pckOut.writeString("done\n"); - pckOut.flush(); + if (!receivedReady || !noDone) { + // When statelessRPC is true we should always leave SEND_HAVES + // loop above while in the middle of a request. This allows us + // to just write done immediately. + // + pckOut.writeString("done\n"); + pckOut.flush(); + } if (!receivedAck) { // Apparently if we have never received an ACK earlier diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 2802c0712e..50f57130c3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -100,6 +100,8 @@ public class UploadPack { static final String OPTION_NO_PROGRESS = BasePackFetchConnection.OPTION_NO_PROGRESS; + static final String OPTION_NO_DONE = BasePackFetchConnection.OPTION_NO_DONE; + /** Database we read the objects from. */ private final Repository db; @@ -163,6 +165,8 @@ public class UploadPack { /** null if {@link #commonBase} should be examined again. */ private Boolean okToGiveUp; + private boolean sentReady; + /** Objects we sent in our advertisement list, clients can ask for these. */ private Set<ObjectId> advertised; @@ -182,6 +186,8 @@ public class UploadPack { private MultiAck multiAck = MultiAck.OFF; + private boolean noDone; + private PackWriter.Statistics statistics; private UploadPackLogger logger; @@ -401,9 +407,10 @@ public class UploadPack { return; } - if (options.contains(OPTION_MULTI_ACK_DETAILED)) + if (options.contains(OPTION_MULTI_ACK_DETAILED)) { multiAck = MultiAck.DETAILED; - else if (options.contains(OPTION_MULTI_ACK)) + noDone = options.contains(OPTION_NO_DONE); + } else if (options.contains(OPTION_MULTI_ACK)) multiAck = MultiAck.CONTINUE; else multiAck = MultiAck.OFF; @@ -443,6 +450,8 @@ public class UploadPack { adv.advertiseCapability(OPTION_SIDE_BAND_64K); adv.advertiseCapability(OPTION_THIN_PACK); adv.advertiseCapability(OPTION_NO_PROGRESS); + if (!biDirectionalPipe) + adv.advertiseCapability(OPTION_NO_DONE); adv.setDerefTags(true); advertised = adv.send(getAdvertisedRefs()); adv.end(); @@ -496,6 +505,10 @@ public class UploadPack { last = processHaveLines(peerHas, last); if (commonBase.isEmpty() || multiAck != MultiAck.OFF) pckOut.writeString("NAK\n"); + if (noDone && sentReady) { + pckOut.writeString("ACK " + last.name() + "\n"); + return true; + } if (!biDirectionalPipe) return false; pckOut.flush(); @@ -538,6 +551,7 @@ public class UploadPack { List<ObjectId> toParse = peerHas; HashSet<ObjectId> peerHasSet = null; boolean needMissing = false; + sentReady = false; if (wantAll.isEmpty() && !wantIds.isEmpty()) { // We have not yet parsed the want list. Parse it now. @@ -644,7 +658,6 @@ public class UploadPack { // telling us about its history. // boolean didOkToGiveUp = false; - boolean sentReady = false; if (0 < missCnt) { for (int i = peerHas.size() - 1; i >= 0; i--) { ObjectId id = peerHas.get(i); @@ -670,6 +683,7 @@ public class UploadPack { if (multiAck == MultiAck.DETAILED && !didOkToGiveUp && okToGiveUp()) { ObjectId id = peerHas.get(peerHas.size() - 1); + sentReady = true; pckOut.writeString("ACK " + id.name() + " ready\n"); sentReady = true; } |