Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes:
> 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.
Yeah, that makes sense.
> 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.
I agree that it would be a horrible fallback. I actually am
wondering if we should just silently return no objects without even
telling the user there is something unexpected happening. After
all, the user may well be expecting with --shallow-since that is too
recent that the fetch may not result in pulling anything new, and
giving a "die" message, which now needs to be distinguished from
other forms of die's like network connectivity or auth failures, is
not all that helpful.
> 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).
Yup, that latter example makes me more convinced that it also is a
valid outcome if we end up requesting "no" object when shallow-since
names too recent date, e.g. against a project that is dormant since
2013 with --shallow-since=2018/01/01 or something like that, instead
of dying.
> Reported-by: Andreas Krey <a.k...@gmx.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
> shallow.c | 3 +++
> t/t5500-fetch-pack.sh | 11 +++++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/shallow.c b/shallow.c
> index df4d44ea7a..44fdca1ace 100644
> --- 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, ¬_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;
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 0680dec808..c8f2d38a58 100755
> --- 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_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 &&
> (
