aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--refs.c17
-rw-r--r--refs/files-backend.c67
-rw-r--r--refs/packed-backend.c25
-rw-r--r--refs/refs-internal.h2
-rw-r--r--refs/reftable-backend.c54
5 files changed, 51 insertions, 114 deletions
diff --git a/refs.c b/refs.c
index 79d5a8b8d4..22000798c7 100644
--- a/refs.c
+++ b/refs.c
@@ -1175,6 +1175,7 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
CALLOC_ARRAY(tr, 1);
tr->ref_store = refs;
tr->flags = flags;
+ string_list_init_dup(&tr->refnames);
return tr;
}
@@ -1205,6 +1206,7 @@ void ref_transaction_free(struct ref_transaction *transaction)
free((char *)transaction->updates[i]->old_target);
free(transaction->updates[i]);
}
+ string_list_clear(&transaction->refnames, 0);
free(transaction->updates);
free(transaction);
}
@@ -1218,6 +1220,7 @@ struct ref_update *ref_transaction_add_update(
const char *committer_info,
const char *msg)
{
+ struct string_list_item *item;
struct ref_update *update;
if (transaction->state != REF_TRANSACTION_OPEN)
@@ -1245,6 +1248,16 @@ struct ref_update *ref_transaction_add_update(
update->msg = normalize_reflog_message(msg);
}
+ /*
+ * This list is generally used by the backends to avoid duplicates.
+ * But we do support multiple log updates for a given refname within
+ * a single transaction.
+ */
+ if (!(update->flags & REF_LOG_ONLY)) {
+ item = string_list_append(&transaction->refnames, refname);
+ item->util = update;
+ }
+
return update;
}
@@ -2405,6 +2418,10 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
return -1;
}
+ string_list_sort(&transaction->refnames);
+ if (ref_update_reject_duplicates(&transaction->refnames, err))
+ return TRANSACTION_GENERIC_ERROR;
+
ret = refs->be->transaction_prepare(refs, transaction, err);
if (ret)
return ret;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index dab3951ccf..ecf2df556d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2378,9 +2378,7 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
*/
static int split_head_update(struct ref_update *update,
struct ref_transaction *transaction,
- const char *head_ref,
- struct string_list *affected_refnames,
- struct strbuf *err)
+ const char *head_ref, struct strbuf *err)
{
struct ref_update *new_update;
@@ -2398,7 +2396,7 @@ static int split_head_update(struct ref_update *update,
* transaction. This check is O(lg N) in the transaction
* size, but it happens at most once per transaction.
*/
- if (string_list_has_string(affected_refnames, "HEAD")) {
+ if (string_list_has_string(&transaction->refnames, "HEAD")) {
/* An entry already existed */
strbuf_addf(err,
"multiple updates for 'HEAD' (including one "
@@ -2420,7 +2418,6 @@ static int split_head_update(struct ref_update *update,
*/
if (strcmp(new_update->refname, "HEAD"))
BUG("%s unexpectedly not 'HEAD'", new_update->refname);
- string_list_insert(affected_refnames, new_update->refname);
return 0;
}
@@ -2436,7 +2433,6 @@ static int split_head_update(struct ref_update *update,
static int split_symref_update(struct ref_update *update,
const char *referent,
struct ref_transaction *transaction,
- struct string_list *affected_refnames,
struct strbuf *err)
{
struct ref_update *new_update;
@@ -2448,7 +2444,7 @@ static int split_symref_update(struct ref_update *update,
* size, but it happens at most once per symref in a
* transaction.
*/
- if (string_list_has_string(affected_refnames, referent)) {
+ if (string_list_has_string(&transaction->refnames, referent)) {
/* An entry already exists */
strbuf_addf(err,
"multiple updates for '%s' (including one "
@@ -2486,15 +2482,6 @@ static int split_symref_update(struct ref_update *update,
update->flags |= REF_LOG_ONLY | REF_NO_DEREF;
update->flags &= ~REF_HAVE_OLD;
- /*
- * Add the referent. This insertion is O(N) in the transaction
- * size, but it happens at most once per symref in a
- * transaction. Make sure to add new_update->refname, which will
- * be valid as long as affected_refnames is in use, and NOT
- * referent, which might soon be freed by our caller.
- */
- string_list_insert(affected_refnames, new_update->refname);
-
return 0;
}
@@ -2558,7 +2545,6 @@ static int lock_ref_for_update(struct files_ref_store *refs,
struct ref_transaction *transaction,
const char *head_ref,
struct string_list *refnames_to_check,
- struct string_list *affected_refnames,
struct strbuf *err)
{
struct strbuf referent = STRBUF_INIT;
@@ -2575,8 +2561,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
update->flags |= REF_DELETING;
if (head_ref) {
- ret = split_head_update(update, transaction, head_ref,
- affected_refnames, err);
+ ret = split_head_update(update, transaction, head_ref, err);
if (ret)
goto out;
}
@@ -2586,9 +2571,8 @@ static int lock_ref_for_update(struct files_ref_store *refs,
lock->count++;
} else {
ret = lock_raw_ref(refs, update->refname, mustexist,
- refnames_to_check, affected_refnames,
- &lock, &referent,
- &update->type, err);
+ refnames_to_check, &transaction->refnames,
+ &lock, &referent, &update->type, err);
if (ret) {
char *reason;
@@ -2642,9 +2626,8 @@ static int lock_ref_for_update(struct files_ref_store *refs,
* of processing the split-off update, so we
* don't have to do it here.
*/
- ret = split_symref_update(update,
- referent.buf, transaction,
- affected_refnames, err);
+ ret = split_symref_update(update, referent.buf,
+ transaction, err);
if (ret)
goto out;
}
@@ -2799,7 +2782,6 @@ static int files_transaction_prepare(struct ref_store *ref_store,
"ref_transaction_prepare");
size_t i;
int ret = 0;
- struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
struct string_list refnames_to_check = STRING_LIST_INIT_NODUP;
char *head_ref = NULL;
int head_type;
@@ -2818,12 +2800,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
transaction->backend_data = backend_data;
/*
- * Fail if a refname appears more than once in the
- * transaction. (If we end up splitting up any updates using
- * split_symref_update() or split_head_update(), those
- * functions will check that the new updates don't have the
- * same refname as any existing ones.) Also fail if any of the
- * updates use REF_IS_PRUNING without REF_NO_DEREF.
+ * Fail if any of the updates use REF_IS_PRUNING without REF_NO_DEREF.
*/
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];
@@ -2831,16 +2808,6 @@ static int files_transaction_prepare(struct ref_store *ref_store,
if ((update->flags & REF_IS_PRUNING) &&
!(update->flags & REF_NO_DEREF))
BUG("REF_IS_PRUNING set without REF_NO_DEREF");
-
- if (update->flags & REF_LOG_ONLY)
- continue;
-
- string_list_append(&affected_refnames, update->refname);
- }
- string_list_sort(&affected_refnames);
- if (ref_update_reject_duplicates(&affected_refnames, err)) {
- ret = TRANSACTION_GENERIC_ERROR;
- goto cleanup;
}
/*
@@ -2882,7 +2849,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
ret = lock_ref_for_update(refs, update, transaction,
head_ref, &refnames_to_check,
- &affected_refnames, err);
+ err);
if (ret)
goto cleanup;
@@ -2929,7 +2896,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
* So instead, we accept the race for now.
*/
if (refs_verify_refnames_available(refs->packed_ref_store, &refnames_to_check,
- &affected_refnames, NULL, 0, err)) {
+ &transaction->refnames, NULL, 0, err)) {
ret = TRANSACTION_NAME_CONFLICT;
goto cleanup;
}
@@ -2975,7 +2942,6 @@ static int files_transaction_prepare(struct ref_store *ref_store,
cleanup:
free(head_ref);
- string_list_clear(&affected_refnames, 0);
string_list_clear(&refnames_to_check, 0);
if (ret)
@@ -3050,13 +3016,8 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,
if (transaction->state != REF_TRANSACTION_PREPARED)
BUG("commit called for transaction that is not prepared");
- /* Fail if a refname appears more than once in the transaction: */
- for (i = 0; i < transaction->nr; i++)
- if (!(transaction->updates[i]->flags & REF_LOG_ONLY))
- string_list_append(&affected_refnames,
- transaction->updates[i]->refname);
- string_list_sort(&affected_refnames);
- if (ref_update_reject_duplicates(&affected_refnames, err)) {
+ string_list_sort(&transaction->refnames);
+ if (ref_update_reject_duplicates(&transaction->refnames, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
@@ -3074,7 +3035,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,
* that we are creating already exists.
*/
if (refs_for_each_rawref(&refs->base, ref_present,
- &affected_refnames))
+ &transaction->refnames))
BUG("initial ref transaction called with existing refs");
packed_transaction = ref_store_transaction_begin(refs->packed_ref_store,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index f4c82ba2c7..19220d2e99 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1622,8 +1622,6 @@ int is_packed_transaction_needed(struct ref_store *ref_store,
struct packed_transaction_backend_data {
/* True iff the transaction owns the packed-refs lock. */
int own_lock;
-
- struct string_list updates;
};
static void packed_transaction_cleanup(struct packed_ref_store *refs,
@@ -1632,8 +1630,6 @@ static void packed_transaction_cleanup(struct packed_ref_store *refs,
struct packed_transaction_backend_data *data = transaction->backend_data;
if (data) {
- string_list_clear(&data->updates, 0);
-
if (is_tempfile_active(refs->tempfile))
delete_tempfile(&refs->tempfile);
@@ -1658,7 +1654,6 @@ static int packed_transaction_prepare(struct ref_store *ref_store,
REF_STORE_READ | REF_STORE_WRITE | REF_STORE_ODB,
"ref_transaction_prepare");
struct packed_transaction_backend_data *data;
- size_t i;
int ret = TRANSACTION_GENERIC_ERROR;
/*
@@ -1671,34 +1666,16 @@ static int packed_transaction_prepare(struct ref_store *ref_store,
*/
CALLOC_ARRAY(data, 1);
- string_list_init_nodup(&data->updates);
transaction->backend_data = data;
- /*
- * Stick the updates in a string list by refname so that we
- * can sort them:
- */
- for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = transaction->updates[i];
- struct string_list_item *item =
- string_list_append(&data->updates, update->refname);
-
- /* Store a pointer to update in item->util: */
- item->util = update;
- }
- string_list_sort(&data->updates);
-
- if (ref_update_reject_duplicates(&data->updates, err))
- goto failure;
-
if (!is_lock_file_locked(&refs->lock)) {
if (packed_refs_lock(ref_store, 0, err))
goto failure;
data->own_lock = 1;
}
- if (write_with_updates(refs, &data->updates, err))
+ if (write_with_updates(refs, &transaction->refnames, err))
goto failure;
transaction->state = REF_TRANSACTION_PREPARED;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index e5862757a7..92db793026 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -3,6 +3,7 @@
#include "refs.h"
#include "iterator.h"
+#include "string-list.h"
struct fsck_options;
struct ref_transaction;
@@ -198,6 +199,7 @@ enum ref_transaction_state {
struct ref_transaction {
struct ref_store *ref_store;
struct ref_update **updates;
+ struct string_list refnames;
size_t alloc;
size_t nr;
enum ref_transaction_state state;
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index c8f86da731..3688ffd683 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1076,7 +1076,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
struct reftable_ref_store *refs =
reftable_be_downcast(ref_store, REF_STORE_WRITE|REF_STORE_MAIN, "ref_transaction_prepare");
struct strbuf referent = STRBUF_INIT, head_referent = STRBUF_INIT;
- struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
struct string_list refnames_to_check = STRING_LIST_INIT_NODUP;
struct reftable_transaction_data *tx_data = NULL;
struct reftable_backend *be;
@@ -1101,10 +1100,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
transaction->updates[i], err);
if (ret)
goto done;
-
- if (!(transaction->updates[i]->flags & REF_LOG_ONLY))
- string_list_append(&affected_refnames,
- transaction->updates[i]->refname);
}
/*
@@ -1117,17 +1112,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
}
/*
- * Fail if a refname appears more than once in the transaction.
- * This code is taken from the files backend and is a good candidate to
- * be moved into the generic layer.
- */
- string_list_sort(&affected_refnames);
- if (ref_update_reject_duplicates(&affected_refnames, err)) {
- ret = TRANSACTION_GENERIC_ERROR;
- goto done;
- }
-
- /*
* TODO: it's dubious whether we should reload the stack that "HEAD"
* belongs to or not. In theory, it may happen that we only modify
* stacks which are _not_ part of the "HEAD" stack. In that case we
@@ -1194,14 +1178,12 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
!(u->flags & REF_LOG_ONLY) &&
!(u->flags & REF_UPDATE_VIA_HEAD) &&
!strcmp(rewritten_ref, head_referent.buf)) {
- struct ref_update *new_update;
-
/*
* First make sure that HEAD is not already in the
* transaction. This check is O(lg N) in the transaction
* size, but it happens at most once per transaction.
*/
- if (string_list_has_string(&affected_refnames, "HEAD")) {
+ if (string_list_has_string(&transaction->refnames, "HEAD")) {
/* An entry already existed */
strbuf_addf(err,
_("multiple updates for 'HEAD' (including one "
@@ -1211,12 +1193,11 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
goto done;
}
- new_update = ref_transaction_add_update(
- transaction, "HEAD",
- u->flags | REF_LOG_ONLY | REF_NO_DEREF,
- &u->new_oid, &u->old_oid, NULL, NULL, NULL,
- u->msg);
- string_list_insert(&affected_refnames, new_update->refname);
+ ref_transaction_add_update(
+ transaction, "HEAD",
+ u->flags | REF_LOG_ONLY | REF_NO_DEREF,
+ &u->new_oid, &u->old_oid, NULL, NULL, NULL,
+ u->msg);
}
ret = reftable_backend_read_ref(be, rewritten_ref,
@@ -1281,6 +1262,15 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
if (!strcmp(rewritten_ref, "HEAD"))
new_flags |= REF_UPDATE_VIA_HEAD;
+ if (string_list_has_string(&transaction->refnames, referent.buf)) {
+ strbuf_addf(err,
+ _("multiple updates for '%s' (including one "
+ "via symref '%s') are not allowed"),
+ referent.buf, u->refname);
+ ret = TRANSACTION_NAME_CONFLICT;
+ goto done;
+ }
+
/*
* If we are updating a symref (eg. HEAD), we should also
* update the branch that the symref points to.
@@ -1305,16 +1295,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
*/
u->flags |= REF_LOG_ONLY | REF_NO_DEREF;
u->flags &= ~REF_HAVE_OLD;
-
- if (string_list_has_string(&affected_refnames, new_update->refname)) {
- strbuf_addf(err,
- _("multiple updates for '%s' (including one "
- "via symref '%s') are not allowed"),
- referent.buf, u->refname);
- ret = TRANSACTION_NAME_CONFLICT;
- goto done;
- }
- string_list_insert(&affected_refnames, new_update->refname);
}
}
@@ -1383,7 +1363,8 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
}
}
- ret = refs_verify_refnames_available(ref_store, &refnames_to_check, &affected_refnames, NULL,
+ ret = refs_verify_refnames_available(ref_store, &refnames_to_check,
+ &transaction->refnames, NULL,
transaction->flags & REF_TRANSACTION_FLAG_INITIAL,
err);
if (ret < 0)
@@ -1401,7 +1382,6 @@ done:
strbuf_addf(err, _("reftable: transaction prepare: %s"),
reftable_error_str(ret));
}
- string_list_clear(&affected_refnames, 0);
strbuf_release(&referent);
strbuf_release(&head_referent);
string_list_clear(&refnames_to_check, 0);