From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
Introduces a ref backend method transaction_begin, which for the
packed backend takes packed-refs.lock.
This decouples the files backend from the packed backend, allowing the
latter to be swapped out by another ref backend.
Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
---
refs.c | 8 +++++
refs/debug.c | 15 +++++++++
refs/files-backend.c | 78 +++++++++++++++----------------------------
refs/packed-backend.c | 41 ++++++++++++++---------
refs/packed-backend.h | 10 ------
refs/refs-internal.h | 5 +++
6 files changed, 79 insertions(+), 78 deletions(-)
diff --git a/refs.c b/refs.c
index fcae5dddc60..a2d1012520e 100644
--- a/refs.c
+++ b/refs.c
@@ -1130,10 +1130,18 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
struct strbuf *err)
{
struct ref_transaction *tr;
+ int ret = 0;
assert(err);
CALLOC_ARRAY(tr, 1);
tr->ref_store = refs;
+
+ if (refs->be->transaction_begin)
+ ret = refs->be->transaction_begin(refs, tr, err);
+ if (ret) {
+ free(tr);
+ return NULL;
+ }
return tr;
}
diff --git a/refs/debug.c b/refs/debug.c
index b7ffc4ce67e..964806e9301 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -41,6 +41,20 @@ static int debug_init_db(struct ref_store *refs, struct strbuf *err)
return res;
}
+static int debug_transaction_begin(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+ int res;
+ transaction->ref_store = drefs->refs;
+ res = drefs->refs->be->transaction_begin(drefs->refs, transaction,
+ err);
+ trace_printf_key(&trace_refs, "transaction_begin: %d \"%s\"\n", res,
+ err->buf);
+ return res;
+}
+
static int debug_transaction_prepare(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err)
@@ -451,6 +465,7 @@ struct ref_storage_be refs_be_debug = {
* has a function we should also have a wrapper for it here.
* Test the output with "GIT_TRACE_REFS=1".
*/
+ .transaction_begin = debug_transaction_begin,
.transaction_prepare = debug_transaction_prepare,
.transaction_finish = debug_transaction_finish,
.transaction_abort = debug_transaction_abort,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 341354182bb..4a6781af783 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1219,10 +1219,10 @@ static int files_pack_refs(struct ref_store *ref_store,
struct ref_transaction *transaction;
transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);
- if (!transaction)
+ if (!transaction) {
+ die("could not start transaction: %s", err.buf);
return -1;
-
- packed_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR, &err);
+ }
iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,
the_repository, 0);
@@ -1262,8 +1262,6 @@ static int files_pack_refs(struct ref_store *ref_store,
ref_transaction_free(transaction);
- packed_refs_unlock(refs->packed_ref_store);
-
prune_refs(refs, &refs_to_prune);
strbuf_release(&err);
return 0;
@@ -1280,16 +1278,10 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
if (!refnames->nr)
return 0;
- if (packed_refs_lock(refs->packed_ref_store, 0, &err))
- goto error;
-
if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) {
- packed_refs_unlock(refs->packed_ref_store);
goto error;
}
- packed_refs_unlock(refs->packed_ref_store);
-
for (i = 0; i < refnames->nr; i++) {
const char *refname = refnames->items[i].string;
@@ -2640,7 +2632,7 @@ out:
struct files_transaction_backend_data {
struct ref_transaction *packed_transaction;
- int packed_refs_locked;
+ int packed_transaction_needed;
};
/*
@@ -2672,10 +2664,8 @@ static void files_transaction_cleanup(struct files_ref_store *refs,
strbuf_release(&err);
}
- if (backend_data->packed_refs_locked)
- packed_refs_unlock(refs->packed_ref_store);
-
free(backend_data);
+ transaction->backend_data = NULL;
}
transaction->state = REF_TRANSACTION_CLOSED;
@@ -2804,14 +2794,9 @@ static int files_transaction_prepare(struct ref_store *ref_store,
}
if (packed_transaction) {
- if (packed_refs_lock(refs->packed_ref_store, 0, err)) {
- ret = TRANSACTION_GENERIC_ERROR;
- goto cleanup;
- }
- backend_data->packed_refs_locked = 1;
-
- if (is_packed_transaction_needed(refs->packed_ref_store,
- packed_transaction)) {
+ backend_data->packed_transaction_needed = is_packed_transaction_needed(refs->packed_ref_store,
+ packed_transaction);
+ if (backend_data->packed_transaction_needed) {
ret = ref_transaction_prepare(packed_transaction, err);
/*
* A failure during the prepare step will abort
@@ -2823,22 +2808,6 @@ static int files_transaction_prepare(struct ref_store *ref_store,
ref_transaction_free(packed_transaction);
backend_data->packed_transaction = NULL;
}
- } else {
- /*
- * We can skip rewriting the `packed-refs`
- * file. But we do need to leave it locked, so
- * that somebody else doesn't pack a reference
- * that we are trying to delete.
- *
- * We need to disconnect our transaction from
- * backend_data, since the abort (whether successful or
- * not) will free it.
- */
- backend_data->packed_transaction = NULL;
- if (ref_transaction_abort(packed_transaction, err)) {
- ret = TRANSACTION_GENERIC_ERROR;
- goto cleanup;
- }
}
}
@@ -2940,13 +2909,24 @@ static int files_transaction_finish(struct ref_store *ref_store,
* First delete any packed versions of the references, while
* retaining the packed-refs lock:
*/
- if (packed_transaction) {
- ret = ref_transaction_commit(packed_transaction, err);
- ref_transaction_free(packed_transaction);
- packed_transaction = NULL;
- backend_data->packed_transaction = NULL;
- if (ret)
- goto cleanup;
+ if (backend_data->packed_transaction) {
+ if (backend_data->packed_transaction_needed) {
+ ret = ref_transaction_commit(packed_transaction, err);
+ if (ret)
+ goto cleanup;
+ /* TODO: leaks on error path. */
+ ref_transaction_free(packed_transaction);
+ packed_transaction = NULL;
+ backend_data->packed_transaction = NULL;
+ } else {
+ ret = ref_transaction_abort(packed_transaction, err);
+ if (ret)
+ goto cleanup;
+
+ /* transaction_commit doesn't free the data, but transaction_abort does. Go figure. */
+ packed_transaction = NULL;
+ backend_data->packed_transaction = NULL;
+ }
}
/* Now delete the loose versions of the references: */
@@ -3087,16 +3067,10 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
NULL);
}
- if (packed_refs_lock(refs->packed_ref_store, 0, err)) {
- ret = TRANSACTION_GENERIC_ERROR;
- goto cleanup;
- }
-
if (initial_ref_transaction_commit(packed_transaction, err)) {
ret = TRANSACTION_GENERIC_ERROR;
}
- packed_refs_unlock(refs->packed_ref_store);
cleanup:
if (packed_transaction)
ref_transaction_free(packed_transaction);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 59c78d7941f..5df7fa8004f 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1552,8 +1552,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;
};
@@ -1568,9 +1566,8 @@ static void packed_transaction_cleanup(struct packed_ref_store *refs,
if (is_tempfile_active(refs->tempfile))
delete_tempfile(&refs->tempfile);
- if (data->own_lock && is_lock_file_locked(&refs->lock)) {
+ if (is_lock_file_locked(&refs->lock)) {
packed_refs_unlock(&refs->base);
- data->own_lock = 0;
}
free(data);
@@ -1580,6 +1577,28 @@ static void packed_transaction_cleanup(struct packed_ref_store *refs,
transaction->state = REF_TRANSACTION_CLOSED;
}
+static int packed_transaction_begin(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct packed_ref_store *refs = packed_downcast(
+ ref_store,
+ REF_STORE_READ | REF_STORE_WRITE | REF_STORE_ODB,
+ "ref_transaction_begin");
+ struct packed_transaction_backend_data *data;
+
+ CALLOC_ARRAY(data, 1);
+ string_list_init_nodup(&data->updates);
+
+ if (!is_lock_file_locked(&refs->lock)) {
+ if (packed_refs_lock(ref_store, 0, err))
+ /* todo: leaking data */
+ return -1;
+ }
+ transaction->backend_data = data;
+ return 0;
+}
+
static int packed_transaction_prepare(struct ref_store *ref_store,
struct ref_transaction *transaction,
struct strbuf *err)
@@ -1588,7 +1607,7 @@ static int packed_transaction_prepare(struct ref_store *ref_store,
ref_store,
REF_STORE_READ | REF_STORE_WRITE | REF_STORE_ODB,
"ref_transaction_prepare");
- struct packed_transaction_backend_data *data;
+ struct packed_transaction_backend_data *data = transaction->backend_data;
size_t i;
int ret = TRANSACTION_GENERIC_ERROR;
@@ -1601,11 +1620,6 @@ static int packed_transaction_prepare(struct ref_store *ref_store,
* do so itself.
*/
- 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:
@@ -1623,12 +1637,6 @@ static int packed_transaction_prepare(struct ref_store *ref_store,
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))
goto failure;
@@ -1758,6 +1766,7 @@ struct ref_storage_be refs_be_packed = {
.name = "packed",
.init = packed_ref_store_create,
.init_db = packed_init_db,
+ .transaction_begin = packed_transaction_begin,
.transaction_prepare = packed_transaction_prepare,
.transaction_finish = packed_transaction_finish,
.transaction_abort = packed_transaction_abort,
diff --git a/refs/packed-backend.h b/refs/packed-backend.h
index 9dd8a344c34..ade3c8a5ac4 100644
--- a/refs/packed-backend.h
+++ b/refs/packed-backend.h
@@ -17,16 +17,6 @@ struct ref_store *packed_ref_store_create(struct repository *repo,
const char *gitdir,
unsigned int store_flags);
-/*
- * Lock the packed-refs file for writing. Flags is passed to
- * hold_lock_file_for_update(). Return 0 on success. On errors, write
- * an error message to `err` and return a nonzero value.
- */
-int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err);
-
-void packed_refs_unlock(struct ref_store *ref_store);
-int packed_refs_is_locked(struct ref_store *ref_store);
-
/*
* Return true if `transaction` really needs to be carried out against
* the specified packed_ref_store, or false if it can be skipped
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 9db8aec4da8..87f8e8b51d6 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -531,6 +531,10 @@ typedef struct ref_store *ref_store_init_fn(struct repository *repo,
typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err);
+typedef int ref_transaction_begin_fn(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err);
+
typedef int ref_transaction_prepare_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
@@ -670,6 +674,7 @@ struct ref_storage_be {
ref_store_init_fn *init;
ref_init_db_fn *init_db;
+ ref_transaction_begin_fn *transaction_begin;
ref_transaction_prepare_fn *transaction_prepare;
ref_transaction_finish_fn *transaction_finish;
ref_transaction_abort_fn *transaction_abort;
--
gitgitgadget
