{"thread":{"id":"59227","subject":"Subject: [RFC PATCH] upload_pack.c: make deepen-not more tree-ish","startedAt":"2023-02-10T21:32:35Z","lastAt":"2023-02-12T14:12:47Z","messageCount":4,"participants":["Andrew Wansink","Son Luong Ngoc"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"471946","messageId":"CA+tAvojz0u7AbcNnY1qyy3VznKhYTiAO1dL+rfOD3O6mOtsa8A@mail.gmail.com","threadId":"59227","inReplyTo":null,"subject":"Subject: [RFC PATCH] upload_pack.c: make deepen-not more tree-ish","fromName":"Andrew Wansink","fromEmail":"wansink@uber.com","sentAt":"2023-02-10T21:31:54Z","receivedAt":"2023-02-10T21:32:35Z","isPatch":true,"sender":{"key":"wansink@uber.com","avatar":null},"body":"This unlocks `git clone --shallow-exclude=<commit-sha1>`\n\ngit-clone only accepts --shallow-excude arguments where\nthe argument is a branch or tag because upload_pack only\nsearches deepen-not arguments for branches and tags.\n\nMake process_deepen_not search for commit objects if no\nbranch or tag is found then add them to the deepen_not\nlist.\n\nSigned-off-by: Andrew Wansink <wansink@uber.com>\n---\n\nAt Uber we have a lot of patches in CI simultaneously,\nthe CI jobs will frequently clone the monorepo multiple\ntimes for each patch.  They do this to calculate diffs\nbetween a patch and its parent commit.\n\nOne optimisation in this flow is to clone only to a specific\ndepth, this may or may not work, depending on how old the\npatch is.  In this case we have to --unshallow or discard\nthe shallow clone and fully clone the repo.\n\nThis patch would allow us to clone to exactly the depth we\nneed to find a patch's parent commit.\n\n t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++\n upload-pack.c         | 35 +++++++++++++++++++++++++++++++----\n 2 files changed, 61 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex d18f2823d86..8d5045cc1b9 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -899,6 +899,36 @@ test_expect_success 'shallow clone exclude tag two' '\n  )\n '\n\n+test_expect_success 'shallow clone exclude commit' '\n+ test_create_repo shallow-exclude-commit &&\n+ (\n+ cd shallow-exclude-commit &&\n+ test_commit one &&\n+ test_commit two &&\n+ test_commit three &&\n+ commit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&\n+ git clone --shallow-exclude=${commit_two_sha1} \"file://$(pwd)/.\"\n../shallow3-by-commit &&\n+ git -C ../shallow3-by-commit log --pretty=tformat:%s HEAD >actual &&\n+ git log -n 1 --pretty=tformat:%s HEAD >expected &&\n+ test_cmp expected actual\n+ )\n+'\n+\n+test_expect_success 'shallow clone exclude commit^' '\n+ test_create_repo shallow-exclude-commit-carat &&\n+ (\n+ cd shallow-exclude-commit-carat &&\n+ test_commit one &&\n+ test_commit two &&\n+ test_commit three &&\n+ commit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&\n+ git clone --shallow-exclude=${commit_two_sha1}^ \"file://$(pwd)/.\"\n../shallow23-by-commit &&\n+ git -C ../shallow23-by-commit log --pretty=tformat:%s HEAD >actual &&\n+ git log -n 2 --pretty=tformat:%s HEAD >expected &&\n+ test_cmp expected actual\n+ )\n+'\n+\n test_expect_success 'fetch exclude tag one' '\n  git -C shallow12 fetch --shallow-exclude one origin &&\n  git -C shallow12 log --pretty=tformat:%s origin/main >actual &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 551f22ffa5d..0c8594f4744 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -985,10 +985,37 @@ static int process_deepen_not(const char *line,\nstruct string_list *deepen_not,\n  if (skip_prefix(line, \"deepen-not \", &arg)) {\n  char *ref = NULL;\n  struct object_id oid;\n- if (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)\n- die(\"git upload-pack: ambiguous deepen-not: %s\", line);\n- string_list_append(deepen_not, ref);\n- free(ref);\n+\n+ switch (expand_ref(the_repository, arg, strlen(arg), &oid, &ref)) {\n+ case 1:\n+ // tag or branch matching arg found\n+ string_list_append(deepen_not, ref);\n+ free(ref);\n+ break;\n+ case 0: {\n+ // no tags or branches matching arg\n+ struct object *obj = NULL;\n+ struct commit *commit = NULL;\n+\n+ if (get_oid(arg, &oid))\n+ die(\"git upload-pack: deepen-not: no ref or object %s\", arg);\n+\n+ obj = parse_object(the_repository, &oid);\n+ if (!obj)\n+ die(\"git upload-pack: deepen-not: object could not be parsed: %s\", arg);\n+\n+ commit = (struct commit *)peel_to_type(arg, 0, obj, OBJ_COMMIT);\n+ if (!commit)\n+ die(\"git upload-pack: deepen-not: object not a commit: %s\", arg);\n+\n+ string_list_append(deepen_not, oid_to_hex(&commit->object.oid));\n+ break;\n+ }\n+ default:\n+ // more than 1 tag or branch matches arg\n+ die(\"git upload-pack: ambiguous deepen-not: %s\", arg);\n+ }\n+\n  *deepen_rev_list = 1;\n  return 1;\n  }\n-- \n2.39.1\n"},{"id":"471986","messageId":"20230211222353.1984150-1-andy@halogix.com","threadId":"59227","inReplyTo":"CA+tAvojz0u7AbcNnY1qyy3VznKhYTiAO1dL+rfOD3O6mOtsa8A@mail.gmail.com","subject":"[RFC PATCH] upload_pack.c: make deepen-not more tree-ish","fromName":"Andrew Wansink","fromEmail":"andy@halogix.com","sentAt":"2023-02-11T22:23:53Z","receivedAt":"2023-02-11T22:28:03Z","isPatch":true,"sender":{"key":"andy@halogix.com","avatar":null},"body":"This unlocks `git clone --shallow-exclude=<commit-sha1>`\n\ngit-clone only accepts --shallow-excude arguments where\nthe argument is a branch or tag because upload_pack only\nsearches deepen-not arguments for branches and tags.\n\nMake process_deepen_not search for commit objects if no\nbranch or tag is found then add them to the deepen_not\nlist.\n\nSigned-off-by: Andrew Wansink <wansink@uber.com>\n---\n\nAt Uber we have a lot of patches in CI simultaneously,\nthe CI jobs will frequently clone the monorepo multiple\ntimes for each patch.  They do this to calculate diffs\nbetween a patch and its parent commit.\n\nOne optimisation in this flow is to clone only to a specific\ndepth, this may or may not work, depending on how old the \npatch is.  In this case we have to --unshallow or discard\nthe shallow clone and fully clone the repo.\n\nThis patch would allow us to clone to exactly the depth we\nneed to find a patch's parent commit.\n\n t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++\n upload-pack.c         | 35 +++++++++++++++++++++++++++++++----\n 2 files changed, 61 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex d18f2823d86..8d5045cc1b9 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -899,6 +899,36 @@ test_expect_success 'shallow clone exclude tag two' '\n \t)\n '\n \n+test_expect_success 'shallow clone exclude commit' '\n+\ttest_create_repo shallow-exclude-commit &&\n+\t(\n+\tcd shallow-exclude-commit &&\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\ttest_commit three &&\n+\tcommit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&\n+\tgit clone --shallow-exclude=${commit_two_sha1} \"file://$(pwd)/.\" ../shallow3-by-commit &&\n+\tgit -C ../shallow3-by-commit log --pretty=tformat:%s HEAD >actual &&\n+\tgit log -n 1 --pretty=tformat:%s HEAD >expected &&\n+\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'shallow clone exclude commit^' '\n+\ttest_create_repo shallow-exclude-commit-carat &&\n+\t(\n+\tcd shallow-exclude-commit-carat &&\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\ttest_commit three &&\n+\tcommit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&\n+\tgit clone --shallow-exclude=${commit_two_sha1}^ \"file://$(pwd)/.\" ../shallow23-by-commit &&\n+\tgit -C ../shallow23-by-commit log --pretty=tformat:%s HEAD >actual &&\n+\tgit log -n 2 --pretty=tformat:%s HEAD >expected &&\n+\ttest_cmp expected actual\n+\t)\n+'\n+\n test_expect_success 'fetch exclude tag one' '\n \tgit -C shallow12 fetch --shallow-exclude one origin &&\n \tgit -C shallow12 log --pretty=tformat:%s origin/main >actual &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 551f22ffa5d..0c8594f4744 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -985,10 +985,37 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,\n \tif (skip_prefix(line, \"deepen-not \", &arg)) {\n \t\tchar *ref = NULL;\n \t\tstruct object_id oid;\n-\t\tif (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)\n-\t\t\tdie(\"git upload-pack: ambiguous deepen-not: %s\", line);\n-\t\tstring_list_append(deepen_not, ref);\n-\t\tfree(ref);\n+\n+\t\tswitch (expand_ref(the_repository, arg, strlen(arg), &oid, &ref)) {\n+\t\tcase 1:\n+\t\t\t// tag or branch matching arg found\n+\t\t\tstring_list_append(deepen_not, ref);\n+\t\t\tfree(ref);\n+\t\t\tbreak;\n+\t\tcase 0: {\n+\t\t\t// no tags or branches matching arg\n+\t\t\tstruct object *obj = NULL;\n+\t\t\tstruct commit *commit = NULL;\n+\n+\t\t\tif (get_oid(arg, &oid))\n+\t\t\t\tdie(\"git upload-pack: deepen-not: no ref or object %s\", arg);\n+\n+\t\t\tobj = parse_object(the_repository, &oid);\n+\t\t\tif (!obj)\n+\t\t\t\tdie(\"git upload-pack: deepen-not: object could not be parsed: %s\", arg);\n+\n+\t\t\tcommit = (struct commit *)peel_to_type(arg, 0, obj, OBJ_COMMIT);\n+\t\t\tif (!commit)\n+\t\t\t\tdie(\"git upload-pack: deepen-not: object not a commit: %s\", arg);\n+\n+\t\t\tstring_list_append(deepen_not, oid_to_hex(&commit->object.oid));\n+\t\t\tbreak;\n+\t\t}\n+\t\tdefault:\n+\t\t\t// more than 1 tag or branch matches arg\n+\t\t\tdie(\"git upload-pack: ambiguous deepen-not: %s\", arg);\n+\t\t}\n+\n \t\t*deepen_rev_list = 1;\n \t\treturn 1;\n \t}\n-- \n2.39.1\n\n"},{"id":"471987","messageId":"CA+tAvoijGhyySwfQCAuf2=vK5dvvLHu-U+YikRef2v24ECDr9Q@mail.gmail.com","threadId":"59227","inReplyTo":"20230211222353.1984150-1-andy@halogix.com","subject":"Re: [RFC PATCH] upload_pack.c: make deepen-not more tree-ish","fromName":"Andrew Wansink","fromEmail":"wansink@uber.com","sentAt":"2023-02-11T22:40:26Z","receivedAt":"2023-02-11T22:41:09Z","isPatch":true,"sender":{"key":"wansink@uber.com","avatar":null},"body":"Sorry to spam the list with this patch twice, I failed to follow the\ninstructions correctly the first time and sent a diff with the\nwhitespace stipped.\n\n- Andrew\n\n\nOn Sat, Feb 11, 2023 at 2:23 PM Andrew Wansink <andy@halogix.com> wrote:\n>\n> This unlocks `git clone --shallow-exclude=<commit-sha1>`\n>\n> git-clone only accepts --shallow-excude arguments where\n> the argument is a branch or tag because upload_pack only\n> searches deepen-not arguments for branches and tags.\n>\n> Make process_deepen_not search for commit objects if no\n> branch or tag is found then add them to the deepen_not\n> list.\n>\n> Signed-off-by: Andrew Wansink <wansink@uber.com>\n> ---\n>\n> At Uber we have a lot of patches in CI simultaneously,\n> the CI jobs will frequently clone the monorepo multiple\n> times for each patch.  They do this to calculate diffs\n> between a patch and its parent commit.\n>\n> One optimisation in this flow is to clone only to a specific\n> depth, this may or may not work, depending on how old the\n> patch is.  In this case we have to --unshallow or discard\n> the shallow clone and fully clone the repo.\n>\n> This patch would allow us to clone to exactly the depth we\n> need to find a patch's parent commit.\n>\n>  t/t5500-fetch-pack.sh | 30 ++++++++++++++++++++++++++++++\n>  upload-pack.c         | 35 +++++++++++++++++++++++++++++++----\n>  2 files changed, 61 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\n> index d18f2823d86..8d5045cc1b9 100755\n> --- a/t/t5500-fetch-pack.sh\n> +++ b/t/t5500-fetch-pack.sh\n> @@ -899,6 +899,36 @@ test_expect_success 'shallow clone exclude tag two' '\n>         )\n>  '\n>\n> +test_expect_success 'shallow clone exclude commit' '\n> +       test_create_repo shallow-exclude-commit &&\n> +       (\n> +       cd shallow-exclude-commit &&\n> +       test_commit one &&\n> +       test_commit two &&\n> +       test_commit three &&\n> +       commit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&\n> +       git clone --shallow-exclude=${commit_two_sha1} \"file://$(pwd)/.\" ../shallow3-by-commit &&\n> +       git -C ../shallow3-by-commit log --pretty=tformat:%s HEAD >actual &&\n> +       git log -n 1 --pretty=tformat:%s HEAD >expected &&\n> +       test_cmp expected actual\n> +       )\n> +'\n> +\n> +test_expect_success 'shallow clone exclude commit^' '\n> +       test_create_repo shallow-exclude-commit-carat &&\n> +       (\n> +       cd shallow-exclude-commit-carat &&\n> +       test_commit one &&\n> +       test_commit two &&\n> +       test_commit three &&\n> +       commit_two_sha1=$(git log -n 1 --pretty=tformat:%h HEAD^) &&\n> +       git clone --shallow-exclude=${commit_two_sha1}^ \"file://$(pwd)/.\" ../shallow23-by-commit &&\n> +       git -C ../shallow23-by-commit log --pretty=tformat:%s HEAD >actual &&\n> +       git log -n 2 --pretty=tformat:%s HEAD >expected &&\n> +       test_cmp expected actual\n> +       )\n> +'\n> +\n>  test_expect_success 'fetch exclude tag one' '\n>         git -C shallow12 fetch --shallow-exclude one origin &&\n>         git -C shallow12 log --pretty=tformat:%s origin/main >actual &&\n> diff --git a/upload-pack.c b/upload-pack.c\n> index 551f22ffa5d..0c8594f4744 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -985,10 +985,37 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,\n>         if (skip_prefix(line, \"deepen-not \", &arg)) {\n>                 char *ref = NULL;\n>                 struct object_id oid;\n> -               if (expand_ref(the_repository, arg, strlen(arg), &oid, &ref) != 1)\n> -                       die(\"git upload-pack: ambiguous deepen-not: %s\", line);\n> -               string_list_append(deepen_not, ref);\n> -               free(ref);\n> +\n> +               switch (expand_ref(the_repository, arg, strlen(arg), &oid, &ref)) {\n> +               case 1:\n> +                       // tag or branch matching arg found\n> +                       string_list_append(deepen_not, ref);\n> +                       free(ref);\n> +                       break;\n> +               case 0: {\n> +                       // no tags or branches matching arg\n> +                       struct object *obj = NULL;\n> +                       struct commit *commit = NULL;\n> +\n> +                       if (get_oid(arg, &oid))\n> +                               die(\"git upload-pack: deepen-not: no ref or object %s\", arg);\n> +\n> +                       obj = parse_object(the_repository, &oid);\n> +                       if (!obj)\n> +                               die(\"git upload-pack: deepen-not: object could not be parsed: %s\", arg);\n> +\n> +                       commit = (struct commit *)peel_to_type(arg, 0, obj, OBJ_COMMIT);\n> +                       if (!commit)\n> +                               die(\"git upload-pack: deepen-not: object not a commit: %s\", arg);\n> +\n> +                       string_list_append(deepen_not, oid_to_hex(&commit->object.oid));\n> +                       break;\n> +               }\n> +               default:\n> +                       // more than 1 tag or branch matches arg\n> +                       die(\"git upload-pack: ambiguous deepen-not: %s\", arg);\n> +               }\n> +\n>                 *deepen_rev_list = 1;\n>                 return 1;\n>         }\n> --\n> 2.39.1\n>\n"},{"id":"471993","messageId":"CAL3xRKcw5KD2xE2Z1hEc-j8WMWknoMTJwfdEVa=h5sGb=SmhNQ@mail.gmail.com","threadId":"59227","inReplyTo":"CAL3xRKdCkAAR0r3jyKFy+TtUi65LQcHaste=2WCqYHtwi8cUhw@mail.gmail.com","subject":"Re: [RFC PATCH] upload_pack.c: make deepen-not more tree-ish","fromName":"Son Luong Ngoc","fromEmail":"sluongng@gmail.com","sentAt":"2023-02-12T14:12:29Z","receivedAt":"2023-02-12T14:12:47Z","isPatch":true,"sender":{"key":"sluongng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26684313?v=4"},"body":"Re-send to the Git mailing-list as setting a font on gmail switched\nplain-text to HTML and thus, got blocked by mailing-list.\n\nOn Sun, Feb 12, 2023 at 3:09 PM Son Luong Ngoc <sluongng@gmail.com> wrote:\n>\n> Hi Andrew,\n>\n> On Sat, Feb 11, 2023 at 11:49 PM Andrew Wansink <andy@halogix.com> wrote:\n> >\n> > This unlocks `git clone --shallow-exclude=<commit-sha1>`\n> >\n> > git-clone only accepts --shallow-excude arguments where\n> > the argument is a branch or tag because upload_pack only\n> > searches deepen-not arguments for branches and tags.\n> >\n> > Make process_deepen_not search for commit objects if no\n> > branch or tag is found then add them to the deepen_not\n> > list.\n> >\n> > Signed-off-by: Andrew Wansink <wansink@uber.com>\n> > ---\n> >\n> > At Uber we have a lot of patches in CI simultaneously,\n> > the CI jobs will frequently clone the monorepo multiple\n> > times for each patch.  They do this to calculate diffs\n> > between a patch and its parent commit.\n> >\n>\n> I used to manage a CI system that support monorepo use cases not so long ago.\n> We had several hosts(VM/Baremetal) on which we spin up containers for CI to run.\n>\n> We maintain a bare copy of the monorepo on the host level (cron job / systemd / DaemonSet) and mount this as read-only into each of the CI containers.\n>\n> Each of the CI containers would attempt to clone/fetch the monorepo with `--reference-if-able ./path/to/read-only-mount/repo.git` (1)\n> So that most of the needed objects are already on disk in the shared bare repo.\n>\n>\n> +-----------+  +-----------+  +-----------+\n> | container |  | container |  | container |\n> +-----------+  +-----------+  +-----------+\n>              \\       |       /\n>       (mount) \\      |      /\n>               +------------+                 +--------+\n>               | bare-repo  | <-------------- | Remote |\n>               +------------+   (git-fetch)   +--------+\n>                     |\n>                     | (maintain)\n>                     |\n>               +----------+\n>               | cron-job |\n>               +----------+\n>\n> (forgive my horrible drawing)\n>\n> With this setup, we did not have a need to shallow clone any longer,\n> and our git-clone in each container is simply a combination of git-ls-remote and a very light-weighted git-fetch.\n> In some cases, such as a job in the later stages of a CI pipeline,\n> the host would already download all the needed objects into the bare copy of the repository.\n> This lets us skip git-fetch entirely when the CI container executes.\n>\n> Compared to the shallow clone approach,\n> our \"local cache\" approach sped up the clone speed drastically\n> while allowing developers to interact with git history inside tests a lot easier.\n>\n> > One optimisation in this flow is to clone only to a specific\n> > depth, this may or may not work, depending on how old the\n> > patch is.  In this case we have to --unshallow or discard\n> > the shallow clone and fully clone the repo.\n> >\n> > This patch would allow us to clone to exactly the depth we\n> > need to find a patch's parent commit.\n>\n> Hope it helps,\n> Son Luong.\n>\n> (1): https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---reference-if-ableltrepositorygt\n"}]}