fetch: backfill tags before setting upstream

The fetch code flow is a bit hard to understand right now: 1. We optionally prune all references which have vanished on the remote side. 2. We fetch and update all other references locally. 3. We update the upstream branch in the gitconfig. 4. We backfill tags pointing into the history we have just fetched. It is quite confusing that we fetch objects and update references in both (2) and (4), which is further stressed by the point that we use a `skip` goto label to jump from (3) to (4) in case we fail to update the gitconfig as expected. Reorder the code to first update all local references, and only after we have done so update the upstream branch information. This improves the code flow and furthermore makes it easier to refactor the way we update references together. 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 efbade066083eb0a8ccee5a8290cd3fc834705f3
1 file changed +18 -17
builtin/fetch.c
+18 -17
@@ -1536,7 +1536,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map,
1536 static int do_fetch(struct transport *transport,
1537 struct refspec *rs)
1538 {
1539 - struct ref *ref_map;
1539 + struct ref *ref_map = NULL;
1540 int autotags = (transport->remote->fetch_tags == 1);
1541 int retcode = 0;
1542 const struct ref *remote_refs;
@@ -1620,11 +1620,24 @@ static int do_fetch(struct transport *transport,
1620 retcode = 1;
1621 }
1622 if (fetch_and_consume_refs(transport, ref_map, worktrees)) {
1623 - free_refs(ref_map);
1623 retcode = 1;
1624 goto cleanup;
1625 }
1626
1627 + /*
1628 + * If neither --no-tags nor --tags was specified, do automated tag
1629 + * following.
1630 + */
1631 + if (tags == TAGS_DEFAULT && autotags) {
1632 + struct ref *tags_ref_map = NULL, **tail = &tags_ref_map;
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);
1637 +
1638 + free_refs(tags_ref_map);
1639 + }
1640 +
1641 if (set_upstream) {
1642 struct branch *branch = branch_get("HEAD");
1643 struct ref *rm;
@@ -1644,7 +1657,7 @@ static int do_fetch(struct transport *transport,
1657 if (!rm->peer_ref) {
1658 if (source_ref) {
1659 warning(_("multiple branches detected, incompatible with --set-upstream"));
1647 - goto skip;
1660 + goto cleanup;
1661 } else {
1662 source_ref = rm;
1663 }
@@ -1658,7 +1671,7 @@ static int do_fetch(struct transport *transport,
1671 warning(_("could not set upstream of HEAD to '%s' from '%s' when "
1672 "it does not point to any branch."),
1673 shortname, transport->remote->name);
1661 - goto skip;
1674 + goto cleanup;
1675 }
1676
1677 if (!strcmp(source_ref->name, "HEAD") ||
@@ -1678,21 +1691,9 @@ static int do_fetch(struct transport *transport,
1691 "you need to specify exactly one branch with the --set-upstream option"));
1692 }
1693 }
1681 -skip:
1682 - free_refs(ref_map);
1683 -
1684 - /* if neither --no-tags nor --tags was specified, do automated tag
1685 - * following ... */
1686 - if (tags == TAGS_DEFAULT && autotags) {
1687 - struct ref **tail = &ref_map;
1688 - ref_map = NULL;
1689 - find_non_local_tags(remote_refs, &ref_map, &tail);
1690 - if (ref_map)
1691 - backfill_tags(transport, ref_map, worktrees);
1692 - free_refs(ref_map);
1693 - }
1694
1695 cleanup:
1696 + free_refs(ref_map);
1697 free_worktrees(worktrees);
1698 return retcode;
1699 }