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

From: Jonathan Nieder <[email-masked]>
Date: Fri, 12 Sep 2014 12:18:12 -0700

Junio C Hamano wrote:
> Jonathan Nieder <[email-masked]> writes:
> > Junio C Hamano wrote:

>>> 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].
>
> Is this meant to replace anything, or is it "Oops, the previous ones
> are broken, and this is to patch it up on top"?

It's "Oops, the next one (refs.c: do not permit err == NULL) is broken,
and this is to patch it up in advance". :)

But it should apply on top, too.

There were a few other minor changes, and I think with them the series
should be ready for "next".  My "send and hope that more reviewers
appear" strategy didn't really work, so I'll send a reroll of the
series as-is in an hour or so.

Here's an interdiff as a preview.

diff --git a/builtin/branch.c b/builtin/branch.c
index 5d5bc56..4bf931e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -238,9 +238,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 					    RESOLVE_REF_READING
 					    | RESOLVE_REF_NODEREF
 					    | RESOLVE_REF_ALLOW_BAD_NAME);
-		if (!target ||
-		    (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
-		     is_null_sha1(sha1))) {
+		if (!target) {
 			error(remote_branch
 			      ? _("remote branch '%s' not found.")
 			      : _("branch '%s' not found."), bname.buf);
@@ -268,8 +266,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			       ? _("Deleted remote branch %s (was %s).\n")
 			       : _("Deleted branch %s (was %s).\n"),
 			       bname.buf,
-			       (flags & REF_ISSYMREF)
-			       ? target
+			       (flags & REF_ISBROKEN) ? "broken"
+			       : (flags & REF_ISSYMREF) ? target
 			       : find_unique_abbrev(sha1, DEFAULT_ABBREV));
 		}
 		delete_branch_config(bname.buf);
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);
 	}