Re: [RFC PATCH] upload_pack.c: make deepen-not more tree-ish

[Andrew Wansink <wansink@uber.com>]

Sorry to spam the list with this patch twice, I failed to follow the
instructions correctly the first time and sent a diff with the
whitespace stipped.

- Andrew


On Sat, Feb 11, 2023 at 2:23 PM Andrew Wansink <andy@halogix.com> wrote:
>
> This unlocks `git clone --shallow-exclude=<commit-sha1>`
>
> git-clone only accepts --shallow-excude arguments where
> the argument is a branch or tag because upload_pack only
> searches deepen-not arguments for branches and tags.
>
> Make process_deepen_not search for commit objects if no
> branch or tag is found then add them to the deepen_not
> list.
>
> Signed-off-by: Andrew Wansink <wansink@uber.com>
> ---
>
> At Uber we have a lot of patches in CI simultaneously,
> the CI jobs will frequently clone the monorepo multiple
> times for each patch.  They do this to calculate diffs
> between a patch and its parent commit.
>
> One optimisation in this flow is to clone only to a specific
> depth, this may or may not work, depending on how old the
> patch is.  In this case we have to --unshallow or discard
> the shallow clone and fully clone the repo.
>
> This patch would allow us to clone to exactly the depth we
> need to find a patch's parent commit.
>
>  t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++
>  upload-pack.c         | 35 +++++++++++++++++++++++++++++++----
>  2 files changed, 61 insertions(+), 4 deletions(-)
>
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index d18f2823d86..8d5045cc1b9 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -899,6 +899,36 @@ test_expect_success 'shallow clone exclude tag two' '
>         )
>  '
>
> +test_expect_success 'shallow clone exclude commit' '
> +       test_create_repo shallow-exclude-commit &&
> +       (
> +       cd shallow-exclude-commit &&
> +       test_commit one &&
> +       test_commit two &&
> +       test_commit three &&
> +       commit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&
> +       git clone --shallow-exclude=${commit_two_sha1} "file://$(pwd)/." ../shallow3-by-commit &&
> +       git -C ../shallow3-by-commit log --pretty=tformat:%s HEAD >actual &&
> +       git log -n 1 --pretty=tformat:%s HEAD >expected &&
> +       test_cmp expected actual
> +       )
> +'
> +
> +test_expect_success 'shallow clone exclude commit^' '
> +       test_create_repo shallow-exclude-commit-carat &&
> +       (
> +       cd shallow-exclude-commit-carat &&
> +       test_commit one &&
> +       test_commit two &&
> +       test_commit three &&
> +       commit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&
> +       git clone --shallow-exclude=${commit_two_sha1}^ "file://$(pwd)/." ../shallow23-by-commit &&
> +       git -C ../shallow23-by-commit log --pretty=tformat:%s HEAD >actual &&
> +       git log -n 2 --pretty=tformat:%s HEAD >expected &&
> +       test_cmp expected actual
> +       )
> +'
> +
>  test_expect_success 'fetch exclude tag one' '
>         git -C shallow12 fetch --shallow-exclude one origin &&
>         git -C shallow12 log --pretty=tformat:%s origin/main >actual &&
> diff --git a/upload-pack.c b/upload-pack.c
> index 551f22ffa5d..0c8594f4744 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -985,10 +985,37 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,
>         if (skip_prefix(line, "deepen-not ", &arg)) {
>                 char *ref = NULL;
>                 struct object_id oid;
> -               if (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)
> -                       die("git upload-pack: ambiguous deepen-not: %s", line);
> -               string_list_append(deepen_not, ref);
> -               free(ref);
> +
> +               switch (expand_ref(the_repository, arg, strlen(arg), &oid, &ref)) {
> +               case 1:
> +                       // tag or branch matching arg found
> +                       string_list_append(deepen_not, ref);
> +                       free(ref);
> +                       break;
> +               case 0: {
> +                       // no tags or branches matching arg
> +                       struct object *obj = NULL;
> +                       struct commit *commit = NULL;
> +
> +                       if (get_oid(arg, &oid))
> +                               die("git upload-pack: deepen-not: no ref or object %s", arg);
> +
> +                       obj = parse_object(the_repository, &oid);
> +                       if (!obj)
> +                               die("git upload-pack: deepen-not: object could not be parsed: %s", arg);
> +
> +                       commit = (struct commit *)peel_to_type(arg, 0, obj, OBJ_COMMIT);
> +                       if (!commit)
> +                               die("git upload-pack: deepen-not: object not a commit: %s", arg);
> +
> +                       string_list_append(deepen_not, oid_to_hex(&commit->object.oid));
> +                       break;
> +               }
> +               default:
> +                       // more than 1 tag or branch matches arg
> +                       die("git upload-pack: ambiguous deepen-not: %s", arg);
> +               }
> +
>                 *deepen_rev_list = 1;
>                 return 1;
>         }
> --
> 2.39.1
>