upload-pack: reject shallow requests that would return nothing

Shallow clones with --shallow-since or --shalow-exclude work by
running rev-list to get all reachable commits, then draw a boundary
between reachable and unreachable and send "shallow" requests based on
that.

The code does miss one corner case: if rev-list returns nothing, we'll
have no border and we'll send no shallow requests back to the client
(i.e. no history cuts). This essentially means a full clone (or a full
branch if the client requests just one branch). One example is the
oldest commit is older than what is specified by --shallow-since.

To avoid this, if rev-list returns nothing, we abort the clone/fetch.
The user could adjust their request (e.g. --shallow-since further back
in the past) and retry.

Another possible option for this case is to fall back to a default
depth (like depth 1). But I don't like too much magic that way because
we may return something unexpected to the user. If they request
"history since 2008" and we return a single depth at 2000, that might
break stuff for them. It is better to tell them that something is
wrong and let them take the best course of action.

Note that we need to die() in get_shallow_commits_by_rev_list()
instead of just checking for empty result from its caller
deepen_by_rev_list() and handling the error there. The reason is,
empty result could be a valid case: if you have commits in year 2013
and you request --shallow-since=year.2000 then you should get a full
clone (i.e. empty result).

Reported-by: Andreas Krey <a.krey@gmx.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/shallow.c b/shallow.c
index df4d44ea7a34a6d6bef119d1494f1c055614cbb1..44fdca1ace6938cd944f12b7e106ad3747f5a868 100644 (file)
--- a/shallow.c
+++ b/shallow.c
@@ -175,6 +175,9 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av,
                die("revision walk setup failed");
        traverse_commit_list(&revs, show_commit, NULL, &not_shallow_list);
 
                die("revision walk setup failed");
        traverse_commit_list(&revs, show_commit, NULL, &not_shallow_list);
 

+       if (!not_shallow_list)
+               die("no commits selected for shallow requests");
+

        /* Mark all reachable commits as NOT_SHALLOW */
        for (p = not_shallow_list; p; p = p->next)
                p->item->object.flags |= not_shallow_flag;
        /* Mark all reachable commits as NOT_SHALLOW */
        for (p = not_shallow_list; p; p = p->next)
                p->item->object.flags |= not_shallow_flag;

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 80a1a3239a64a4f551bc394ab88be500c9b5c44f..2a6055d47818a697e75fec7eb465977a2f37e5bc 100755 (executable)
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -711,6 +711,17 @@ test_expect_success 'fetch shallow since ...' '
        test_cmp expected actual
 '
 
        test_cmp expected actual
 '
 

+test_expect_success 'clone shallow since selects no commits' '
+       test_create_repo shallow-since-the-future &&
+       (
+       cd shallow-since-the-future &&
+       GIT_COMMITTER_DATE="100000000 +0700" git commit --allow-empty -m one &&
+       GIT_COMMITTER_DATE="200000000 +0700" git commit --allow-empty -m two &&
+       GIT_COMMITTER_DATE="300000000 +0700" git commit --allow-empty -m three &&
+       test_must_fail git clone --shallow-since "900000000 +0700" "file://$(pwd)/." ../shallow111
+       )
+'
+

 test_expect_success 'shallow clone exclude tag two' '
        test_create_repo shallow-exclude &&
        (
 test_expect_success 'shallow clone exclude tag two' '
        test_create_repo shallow-exclude &&
        (