[PATCH 20/31] refs.c: check for lock conflicts already in _update

[Ronnie Sahlberg <sahlberg@google.com>]

Check for lock conflicts in _update and flag the transaction as errored
instead of waiting until _commit for doing these checks. If there was a lock
failure we check if this was due to a previous update in the same transaction
or not. This so that we can preserve the current error messages on lock failure
and log "Multiple updates for ref '%s' not allowed." when we have multiple
updates in the same transaction and "Cannot lock the ref '%s'." for any other
reason that the lock failed.

This also means that we no longer need to sort all the refs and check for
duplicates during _commit so all that code can be removed too.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
 refs.c | 85 +++++++++++++++++++++++++++++++++---------------------------------
 1 file changed, 42 insertions(+), 43 deletions(-)

diff --git a/refs.c b/refs.c
index 93f01e8..76cab6e 100644
--- a/refs.c
+++ b/refs.c
@@ -3415,6 +3415,7 @@ int transaction_update_sha1(struct ref_transaction *transaction,
 			    struct strbuf *err)
 {
 	struct ref_update *update;
+	int i;
 
 	if (have_old && !old_sha1)
 		die("BUG: have_old is true but old_sha1 is NULL");
@@ -3438,19 +3439,49 @@ int transaction_update_sha1(struct ref_transaction *transaction,
 	 * we do not need to check against this ref for name collissions
 	 * during locking.
 	 */
-	if (update->flags & REF_ISPACKONLY)
+	if (update->flags & REF_ISPACKONLY) {
 		add_delname(transaction, update->refname);
-	else {
-		update->lock = lock_ref_sha1_basic(update->refname,
-						   (update->have_old ?
-						    update->old_sha1 :
-						    NULL),
-						   update->flags,
-						   &update->type,
-						   transaction->delnames,
-						   transaction->delnum);
+		return 0;
 	}
-	return 0;
+
+	update->lock = lock_ref_sha1_basic(update->refname,
+					   (update->have_old ?
+					    update->old_sha1 :
+					    NULL),
+					   update->flags,
+					   &update->type,
+					   transaction->delnames,
+					   transaction->delnum);
+	if (update->lock)
+		return 0;
+
+	/* If we could not lock the ref it means we either collided with a
+	   different command or that we tried to perform a second update to
+	   the same ref from within the same transaction.
+	*/
+	transaction->status = REF_TRANSACTION_ERROR;
+
+	/* -1 is the update we just added. Start at -2 and find the most recent
+	   previous update for this ref.
+	*/
+	for (i = transaction->nr - 2; i >= 0; i--) {
+		if (transaction->updates[i]->update_type != UPDATE_SHA1) {
+			continue;
+		}
+		if (!strcmp(transaction->updates[i]->refname,
+			    update->refname))
+			break;
+	}
+	if (err)
+		if (i >= 0) {
+			const char *str =
+				"Multiple updates for ref '%s' not allowed.";
+			strbuf_addf(err, str, update->refname);
+		} else {
+			const char *str = "Cannot lock the ref '%s'.";
+			strbuf_addf(err, str, update->refname);
+		}
+	return 1;
 }
 
 int transaction_create_sha1(struct ref_transaction *transaction,
@@ -3525,32 +3556,6 @@ int update_ref(const char *action, const char *refname,
 	return 0;
 }
 
-static int ref_update_compare(const void *r1, const void *r2)
-{
-	const struct ref_update * const *u1 = r1;
-	const struct ref_update * const *u2 = r2;
-	return strcmp((*u1)->refname, (*u2)->refname);
-}
-
-static int ref_update_reject_duplicates(struct ref_update **updates, int n,
-					struct strbuf *err)
-{
-	int i;
-	for (i = 1; i < n; i++) {
-		if (updates[i]->update_type != UPDATE_SHA1)
-			continue;
-		if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
-			const char *str =
-				"Multiple updates for ref '%s' not allowed.";
-			if (err)
-				strbuf_addf(err, str, updates[i]->refname);
-
-			return 1;
-		}
-	}
-	return 0;
-}
-
 int transaction_commit(struct ref_transaction *transaction,
 			   struct strbuf *err)
 {
@@ -3569,12 +3574,6 @@ int transaction_commit(struct ref_transaction *transaction,
 		return 0;
 	}
 
-	/* Copy, sort, and reject duplicate refs */
-	qsort(updates, n, sizeof(*updates), ref_update_compare);
-	ret = ref_update_reject_duplicates(updates, n, err);
-	if (ret)
-		goto cleanup;
-
 	/* Acquire all ref locks while verifying old values */
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
-- 
2.0.0.rc3.506.g3739a35