Re: [PATCH 2/6] fetch-pack: add partial clone refiltering
From: Jonathan Tan <hidden>
Date: 2022-02-04 18:02:37
"Robert Coup via GitGitGadget" [off-list ref] writes:
quoted hunk ↗ jump to hunk
@@ -312,19 +312,21 @@ static int find_common(struct fetch_negotiator *negotiator, const char *remote_hex; struct object *o; - /* - * If that object is complete (i.e. it is an ancestor of a - * local ref), we tell them we have it but do not have to - * tell them about its ancestors, which they already know - * about. - * - * We use lookup_object here because we are only - * interested in the case we *know* the object is - * reachable and we have already scanned it. - */ - if (((o = lookup_object(the_repository, remote)) != NULL) && - (o->flags & COMPLETE)) { - continue; + if (!args->refilter) { + /* + * If that object is complete (i.e. it is an ancestor of a + * local ref), we tell them we have it but do not have to + * tell them about its ancestors, which they already know + * about. + * + * We use lookup_object here because we are only + * interested in the case we *know* the object is + * reachable and we have already scanned it. + */ + if (((o = lookup_object(the_repository, remote)) != NULL) && + (o->flags & COMPLETE)) { + continue; + }
The approach that I would have expected is to not call mark_complete_and_common_ref(), filter_refs(), everything_local(), and find_common(), but your approach here is to ensure that mark_complete_and_common_ref() and find_common() do not do anything. Comparing the two approaches, the advantage of yours is that we only need to make the change once to support both protocol v0 and v2 (although the change looks more substantial than just skipping function calls), but it makes the code more difficult to read in that we now have function calls that do nothing. What do you think about my approach?