Re: [PATCH 3/3] fetch --prune: Repair branchname DF conflicts

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

Tom Miller <jackerran@gmail.com> writes:

> When a branchname DF conflict occurs during a fetch,

You may have started with a specific case in which you want to
change the behaviour of current Git, so it may be clear what you
meant by "branchname DF conflict", but that is true for nobody other
than you who will read this log message.  Introducing new lingo is
OK as long as it is necessary, but in a case like this, where you
have to describe what situation you are trying to address anyway,
I do not think you need to add a new word to our vocabulary.

	When we have a remote-tracking branch frotz/nitfol from a
	previous fetch, and the upstream now has branch frotz, we
	used to fail to remove frotz/nitfol and recreate frotz with
	"git fetch --prune" from the upstream.

or something like that?

But what should happen when we do not give --prune to "git fetch" in
such a situation?  Should it fail, because we still have frotz/nitfol
and we cannot create frotz without losing it?

> --prune should
> be able to fix it. When fetching with --prune, the fetching process
> happens before pruning causing the branchname DF conflict to persist
> and report an error. This patch prunes before fetching, thus
> correcting DF conflicts during a fetch.
>
> Signed-off-by: Tom Miller <jackerran@gmail.com>
> Tested-by: Thomas Rast <tr@thomasrast.ch>

I wasn't following previous threads closely (was there a previous
thread???); has this iteration been already tested by trast?

> ---
>  builtin/fetch.c  | 10 +++++-----
>  t/t5510-fetch.sh | 14 ++++++++++++++
>  2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index e50b697..845c687 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,
>  
>  	if (tags == TAGS_DEFAULT && autotags)
>  		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
> -	if (fetch_refs(transport, ref_map)) {
> -		free_refs(ref_map);
> -		retcode = 1;
> -		goto cleanup;
> -	}
>  	if (prune) {
>  		/*
>  		 * We only prune based on refspecs specified
> @@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,
>  				   transport->url);
>  		}
>  	}
> +	if (fetch_refs(transport, ref_map)) {
> +		free_refs(ref_map);
> +		retcode = 1;
> +		goto cleanup;
> +	}
>  	free_refs(ref_map);
>  
>  	/* if neither --no-tags nor --tags was specified, do automated tag
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 5d4581d..a981125 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded' '
>  	test_bundle_object_count .git/objects/pack/pack-${pack##pack	}.pack 3
>  '
>  
> +test_expect_success 'branchname D/F conflict resolved by --prune' '
> +	git branch dir/file &&
> +	git clone . prune-df-conflict &&
> +	git branch -D dir/file &&
> +	git branch dir &&
> +	(
> +		cd prune-df-conflict &&
> +		git fetch --prune &&
> +		git rev-parse origin/dir >../actual
> +	) &&
> +	git rev-parse dir >expect &&
> +	test_cmp expect actual
> +'
> +
>  test_done