From: Junio C Hamano Date: Sat, 10 Jan 2026 04:17:32 GMT Subject: Re: [PATCH v2 2/2] shallow: handling fetch relative-deepen Message-ID: In-Reply-To: "Samo Pogačnik via GitGitGadget" writes: > 2. Deepen shallow clone with fetch --deepen=1 (NOT OK) > Shallows: > 0cb5d204f4ef96ed241feb0f2088c9f4794ba758 > 61ba98be443fd51c542eb66585a1f6d7e15fcdae > Graph: > * 033585d (HEAD -> main) Merge branch 'branch' > |\ > | * 984f8b1 five > | * ecb578a four > |/ > * 0cb5d20 (grafted) three > --- This three-dash line will act as a marker to tell "git am" that your log message ends here. To avoid such an accident, make it a habit to indent any and all displayed material used as examples, e.g., 2. Deepen shallow clone with fetch --deepen=1 (NOT OK) Shallows: 0cb5d204f4ef96ed241feb0f2088c9f4794ba758 61ba98be443fd51c542eb66585a1f6d7e15fcdae Graph: * 033585d (HEAD -> main) Merge branch 'branch' |\ | * 984f8b1 five | * ecb578a four |/ * 0cb5d20 (grafted) three --- > Note that second shallow commit 61ba98be443fd51c542eb66585a1f6d7e15fcdae > is not reachable. > > On the other hand, it seems that equivalent absolute depth driven > fetches result in all the correct shallows. That led to this proposal, > which unifies absolute and relative deepening in a way that the same > get_shallow_commits() call is used in both cases. The difference is > only that depth is adapted for relative deepening by measuring > equivalent depth of current local shallow commits in the current remote > repo. Thus a new function get_shallows_depth() has been added and the > function get_reachable_list() became redundant / removed. > > Same example showing the corrected second step: > 2. Deepen shallow clone with fetch --deepen=1 (all good) > Shallow: > 61ba98be443fd51c542eb66585a1f6d7e15fcdae > Graph: > * 033585d (HEAD -> main) Merge branch 'branch' > |\ > | * 984f8b1 five > | * ecb578a four > |/ > * 0cb5d20 three > * 2b4e70d two > * 61ba98b (grafted) one > > The get_shallows_depth() function also shares the logic of the > get_shallow_commits() function, but it focuses on counting depth of > each existing shallow commit. The minimum result is stored as > 'data->deepen_relative', which is set not to be zero for relative > deepening anyway. That way we can allways summ 'data->deepen_relative' > and 'depth' values, because 'data->deepen_relative' is always 0 in > absolute deepening. > > Signed-off-by: Samo Pogačnik > --- > shallow.c | 44 +++++++++++++++++-------- > shallow.h | 1 + > t/t5500-fetch-pack.sh | 23 +++++++++++++ > upload-pack.c | 76 +++++-------------------------------------- > 4 files changed, 63 insertions(+), 81 deletions(-) > > diff --git a/shallow.c b/shallow.c > index 497a25836b..1a32808865 100644 > --- a/shallow.c > +++ b/shallow.c > @@ -130,11 +130,12 @@ static void free_depth_in_slab(int **ptr) > { > FREE_AND_NULL(*ptr); > } > -struct commit_list *get_shallow_commits(struct object_array *heads, int depth, > - int shallow_flag, int not_shallow_flag) > +struct commit_list *get_shallow_commits(struct object_array *heads, > + struct object_array *shallows, int *deepen_relative, > + int depth, int shallow_flag, int not_shallow_flag) > { > - size_t i = 0; > - int cur_depth = 0; > + size_t i = 0, j; > + int cur_depth = 0, cur_depth_shallow = 0; > struct commit_list *result = NULL; > struct object_array stack = OBJECT_ARRAY_INIT; > struct commit *commit = NULL; > @@ -168,16 +169,30 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth, > } > parse_commit_or_die(commit); > cur_depth++; > - if ((depth != INFINITE_DEPTH && cur_depth >= depth) || > - (is_repository_shallow(the_repository) && !commit->parents && > - (graft = lookup_commit_graft(the_repository, &commit->object.oid)) != NULL && > - graft->nr_parent < 0)) { > - commit_list_insert(commit, &result); > - commit->object.flags |= shallow_flag; > - commit = NULL; > - continue; > + if (shallows) { > + for (j = 0; j < shallows->nr; j++) > + if (oideq(&commit->object.oid, &shallows->objects[j].item->oid)) > + if ((!cur_depth_shallow) || (cur_depth < cur_depth_shallow)) > + cur_depth_shallow = cur_depth; > + > + if ((is_repository_shallow(the_repository) && !commit->parents && > + (graft = lookup_commit_graft(the_repository, &commit->object.oid)) != NULL && > + graft->nr_parent < 0)) { > + commit = NULL; > + continue; > + } > + } else { > + if ((depth != INFINITE_DEPTH && cur_depth >= depth) || > + (is_repository_shallow(the_repository) && !commit->parents && > + (graft = lookup_commit_graft(the_repository, &commit->object.oid)) != NULL && > + graft->nr_parent < 0)) { > + commit_list_insert(commit, &result); > + commit->object.flags |= shallow_flag; > + commit = NULL; > + continue; > + } > + commit->object.flags |= not_shallow_flag; > } > - commit->object.flags |= not_shallow_flag; > for (p = commit->parents, commit = NULL; p; p = p->next) { > int **depth_slot = commit_depth_at(&depths, p->item); > if (!*depth_slot) { > @@ -199,7 +214,8 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth, > } > deep_clear_commit_depth(&depths, free_depth_in_slab); > object_array_clear(&stack); > - > + if (shallows && deepen_relative) > + *deepen_relative = cur_depth_shallow; > return result; > } > > diff --git a/shallow.h b/shallow.h > index ad591bd139..d1b3878635 100644 > --- a/shallow.h > +++ b/shallow.h > @@ -36,6 +36,7 @@ int commit_shallow_file(struct repository *r, struct shallow_lock *lk); > void rollback_shallow_file(struct repository *r, struct shallow_lock *lk); > > struct commit_list *get_shallow_commits(struct object_array *heads, > + struct object_array *shallows, int *deepen_relative, > int depth, int shallow_flag, int not_shallow_flag); > struct commit_list *get_shallow_commits_by_rev_list(struct strvec *argv, > int shallow_flag, int not_shallow_flag); > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > index 2677cd5faa..5a8b30e1fd 100755 > --- a/t/t5500-fetch-pack.sh > +++ b/t/t5500-fetch-pack.sh > @@ -955,6 +955,29 @@ test_expect_success 'fetching deepen' ' > ) > ' > > +test_expect_success 'fetching deepen beyond merged branch' ' > + test_create_repo shallow-deepen-merged && > + ( > + cd shallow-deepen-merged && > + git commit --allow-empty -m one && > + git commit --allow-empty -m two && > + git commit --allow-empty -m three && > + git switch -c branch && > + git commit --allow-empty -m four && > + git commit --allow-empty -m five && > + git switch main && > + git merge --no-ff branch && > + cd - && > + git clone --bare --depth 3 "file://$(pwd)/shallow-deepen-merged" deepen.git && > + git -C deepen.git fetch origin --deepen=1 && > + git -C deepen.git rev-list --all >actual && > + for commit in $(sed "/^$/d" deepen.git/shallow) > + do > + test_grep "$commit" actual || exit 1 > + done > + ) > +' > + > test_negotiation_algorithm_default () { > test_when_finished rm -rf clientv0 clientv2 && > rm -rf server client && > diff --git a/upload-pack.c b/upload-pack.c > index 2d2b70cbf2..4232eef34f 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -704,54 +704,11 @@ error: > return -1; > } > > -static int get_reachable_list(struct upload_pack_data *data, > - struct object_array *reachable) > +static void get_shallows_depth(struct upload_pack_data *data) > { > - struct child_process cmd = CHILD_PROCESS_INIT; > - int i; > - struct object *o; > - char namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */ > - const unsigned hexsz = the_hash_algo->hexsz; > - int ret; > - > - if (do_reachable_revlist(&cmd, &data->shallows, reachable, > - data->allow_uor) < 0) { > - ret = -1; > - goto out; > - } > - > - while ((i = read_in_full(cmd.out, namebuf, hexsz + 1)) == hexsz + 1) { > - struct object_id oid; > - const char *p; > - > - if (parse_oid_hex(namebuf, &oid, &p) || *p != '\n') > - break; > - > - o = lookup_object(the_repository, &oid); > - if (o && o->type == OBJ_COMMIT) { > - o->flags &= ~TMP_MARK; > - } > - } > - for (i = get_max_object_index(the_repository); 0 < i; i--) { > - o = get_indexed_object(the_repository, i - 1); > - if (o && o->type == OBJ_COMMIT && > - (o->flags & TMP_MARK)) { > - add_object_array(o, NULL, reachable); > - o->flags &= ~TMP_MARK; > - } > - } > - close(cmd.out); > - > - if (finish_command(&cmd)) { > - ret = -1; > - goto out; > - } > - > - ret = 0; > - > -out: > - child_process_clear(&cmd); > - return ret; > + get_shallow_commits(&data->want_obj, &data->shallows, > + &data->deepen_relative, 0, > + SHALLOW, NOT_SHALLOW); > } > > static int has_unreachable(struct object_array *src, enum allow_uor allow_uor) > @@ -881,29 +838,14 @@ static void deepen(struct upload_pack_data *data, int depth) > struct object *object = data->shallows.objects[i].item; > object->flags |= NOT_SHALLOW; > } > - } else if (data->deepen_relative) { > - struct object_array reachable_shallows = OBJECT_ARRAY_INIT; > - struct commit_list *result; > - > - /* > - * Checking for reachable shallows requires that our refs be > - * marked with OUR_REF. > - */ > - refs_head_ref_namespaced(get_main_ref_store(the_repository), > - check_ref, data); > - for_each_namespaced_ref_1(check_ref, data); > - > - get_reachable_list(data, &reachable_shallows); > - result = get_shallow_commits(&reachable_shallows, > - depth + 1, > - SHALLOW, NOT_SHALLOW); > - send_shallow(data, result); > - free_commit_list(result); > - object_array_clear(&reachable_shallows); > } else { > struct commit_list *result; > > - result = get_shallow_commits(&data->want_obj, depth, > + if (data->deepen_relative) > + get_shallows_depth(data); > + > + result = get_shallow_commits(&data->want_obj, NULL, NULL, > + data->deepen_relative + depth, > SHALLOW, NOT_SHALLOW); > send_shallow(data, result); > free_commit_list(result);