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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +static int backfill_tags(struct transport *transport,
> ...
>  }

The change to this function is quite straight-forward.

>  static int do_fetch(struct transport *transport,
> @@ -1632,8 +1634,16 @@ 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 of tags fails then we want to tell
> +			 * the user so, but we have to continue regardless to
> +			 * populate upstream information of the references we
> +			 * have already fetched above.
> +			 */

OK.  Unless atomic, using the available information on successfully
updated ones would be fine.  Perhaps under --atomic mode, a failure
to commit the ref transaction will also prevent the upstream
information from getting updated?  We'll see soon enough.

> +			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 6ffe2a5719..c057c49e80 100755
> --- a/t/t5503-tagfollow.sh
> +++ b/t/t5503-tagfollow.sh
> @@ -233,9 +233,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 &&

OK.

>  	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