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@xxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html