Re: [PATCH v2 7/8] fetch-pack: put shallow info in output parameter
From: Jonathan Tan <hidden>
Date: 2018-06-14 23:59:57
quoted hunk ↗ jump to hunk
@@ -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.