> @@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
> int autotags = (transport->remote->fetch_tags == 1);
> int retcode = 0;
> const struct ref *remote_refs;
> + struct ref *new_remote_refs = NULL;
Above, you use the name "updated_remote_refs" - it's probably better to
standardize on one. I think "updated" is better.
(The transport calling it "fetched_refs" is fine, because that's what
they are from the perspective of the transport. From the perspective of
fetch-pack, it is indeed a new or updated set of remote refs.)
> - if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map))
> {
> +
> + if (fetch_refs(transport, ref_map, &new_remote_refs)) {
> + free_refs(ref_map);
> + retcode = 1;
> + goto cleanup;
> + }
> + if (new_remote_refs) {
> + free_refs(ref_map);
> + ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
> + tags, &autotags);
> + free_refs(new_remote_refs);
> + }
> + if (consume_refs(transport, ref_map)) {
> free_refs(ref_map);
> retcode = 1;
> goto cleanup;
Here, if we got updated remote refs, we need to regenerate ref_map,
since it is the source of truth.
Maybe add a comment in the "if (new_remote_refs)" block explaining this
- something like: Regenerate ref_map using the updated remote refs,
because the transport would place shallow (and other) information
there.
> - for (i = 0; i < nr_sought; i++)
> + for (r = refs; r; r = r->next, i++)
> if (status[i])
> - sought[i]->status = REF_STATUS_REJECT_SHALLOW;
> + r->status = REF_STATUS_REJECT_SHALLOW;
You use i here without initializing it to 0. t5703 also fails with this
patch - probably related to this, but I didn't check.
If you initialize i here, I don't think you need to initialize it to 0
at the top of this function.
