Throw error when deepen-since excludes all commits

In C Git, when a client fetches with "git fetch --shallow-since=<date>
origin <ref>", and all commits reachable from <ref> are older than
<date>, the server dies with a message "no commits selected for shallow
requests". That is, (1) the --shallow-since filter applies to the commit
pointed to by the ref itself, and (2) there is a check that at least one
commit is not filtered out. (The pack-protocol.txt documentation does
not describe this, but the C implementation does this.)

The implementation in commit 1bb430dc21 ("UploadPack: support
deepen-since in protocol v2", 2018-09-27) does neither (1) nor (2), so
do both of these.

Change-Id: I9946327a71627626ecce34ca2d017d2add8867fc
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
index b74dcca583a84e923863308dce24e4847d4cf9a3..ad3eb2e32297ef1591f92672e0a2fd6785de07d1 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
@@ -1279,6 +1279,26 @@ public class UploadPackTest {
                assertTrue(client.hasObject(merge.toObjectId()));
        }
 
+       @Test
+       public void testV2FetchShallowSince_noCommitsSelected() throws Exception {
+               PersonIdent person = new PersonIdent(remote.getRepository());
+
+               RevCommit tooOld = remote.commit()
+                       .committer(new PersonIdent(person, 1500000000, 0)).create();
+
+               remote.update("branch1", tooOld);
+
+               thrown.expect(PackProtocolException.class);
+               thrown.expectMessage("No commits selected for shallow request");
+               uploadPackV2(
+                       "command=fetch\n",
+                       PacketLineIn.DELIM,
+                       "deepen-since 1510000\n",
+                       "want " + tooOld.toObjectId().getName() + "\n",
+                       "done\n",
+                       PacketLineIn.END);
+       }
+
        @Test
        public void testV2FetchUnrecognizedArgument() throws Exception {
                thrown.expect(PackProtocolException.class);

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index b3f7b89c9bbec01600f5dbbd0fba0724257f2d65..311d68b3954a4e22ec7be1482e7337e21a3e344e 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -467,6 +467,7 @@ newIdMustNotBeNull=New ID must not be null
 newlineInQuotesNotAllowed=Newline in quotes not allowed
 noApplyInDelete=No apply in delete
 noClosingBracket=No closing {0} found for {1} at index {2}.
+noCommitsSelectedForShallow=No commits selected for shallow request
 noCredentialsProvider=Authentication is required but no CredentialsProvider has been registered
 noHEADExistsAndNoExplicitStartingRevisionWasSpecified=No HEAD exists and no explicit starting revision was specified
 noHMACsupport=No {0} support: {1}

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 9f0e3e0c2a45096c78857fc96a66a3dadff048c3..4479525dd837cde6f6c21f700a2d2a1c5dd0cdc1 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -528,6 +528,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String newlineInQuotesNotAllowed;
        /***/ public String noApplyInDelete;
        /***/ public String noClosingBracket;
+       /***/ public String noCommitsSelectedForShallow;
        /***/ public String noCredentialsProvider;
        /***/ public String noHEADExistsAndNoExplicitStartingRevisionWasSpecified;
        /***/ public String noHMACsupport;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java
index 6c6cc95c2e58a0015cb9b55cb995537c67be8ffc..ef257585893b4b763db0b4b6efd4c35d293c11e7 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java
@@ -135,6 +135,10 @@ class DepthGenerator extends Generator {
                        if ((c.flags & RevWalk.PARSED) == 0)
                                c.parseHeaders(walk);
 
+                       if (c.getCommitTime() < deepenSince) {
+                               continue;
+                       }
+
                        int newDepth = c.depth + 1;
 
                        for (RevCommit p : c.parents) {

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 b69f2cd92fc3a37a2a981d5bfcde1e844c7c6a7d..a2174f641115c2c33b76f5a843da6c2f4409441c 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -1168,8 +1168,10 @@ public class UploadPack {
                        }
 
                        RevCommit o;
+                       boolean atLeastOne = false;
                        while ((o = depthWalk.next()) != null) {
                                DepthWalk.Commit c = (DepthWalk.Commit) o;
+                               atLeastOne = true;
 
                                boolean isBoundary = (c.getDepth() == walkDepth) || c.isBoundary();
 
@@ -1185,6 +1187,10 @@ public class UploadPack {
                                        unshallowFunc.accept(c.copy());
                                }
                        }
+                       if (!atLeastOne) {
+                               throw new PackProtocolException(
+                                       JGitText.get().noCommitsSelectedForShallow);
+                       }
                }
        }