On Fri, Feb 11, 2022 at 5:12 PM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> 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 require
s/require/use/
> a `skip` label which jumps from (3) to (4) in case we fail to update the
s/`skip` label/`skip` goto label/
s/which jumps/to jump/
> 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.
> @@ -1536,7 +1536,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map,
> static int do_fetch(struct transport *transport,
> struct refspec *rs)
> {
> - struct ref *ref_map;
> + struct ref *ref_map = NULL;
> int autotags = (transport->remote->fetch_tags == 1);
> int retcode = 0;
> const struct ref *remote_refs;
> @@ -1618,11 +1618,22 @@ static int do_fetch(struct transport *transport,
> }
> }
> if (fetch_and_consume_refs(transport, ref_map, worktrees)) {
> - free_refs(ref_map);
> retcode = 1;
> goto cleanup;
> }
>
> + /* if neither --no-tags nor --tags was specified, do automated tag
> + * following ... */
Maybe while at it this could be changed to use our usual style for
multi-line comments:
/*
* If neither --no-tags nor --tags was specified, do automated tag
* following...
*/
> + if (tags == TAGS_DEFAULT && autotags) {
> + struct ref *tags_ref_map = NULL, **tail = &tags_ref_map;
> +
> + find_non_local_tags(remote_refs, &tags_ref_map, &tail);
> + if (tags_ref_map)
> + backfill_tags(transport, tags_ref_map, worktrees);
> +
> + free_refs(tags_ref_map);
> + }
> @@ -1676,21 +1687,9 @@ static int do_fetch(struct transport *transport,
> "you need to specify exactly one branch with the --set-upstream option"));
> }
> }
> -skip:
I like that it's removing one goto label and making the code simpler.
