[PATCH 2/3] refs: move is_packed_transaction_needed out of packed-backend.c

["Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>]

From: Han-Wen Nienhuys <hanwen@google.com>

It is no longer specific to the packed backend.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 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