Thread (126 messages) 126 messages, 9 authors, 2024-12-05

Re: [PATCH v6 4/6] transaction: add TRANSACTION_CREATE_EXISTS error

From: karthik nayak <hidden>
Date: 2024-10-10 21:12:40

Possibly related (same subject, not in this thread)

Bence Ferdinandy [off-list ref] writes:
transaction: add TRANSACTION_CREATE_EXISTS error
Nit: but it would be nice to have s/transaction/refs here and similarly
in other patches. Phrasing 'Documentation/SubmittingPatches':

  It is also conventional in most cases to prefix the first line with
  "area: " where the area is a filename or identifier for the general
  area of the code being modified, e.g.

    * doc: clarify distinction between sign-off and pgp-signing
    * githooks.txt: improve the intro section

So in this sense, it would be the refs area, no?
quoted hunk ↗ jump to hunk
Currently there is only one special error for transaction, for when
there is a naming conflict, all other errors are dumped under a generic
error. Add a new special error case for when the caller requests the
reference to be updated only when it does not yet exist and the
reference actually does exist.

Signed-off-by: Bence Ferdinandy <redacted>
---

Notes:
    v4: new patch
    v5: no change
    v6: no change

 refs.h                  |  4 +++-
 refs/files-backend.c    | 28 ++++++++++++++++++++--------
 refs/reftable-backend.c |  6 ++++--
 3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/refs.h b/refs.h
index f38616db84..166affbc89 100644
--- a/refs.h
+++ b/refs.h
@@ -759,8 +759,10 @@ int ref_transaction_verify(struct ref_transaction *transaction,

 /* Naming conflict (for example, the ref names A and A/B conflict). */
 #define TRANSACTION_NAME_CONFLICT -1
+/* When only creation was requested, but the ref already exists. */
+#define TRANSACTION_CREATE_EXISTS -2
 /* All other errors. */
-#define TRANSACTION_GENERIC_ERROR -2
+#define TRANSACTION_GENERIC_ERROR -3

 /*
  * Perform the preparatory stages of committing `transaction`. Acquire
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8415f2d020..272ad81315 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2502,14 +2502,18 @@ static int split_symref_update(struct ref_update *update,
 static int check_old_oid(struct ref_update *update, struct object_id *oid,
 			 struct strbuf *err)
 {
+	int ret = TRANSACTION_GENERIC_ERROR;
+
 	if (!(update->flags & REF_HAVE_OLD) ||
 		   oideq(oid, &update->old_oid))
 		return 0;

-	if (is_null_oid(&update->old_oid))
+	if (is_null_oid(&update->old_oid)) {
 		strbuf_addf(err, "cannot lock ref '%s': "
 			    "reference already exists",
 			    ref_update_original_update_refname(update));
+		ret = TRANSACTION_CREATE_EXISTS;
+	}
 	else if (is_null_oid(oid))
 		strbuf_addf(err, "cannot lock ref '%s': "
 			    "reference is missing but expected %s",
@@ -2522,7 +2526,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid,
 			    oid_to_hex(oid),
 			    oid_to_hex(&update->old_oid));

-	return -1;
+	return ret;
 }

 /*
@@ -2603,9 +2607,13 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 					ret = TRANSACTION_GENERIC_ERROR;
 					goto out;
 				}
-			} else if  (check_old_oid(update, &lock->old_oid, err)) {
-				ret = TRANSACTION_GENERIC_ERROR;
-				goto out;
+			} else {
+				int checkret;
+				checkret = check_old_oid(update, &lock->old_oid, err);
+				if  (checkret) {
+					ret = checkret;
+					goto out;
+				}
Can't we simply do:

  ret = check_old_oid(update, &lock->old_oid, err);
  if (ret) {
     goto out
  }

if ret is '0', it shouldn't matter no?

[snip]

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help