fetch: control lifecycle of FETCH_HEAD in a single place

There are two different locations where we're appending to FETCH_HEAD: first when storing updated references, and second when backfilling tags. Both times we open the file, append to it and then commit it into place, which is essentially duplicate work. Improve the lifecycle of updating FETCH_HEAD by opening and committing it once in `do_fetch()`, where we pass the structure down to the code which wants to append to it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 17, 2022 at 14:04 UTC 2983cec0f26b7409ccc2dd5710b40ff4809cd4b1
1 file changed +19 -16
builtin/fetch.c
+19 -16
@@ -1084,9 +1084,8 @@ N_("it took %.2f seconds to check forced updates; you can use\n"
1084
1085 static int store_updated_refs(const char *raw_url, const char *remote_name,
1086 int connectivity_checked, struct ref *ref_map,
1087 - struct worktree **worktrees)
1087 + struct fetch_head *fetch_head, struct worktree **worktrees)
1088 {
1089 - struct fetch_head fetch_head;
1089 int url_len, i, rc = 0;
1090 struct strbuf note = STRBUF_INIT, err = STRBUF_INIT;
1091 struct ref_transaction *transaction = NULL;
@@ -1096,10 +1095,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
1095 int want_status;
1096 int summary_width = transport_summary_width(ref_map);
1097
1099 - rc = open_fetch_head(&fetch_head);
1100 - if (rc)
1101 - return -1;
1102 -
1098 if (raw_url)
1099 url = transport_anonymize_url(raw_url);
1100 else
@@ -1206,7 +1201,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
1201 strbuf_addf(&note, "'%s' of ", what);
1202 }
1203
1209 - append_fetch_head(&fetch_head, &rm->old_oid,
1204 + append_fetch_head(fetch_head, &rm->old_oid,
1205 rm->fetch_head_status,
1206 note.buf, url, url_len);
1207
@@ -1246,9 +1241,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
1241 }
1242 }
1243
1249 - if (!rc)
1250 - commit_fetch_head(&fetch_head);
1251 -
1244 if (rc & STORE_REF_ERROR_DF_CONFLICT)
1245 error(_("some local refs could not be updated; try running\n"
1246 " 'git remote prune %s' to remove any old, conflicting "
@@ -1268,7 +1260,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
1260 strbuf_release(&err);
1261 ref_transaction_free(transaction);
1262 free(url);
1271 - close_fetch_head(&fetch_head);
1263 return rc;
1264 }
1265
@@ -1309,6 +1300,7 @@ static int check_exist_and_connected(struct ref *ref_map)
1300
1301 static int fetch_and_consume_refs(struct transport *transport,
1302 struct ref *ref_map,
1303 + struct fetch_head *fetch_head,
1304 struct worktree **worktrees)
1305 {
1306 int connectivity_checked = 1;
@@ -1331,7 +1323,7 @@ static int fetch_and_consume_refs(struct transport *transport,
1323
1324 trace2_region_enter("fetch", "consume_refs", the_repository);
1325 ret = store_updated_refs(transport->url, transport->remote->name,
1334 - connectivity_checked, ref_map, worktrees);
1326 + connectivity_checked, ref_map, fetch_head, worktrees);
1327 trace2_region_leave("fetch", "consume_refs", the_repository);
1328
1329 out:
@@ -1503,7 +1495,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
1495 return transport;
1496 }
1497
1506 -static void backfill_tags(struct transport *transport, struct ref *ref_map,
1498 +static void backfill_tags(struct transport *transport,
1499 + struct ref *ref_map,
1500 + struct fetch_head *fetch_head,
1501 struct worktree **worktrees)
1502 {
1503 int cannot_reuse;
@@ -1525,7 +1519,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map,
1519 transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
1520 transport_set_option(transport, TRANS_OPT_DEPTH, "0");
1521 transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
1528 - fetch_and_consume_refs(transport, ref_map, worktrees);
1522 + fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees);
1523
1524 if (gsecondary) {
1525 transport_disconnect(gsecondary);
@@ -1544,6 +1538,7 @@ static int do_fetch(struct transport *transport,
1538 TRANSPORT_LS_REFS_OPTIONS_INIT;
1539 int must_list_refs = 1;
1540 struct worktree **worktrees = get_worktrees();
1541 + struct fetch_head fetch_head = { 0 };
1542
1543 if (tags == TAGS_DEFAULT) {
1544 if (transport->remote->fetch_tags == 2)
@@ -1601,6 +1596,10 @@ static int do_fetch(struct transport *transport,
1596 if (!update_head_ok)
1597 check_not_current_branch(ref_map, worktrees);
1598
1599 + retcode = open_fetch_head(&fetch_head);
1600 + if (retcode)
1601 + goto cleanup;
1602 +
1603 if (tags == TAGS_DEFAULT && autotags)
1604 transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
1605 if (prune) {
@@ -1619,7 +1618,8 @@ static int do_fetch(struct transport *transport,
1618 if (retcode != 0)
1619 retcode = 1;
1620 }
1622 - if (fetch_and_consume_refs(transport, ref_map, worktrees)) {
1621 +
1622 + if (fetch_and_consume_refs(transport, ref_map, &fetch_head, worktrees)) {
1623 retcode = 1;
1624 goto cleanup;
1625 }
@@ -1633,11 +1633,13 @@ static int do_fetch(struct transport *transport,
1633
1634 find_non_local_tags(remote_refs, &tags_ref_map, &tail);
1635 if (tags_ref_map)
1636 - backfill_tags(transport, tags_ref_map, worktrees);
1636 + backfill_tags(transport, tags_ref_map, &fetch_head, worktrees);
1637
1638 free_refs(tags_ref_map);
1639 }
1640
1641 + commit_fetch_head(&fetch_head);
1642 +
1643 if (set_upstream) {
1644 struct branch *branch = branch_get("HEAD");
1645 struct ref *rm;
@@ -1693,6 +1695,7 @@ static int do_fetch(struct transport *transport,
1695 }
1696
1697 cleanup:
1698 + close_fetch_head(&fetch_head);
1699 free_refs(ref_map);
1700 free_worktrees(worktrees);
1701 return retcode;