Re: [PATCH v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)

> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> These patches are also available from the git repository at
>>>
>>>   git://repo.or.cz/git/jrn.git tags/rs/ref-transaction
>>
>> The tag fetched and built as-is seems to break 5514 among other
>> things ("git remote rm" segfaults).
>
> Yeah, I noticed that right after sending the series out. :/
>
> The patch below fixes it[1].

> -- >8 --
> From: Ronnie Sahlberg <sahlberg@google.com>
> Date: Thu, 11 Sep 2014 08:42:57 -0700
> Subject: remote rm/prune: print a message when writing packed-refs fails
>
> Until v2.1.0-rc0~22^2~11 (refs.c: add an err argument to
> repack_without_refs, 2014-06-20), repack_without_refs forgot to
> provide an error message when commit_packed_refs fails.  Even today,
> it only provides a message for callers that pass a non-NULL err
> parameter.  Internal callers in refs.c pass non-NULL err but
> "git remote" does not.
>
> That means that "git remote rm" and "git remote prune" can fail
> without printing a message about why.  Fix them by passing in a
> non-NULL err parameter and printing the returned message.
>
> This is the last caller to a ref handling function passing err ==
> NULL.  A later patch can drop support for err == NULL, avoiding such
> problems in the future.
>
> Change-Id: Ifb8a726ef03d0aa282a25a102313064d2e8ec283
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> [1] https://code-review.googlesource.com/1110
>     https://code-review.googlesource.com/1060
>
>  builtin/remote.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 6eaeee7..ef1ffc3 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -750,13 +750,16 @@ static int mv(int argc, const char **argv)
>  
>  static int remove_branches(struct string_list *branches)
>  {
> +	struct strbuf err = STRBUF_INIT;
>  	const char **branch_names;
>  	int i, result = 0;
>  
>  	branch_names = xmalloc(branches->nr * sizeof(*branch_names));
>  	for (i = 0; i < branches->nr; i++)
>  		branch_names[i] = branches->items[i].string;
> -	result |= repack_without_refs(branch_names, branches->nr, NULL);
> +	if (repack_without_refs(branch_names, branches->nr, &err))
> +		result |= error("%s", err.buf);
> +	strbuf_release(&err);
>  	free(branch_names);
>  
>  	for (i = 0; i < branches->nr; i++) {
> @@ -1333,9 +1336,13 @@ static int prune_remote(const char *remote, int dry_run)
>  		delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
>  		for (i = 0; i < states.stale.nr; i++)
>  			delete_refs[i] = states.stale.items[i].util;
> -		if (!dry_run)
> -			result |= repack_without_refs(delete_refs,
> -						      states.stale.nr, NULL);
> +		if (!dry_run) {
> +			struct strbuf err = STRBUF_INIT;
> +			if (repack_without_refs(delete_refs, states.stale.nr,
> +						&err))
> +				result |= error("%s", err.buf);
> +			strbuf_release(&err);
> +		}
>  		free(delete_refs);
>  	}