On Fri, Oct 28, 2022 at 11:12:33AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > diff --git a/connected.c b/connected.c
> > index 74a20cb32e..2a4c4e0025 100644
> > --- a/connected.c
> > +++ b/connected.c
> > @@ -98,7 +98,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
> > strvec_push(&rev_list.args, "--stdin");
> > if (has_promisor_remote())
> > strvec_push(&rev_list.args, "--exclude-promisor-objects");
> > - if (!opt->is_deepening_fetch) {
> > + if (!opt->is_deepening_fetch && !opt->reachable_oids_fn) {
> > strvec_push(&rev_list.args, "--not");
> > strvec_push(&rev_list.args, "--all");
> > }
> > @@ -125,6 +125,13 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
> >
> > rev_list_in = xfdopen(rev_list.in, "w");
> >
> > + if (opt->reachable_oids_fn) {
> > + const struct object_id *reachable_oid;
> > + while ((reachable_oid = opt->reachable_oids_fn(opt->reachable_oids_data)) != NULL)
> > + if (fprintf(rev_list_in, "^%s\n", oid_to_hex(reachable_oid)) < 0)
> > + break;
> > + }
>
> It is good that these individual negative references are fed from
> the standard input, not on the command line, as they can be many.
>
> In the original code without the reachable_oids_fn, we refrain from
> excluding when the is_deepening_fetch bit is set, but here we do not
> pay attention to the bit at all. Is that sensible, and if so why?
Hm, good point. On a deepening fetch the commits that were the previous
boundary will likely get replaced by new commits that are at a deeper
point in history, so they cannot be used as a well-defined boundary.
Instead, we do a complete graph-walk that doesn't stop at any previously
known commits at all. At least that's how I understand the code, the
explanation is likely a bit fuzzy.
I guess we should thus also pay attention to `is_deepening_fetch` here.
As this means that `is_deepening_fetch` and `reachable_oids_fn` are
mutually exclusive I'm inclined to go even further and `die()` if both
are set at the same time. We only adapt git-receive-pack(1) anyway, so
we should never run into this situation for now.
Patrick
Attachment:
signature.asc
Description: PGP signature
