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

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

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

> 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?