From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
It is no longer specific to the packed backend.
Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
---
refs/files-backend.c | 96 +++++++++++++++++++++++++++++++++++++++++++
refs/packed-backend.c | 95 ------------------------------------------
refs/packed-backend.h | 9 ----
3 files changed, 96 insertions(+), 104 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4a6781af783..c0a7e3d375b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2630,6 +2630,101 @@ out:
return ret;
}
+
+/*
+ * Return true if `transaction` really needs to be carried out against
+ * the specified packed_ref_store, or false if it can be skipped
+ * (i.e., because it is an obvious NOOP). `ref_store` must be locked
+ * before calling this function.
+ */
+static int is_packed_transaction_needed(struct ref_store *ref_store,
+ struct ref_transaction *transaction)
+{
+ struct strbuf referent = STRBUF_INIT;
+ size_t i;
+ int ret;
+
+ /*
+ * We're only going to bother returning false for the common,
+ * trivial case that references are only being deleted, their
+ * old values are not being checked, and the old `packed-refs`
+ * file doesn't contain any of those reference(s). This gives
+ * false positives for some other cases that could
+ * theoretically be optimized away:
+ *
+ * 1. It could be that the old value is being verified without
+ * setting a new value. In this case, we could verify the
+ * old value here and skip the update if it agrees. If it
+ * disagrees, we could either let the update go through
+ * (the actual commit would re-detect and report the
+ * problem), or come up with a way of reporting such an
+ * error to *our* caller.
+ *
+ * 2. It could be that a new value is being set, but that it
+ * is identical to the current packed value of the
+ * reference.
+ *
+ * Neither of these cases will come up in the current code,
+ * because the only caller of this function passes to it a
+ * transaction that only includes `delete` updates with no
+ * `old_id`. Even if that ever changes, false positives only
+ * cause an optimization to be missed; they do not affect
+ * correctness.
+ */
+
+ /*
+ * Start with the cheap checks that don't require old
+ * reference values to be read:
+ */
+ for (i = 0; i < transaction->nr; i++) {
+ struct ref_update *update = transaction->updates[i];
+
+ if (update->flags & REF_HAVE_OLD)
+ /* Have to check the old value -> needed. */
+ return 1;
+
+ if ((update->flags & REF_HAVE_NEW) && !is_null_oid(&update->new_oid))
+ /* Have to set a new value -> needed. */
+ return 1;
+ }
+
+ /*
+ * The transaction isn't checking any old values nor is it
+ * setting any nonzero new values, so it still might be able
+ * to be skipped. Now do the more expensive check: the update
+ * is needed if any of the updates is a delete, and the old
+ * `packed-refs` file contains a value for that reference.
+ */
+ ret = 0;
+ for (i = 0; i < transaction->nr; i++) {
+ struct ref_update *update = transaction->updates[i];
+ int failure_errno;
+ unsigned int type;
+ struct object_id oid;
+
+ if (!(update->flags & REF_HAVE_NEW))
+ /*
+ * This reference isn't being deleted -> not
+ * needed.
+ */
+ continue;
+
+ if (!refs_read_raw_ref(ref_store, update->refname, &oid,
+ &referent, &type, &failure_errno) ||
+ failure_errno != ENOENT) {
+ /*
+ * We have to actually delete that reference
+ * -> this transaction is needed.
+ */
+ ret = 1;
+ break;
+ }
+ }
+
+ strbuf_release(&referent);
+ return ret;
+}
+
struct files_transaction_backend_data {
struct ref_transaction *packed_transaction;
int packed_transaction_needed;
@@ -3294,3 +3389,4 @@ struct ref_storage_be refs_be_files = {
.delete_reflog = files_delete_reflog,
.reflog_expire = files_reflog_expire
};
+
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 5df7fa8004f..ba895c845c0 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1455,101 +1455,6 @@ error:
return -1;
}
-int is_packed_transaction_needed(struct ref_store *ref_store,
- struct ref_transaction *transaction)
-{
- struct packed_ref_store *refs = packed_downcast(
- ref_store,
- REF_STORE_READ,
- "is_packed_transaction_needed");
- struct strbuf referent = STRBUF_INIT;
- size_t i;
- int ret;
-
- if (!is_lock_file_locked(&refs->lock))
- BUG("is_packed_transaction_needed() called while unlocked");
-
- /*
- * We're only going to bother returning false for the common,
- * trivial case that references are only being deleted, their
- * old values are not being checked, and the old `packed-refs`
- * file doesn't contain any of those reference(s). This gives
- * false positives for some other cases that could
- * theoretically be optimized away:
- *
- * 1. It could be that the old value is being verified without
- * setting a new value. In this case, we could verify the
- * old value here and skip the update if it agrees. If it
- * disagrees, we could either let the update go through
- * (the actual commit would re-detect and report the
- * problem), or come up with a way of reporting such an
- * error to *our* caller.
- *
- * 2. It could be that a new value is being set, but that it
- * is identical to the current packed value of the
- * reference.
- *
- * Neither of these cases will come up in the current code,
- * because the only caller of this function passes to it a
- * transaction that only includes `delete` updates with no
- * `old_id`. Even if that ever changes, false positives only
- * cause an optimization to be missed; they do not affect
- * correctness.
- */
-
- /*
- * Start with the cheap checks that don't require old
- * reference values to be read:
- */
- for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = transaction->updates[i];
-
- if (update->flags & REF_HAVE_OLD)
- /* Have to check the old value -> needed. */
- return 1;
-
- if ((update->flags & REF_HAVE_NEW) && !is_null_oid(&update->new_oid))
- /* Have to set a new value -> needed. */
- return 1;
- }
-
- /*
- * The transaction isn't checking any old values nor is it
- * setting any nonzero new values, so it still might be able
- * to be skipped. Now do the more expensive check: the update
- * is needed if any of the updates is a delete, and the old
- * `packed-refs` file contains a value for that reference.
- */
- ret = 0;
- for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = transaction->updates[i];
- int failure_errno;
- unsigned int type;
- struct object_id oid;
-
- if (!(update->flags & REF_HAVE_NEW))
- /*
- * This reference isn't being deleted -> not
- * needed.
- */
- continue;
-
- if (!refs_read_raw_ref(ref_store, update->refname, &oid,
- &referent, &type, &failure_errno) ||
- failure_errno != ENOENT) {
- /*
- * We have to actually delete that reference
- * -> this transaction is needed.
- */
- ret = 1;
- break;
- }
- }
-
- strbuf_release(&referent);
- return ret;
-}
-
struct packed_transaction_backend_data {
/* True iff the transaction owns the packed-refs lock. */
struct string_list updates;
diff --git a/refs/packed-backend.h b/refs/packed-backend.h
index ade3c8a5ac4..51a3b6a332a 100644
--- a/refs/packed-backend.h
+++ b/refs/packed-backend.h
@@ -17,13 +17,4 @@ struct ref_store *packed_ref_store_create(struct repository *repo,
const char *gitdir,
unsigned int store_flags);
-/*
- * Return true if `transaction` really needs to be carried out against
- * the specified packed_ref_store, or false if it can be skipped
- * (i.e., because it is an obvious NOOP). `ref_store` must be locked
- * before calling this function.
- */
-int is_packed_transaction_needed(struct ref_store *ref_store,
- struct ref_transaction *transaction);
-
#endif /* REFS_PACKED_BACKEND_H */
--
gitgitgadget
