Re: [PATCH 4/6] fetch: report errors when backfilling tags fails

>
> When the backfilling of tags fails we do not report this error to the
> caller, but only report it implicitly at a later point when reporting
> updated references. This leaves callers unable to act upon the
> information of whether the backfilling succeeded or not.
>
> Refactor the function to return an error code and pass it up the
> callstack. This causes us to correctly propagate the error back to the
> user of git-fetch(1).
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/fetch.c      | 29 +++++++++++++++++++++--------
>  t/t5503-tagfollow.sh |  4 +---
>  2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 627847e2f8..1eda0b68ff 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1495,12 +1495,12 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
>         return transport;
>  }
>
> -static void backfill_tags(struct transport *transport,
> -                         struct ref *ref_map,
> -                         struct fetch_head *fetch_head,
> -                         struct worktree **worktrees)
> +static int backfill_tags(struct transport *transport,
> +                        struct ref *ref_map,
> +                        struct fetch_head *fetch_head,
> +                        struct worktree **worktrees)
>  {
> -       int cannot_reuse;
> +       int retcode, cannot_reuse;
>
>         /*
>          * Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it
> @@ -1519,12 +1519,14 @@ static void backfill_tags(struct transport *transport,
>         transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
>         transport_set_option(transport, TRANS_OPT_DEPTH, "0");
>         transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
> -       fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees);
> +       retcode = fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees);
>
>         if (gsecondary) {
>                 transport_disconnect(gsecondary);
>                 gsecondary = NULL;
>         }
> +
> +       return retcode;
>  }
>
>  static int do_fetch(struct transport *transport,
> @@ -1628,8 +1630,19 @@ static int do_fetch(struct transport *transport,
>                 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, &fetch_head, worktrees);
> +               if (tags_ref_map) {
> +                       /*
> +                        * If backfilling tags succeeds we used to not return
> +                        * an error code to the user at all. Instead, we
> +                        * silently swallowed that error and updated the local
> +                        * state of the repository. We now notify the user of
> +                        * any such errors, but we continue to make sure that
> +                        * FETCH_HEAD and the upstream branch are configured as
> +                        * expected.
> +                        */

> +                       if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees))
> +                               retcode = 1;
> +               }
>
>                 free_refs(tags_ref_map);
>         }
> diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
> index 888305ad4d..549f908b90 100755
> --- a/t/t5503-tagfollow.sh
> +++ b/t/t5503-tagfollow.sh
> @@ -237,9 +237,7 @@ test_expect_success 'backfill failure causes command to fail' '
>                 done
>         EOF
>
> -       # Even though we fail to create refs/tags/tag1 the below command
> -       # unexpectedly succeeds.
> -       git -C clone5 fetch .. $B:refs/heads/something &&
> +       test_must_fail git -C clone5 fetch .. $B:refs/heads/something &&
>         test $B = $(git -C clone5 rev-parse --verify refs/heads/something) &&
>         test $S = $(git -C clone5 rev-parse --verify tag2) &&
>         test_must_fail git -C clone5 rev-parse --verify tag1
> --
> 2.35.1
>