Re: [PATCH v2 06/12] prune_remote(): use delete_refs()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 15, 2015, 18:35 UTC
Message-ID: <xmqq381sakgf.fsf@gitster.dls.corp.google.com>
In-Reply-To: <aab9d191609b34fbfee0b68883a5b14fba2421ad.1434206062.git.mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 34 quoted lines

> This will result in errors being emitted for references that can't be
> deleted, but that is a good thing.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  builtin/remote.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index c8dc724..cc3c741 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -1314,19 +1314,12 @@ static int prune_remote(const char *remote, int dry_run)
>  		string_list_append(&refs_to_prune, item->util);
>  	string_list_sort(&refs_to_prune);
>  
> -	if (!dry_run) {
> -		struct strbuf err = STRBUF_INIT;
> -		if (repack_without_refs(&refs_to_prune, &err))
> -			result |= error("%s", err.buf);
> -		strbuf_release(&err);
> -	}
> +	if (!dry_run)
> +		result |= delete_refs(&refs_to_prune);
>  
>  	for_each_string_list_item(item, &states.stale) {
>  		const char *refname = item->util;
>  
> -		if (!dry_run)
> -			result |= delete_ref(refname, NULL, 0);
> -
>  		if (dry_run)
>  			printf_ln(_(" * [would prune] %s"),
>  			       abbrev_ref(refname, "refs/remotes/"));

The resulting code reads better by making the for-each-string-list-item loop only about reporting and not actually doing anything.

But the log message puzzles me. Didn't refs that cannot be deleted cause the original to fail? After repacking without these refs, it called delete-ref, and a failure to delete or commit the deletion would have hit the error() down there, no?