{"thread":{"id":"61588","subject":"[PATCH v4 0/7] update-ref: add symref support for --stdin","startedAt":"2024-06-05T10:30:03Z","lastAt":"2024-06-10T08:26:19Z","messageCount":22,"participants":["Karthik Nayak","Patrick Steinhardt"],"isPatch":true,"patchVersion":4,"patchTotal":7},"messages":[{"id":"496357","messageId":"20240605102958.716432-1-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 0/7] update-ref: add symref support for --stdin","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:51Z","receivedAt":"2024-06-05T10:30:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nThe 'update-ref' command is used to update refs using transactions. The\ncommand allows users to also utilize a '--stdin' mode to provide a\nbatch of sub-commands which can be processed in a transaction.\n\nCurrently, the sub-commands involve {verify, delete, create, update}\nand they allow users to work with regular refs in the repository. To\nwork with symrefs, users only have the option of using\n'git-symbolic-ref', which doesn't provide transaction support to the\nusers eventhough it uses the same behind the hood.\n\nRecently, we modified the reference backend to add symref support,\nfollowing which, 'git-symbolic-ref' also uses the transaction backend.\nBut, it doesn't expose this to the user. To allow users to work with\nsymrefs via transaction, this series adds support for new sub-commands\n{symrer-verify, symref-delete, symref-create, symref-update} to the\n'--stdin' mode of update-ref. These complement the existing\nsub-commands.\n\nThe patches 1, 2, & 6 fix small issues in the reference backends. The other\npatches 3, 4, 5, & 7 each add one of the new sub-commands.\n\nThe series is based off master, with 'kn/ref-transaction-symref' merged\nin.\n\nThere was some discussion [1] also about adding `old_target` support to\nthe existing `update` command. I think its worthwhile to do this with\nsome tests cleanup, will follow that up as a separate series.\n\nChanges since v3:\n* Changed the position of `old_target` and `flags` in `ref_transaction_delete`\nto make it a coherent.\n* Added tests for deletion of regular refs using 'symref-delete', this lead to\nadding a new commit to have specific errors for when a regular update contains\n`old_target`.\n\n[1]: https://lore.kernel.org/r/CAOLa=ZQW-cCV5BP_fCvuZimfkjwAzjEiqXYRPft1Wf9kAX=_bw@mail.gmail.com\n\nKarthik Nayak (7):\n refs: create and use `ref_update_expects_existing_old_ref()`\n refs: specify error for regular refs with `old_target`\n update-ref: add support for 'symref-verify' command\n update-ref: add support for 'symref-delete' command\n update-ref: add support for 'symref-create' command\n reftable: pick either 'oid' or 'target' for new updates\n update-ref: add support for 'symref-update' command\n\n Documentation/git-update-ref.txt | 25 ++\n builtin/clone.c | 2 +-\n builtin/fetch.c | 4 +-\n builtin/receive-pack.c | 3 +-\n builtin/update-ref.c | 237 ++++++++++++++++-\n refs.c | 40 ++-\n refs.h | 6 +-\n refs/files-backend.c | 16 +-\n refs/refs-internal.h | 6 +\n refs/reftable-backend.c | 16 +-\n t/t0600-reffiles-backend.sh | 32 +++\n t/t1400-update-ref.sh | 430 ++++++++++++++++++++++++++++++-\n t/t1416-ref-transaction-hooks.sh | 54 ++++\n t/t5605-clone-local.sh | 2 +-\n 14 files changed, 832 insertions(+), 41 deletions(-)\n\nRange-diff against v3:\n-: ---------- > 1: cab5265c3c refs: create and use `ref_update_expects_existing_old_ref()`\n-: ---------- > 2: 57b5ff46c0 refs: specify error for regular refs with `old_target`\n1: ed54b0dfb9 = 3: 5710fa81bf update-ref: add support for 'symref-verify' command\n2: b82b86ff40 ! 4: 5f8fc4eb6e update-ref: add support for 'symref-delete' command\n @@ Commit message\n within a transaction, which promises atomicity of the operation and can\n be batched with other commands.\n \n + When no 'old_target' is provided it can also delete regular refs,\n + similar to how the 'delete' command can delete symrefs when no 'old_oid'\n + is provided.\n +\n Helped-by: Patrick Steinhardt \n Signed-off-by: Karthik Nayak \n \n @@ Documentation/git-update-ref.txt: verify::\n \n ## builtin/fetch.c ##\n @@ builtin/fetch.c: static int prune_refs(struct display_state *display_state,\n + \tif (!dry_run) {\n \t\tif (transaction) {\n \t\t\tfor (ref = stale_refs; ref; ref = ref->next) {\n - \t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n +-\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n -\t\t\t\t\t\t\t\t\"fetch: prune\", &err);\n -+\t\t\t\t\t\t\t\tNULL, \"fetch: prune\", &err);\n ++\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL,\n ++\t\t\t\t\t\t\t\tNULL, 0, \"fetch: prune\", &err);\n \t\t\t\tif (result)\n \t\t\t\t\tgoto cleanup;\n \t\t\t}\n @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh\n \t\t\t\t\t namespaced_name,\n \t\t\t\t\t old_oid,\n -\t\t\t\t\t 0, \"push\", &err)) {\n -+\t\t\t\t\t 0, NULL,\n ++\t\t\t\t\t NULL, 0,\n +\t\t\t\t\t \"push\", &err)) {\n \t\t\trp_error(\"%s\", err.buf);\n \t\t\tret = \"failed to delete\";\n @@ builtin/update-ref.c: static void parse_cmd_delete(struct ref_transaction *trans\n \tif (ref_transaction_delete(transaction, refname,\n \t\t\t\t have_old ? &old_oid : NULL,\n -\t\t\t\t update_flags, msg, &err))\n -+\t\t\t\t update_flags, NULL, msg, &err))\n ++\t\t\t\t NULL, update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = default_flags;\n @@ builtin/update-ref.c: static void parse_cmd_delete(struct ref_transaction *trans\n +\t\tdie(\"symref-delete %s: extra input: %s\", refname, next);\n +\n +\tif (ref_transaction_delete(transaction, refname, NULL,\n -+\t\t\t\t update_flags, old_target, msg, &err))\n ++\t\t\t\t old_target, update_flags, msg, &err))\n +\t\tdie(\"%s\", err.buf);\n +\n +\tupdate_flags = default_flags;\n @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n -\t\t\t\t flags, msg, &err) ||\n -+\t\t\t\t flags, NULL, msg, &err) ||\n ++\t\t\t\t NULL, flags, msg, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n @@ refs.c: int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n -\t\t\t unsigned int flags, const char *msg,\n -+\t\t\t unsigned int flags,\n +\t\t\t const char *old_target,\n ++\t\t\t unsigned int flags,\n +\t\t\t const char *msg,\n \t\t\t struct strbuf *err)\n {\n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \tfor_each_string_list_item(item, refnames) {\n \t\tret = ref_transaction_delete(transaction, item->string,\n -\t\t\t\t\t NULL, flags, msg, &err);\n -+\t\t\t\t\t NULL, flags, NULL, msg, &err);\n ++\t\t\t\t\t NULL, NULL, flags, msg, &err);\n \t\tif (ret) {\n \t\t\twarning(_(\"could not delete reference %s: %s\"),\n \t\t\t\titem->string, err.buf);\n @@ refs.h: int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n -\t\t\t unsigned int flags, const char *msg,\n -+\t\t\t unsigned int flags,\n +\t\t\t const char *old_target,\n ++\t\t\t unsigned int flags,\n +\t\t\t const char *msg,\n \t\t\t struct strbuf *err);\n \n @@ t/t1400-update-ref.sh: do\n +\t\tgrep \"fatal: symref-delete: missing \" err\n +\t'\n +\n ++\ttest_expect_success \"stdin $type symref-delete fails deleting regular ref\" '\n ++\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n ++\t\tgit update-ref refs/heads/regularref $a &&\n ++\t\tformat_command $type \"symref-delete refs/heads/regularref\" \"$a\" >stdin &&\n ++\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n ++\t\tgrep \"fatal: cannot update regular ref: ${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set\" err\n ++\t'\n ++\n +\ttest_expect_success \"stdin $type symref-delete fails with too many arguments\" '\n +\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" \"$a\" >stdin &&\n +\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n @@ t/t1400-update-ref.sh: do\n +\t\tgit update-ref --stdin $type --no-deref stdin &&\n ++\t\tgit update-ref --stdin $type --no-deref ` we do not have this\n + divergence there as those commands only work with symrefs. Whereas\n + 'symref-update' also works with regular refs and allows users to convert\n + regular refs to symrefs.\n \n The command allows users to perform symbolic ref updates within a\n transaction. This provides atomicity and allows users to perform a set\n-- \n2.43.GIT\n\n"},{"id":"496358","messageId":"20240605102958.716432-2-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 1/7] refs: create and use `ref_update_expects_existing_old_ref()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:52Z","receivedAt":"2024-06-05T10:30:04Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nThe files and reftable backend, need to check if a ref must exist, so\nthat the required validation can be done. A ref must exist only when the\n`old_oid` value of the update has been explicitly set and it is not the\n`null_oid` value.\n\nSince we also support symrefs now, we need to ensure that even when\n`old_target` is set a ref must exist. While this was missed when we\nadded symref support in transactions, there are no active users of this\npath. As we introduce the 'symref-verify' command in the upcoming\ncommits, it is important to fix this.\n\nSo let's export this to a function called\n`ref_update_expects_existing_old_ref()` and expose it internally via\n'refs-internal.h'.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n refs.c | 6 ++++++\n refs/files-backend.c | 3 +--\n refs/refs-internal.h | 6 ++++++\n refs/reftable-backend.c | 2 +-\n 4 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8260c27cde..50d8d7d777 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2679,3 +2679,9 @@ int ref_update_check_old_target(const char *referent, struct ref_update *update,\n \t\t\t referent, update->old_target);\n \treturn -1;\n }\n+\n+int ref_update_expects_existing_old_ref(struct ref_update *update)\n+{\n+\treturn (update->flags & REF_HAVE_OLD) &&\n+\t\t(!is_null_oid(&update->old_oid) || update->old_target);\n+}\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5f3089d947..194e74eb4d 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2412,8 +2412,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t struct strbuf *err)\n {\n \tstruct strbuf referent = STRBUF_INIT;\n-\tint mustexist = (update->flags & REF_HAVE_OLD) &&\n-\t\t!is_null_oid(&update->old_oid);\n+\tint mustexist = ref_update_expects_existing_old_ref(update);\n \tint ret = 0;\n \tstruct ref_lock *lock;\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 53a6c5d842..ee298ec0d5 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -765,4 +765,10 @@ int ref_update_has_null_new_value(struct ref_update *update);\n int ref_update_check_old_target(const char *referent, struct ref_update *update,\n \t\t\t\tstruct strbuf *err);\n \n+/*\n+ * Check if the ref must exist, this means that the old_oid or\n+ * old_target is non NULL.\n+ */\n+int ref_update_expects_existing_old_ref(struct ref_update *update);\n+\n #endif /* REFS_REFS_INTERNAL_H */\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 1af86bbdec..b838cf8f00 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -824,7 +824,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t\t ¤t_oid, &referent, &u->type);\n \t\tif (ret < 0)\n \t\t\tgoto done;\n-\t\tif (ret > 0 && (!(u->flags & REF_HAVE_OLD) || is_null_oid(&u->old_oid))) {\n+\t\tif (ret > 0 && !ref_update_expects_existing_old_ref(u)) {\n \t\t\t/*\n \t\t\t * The reference does not exist, and we either have no\n \t\t\t * old object ID or expect the reference to not exist.\n-- \n2.43.GIT\n\n"},{"id":"496359","messageId":"20240605102958.716432-3-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 2/7] refs: specify error for regular refs with `old_target`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:53Z","receivedAt":"2024-06-05T10:30:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nWhen a regular reference update contains `old_target` set, we call the\n`ref_update_check_old_target` function to check the referent value. But\nfor regular refs we know that the referent value is not set and this\nsimply raises a generic error which says nothing about this being a\nregular ref. Instead let's raise a more specific error when a regular\nref update contains `old_target`.\n\nSigned-off-by: Karthik Nayak \n---\n refs/files-backend.c | 13 +++++++------\n refs/reftable-backend.c | 9 +++++++++\n 2 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 194e74eb4d..f516d4d82f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2491,14 +2491,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \n \t\t/*\n \t\t * Even if the ref is a regular ref, if `old_target` is set, we\n-\t\t * check the referent value. Ideally `old_target` should only\n-\t\t * be set for symrefs, but we're strict about its usage.\n+\t\t * fail with an error.\n \t\t */\n \t\tif (update->old_target) {\n-\t\t\tif (ref_update_check_old_target(referent.buf, update, err)) {\n-\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n-\t\t\t\tgoto out;\n-\t\t\t}\n+\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n+\t\t\t\t\t \"symref target '%s' set\"),\n+\t\t\t\t ref_update_original_update_refname(update),\n+\t\t\t\t update->old_target);\n+\t\t\tret = TRANSACTION_GENERIC_ERROR;\n+\t\t\tgoto out;\n \t\t} else if (check_old_oid(update, &lock->old_oid, err)) {\n \t\t\tret = TRANSACTION_GENERIC_ERROR;\n \t\t\tgoto out;\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex b838cf8f00..bd89ce8d76 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -928,6 +928,15 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t * backend returns, which keeps our tests happy.\n \t\t */\n \t\tif (u->old_target) {\n+\t\t\tif (!(u->type & REF_ISSYMREF)) {\n+\t\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n+\t\t\t\t\t \"symref target '%s' set\"),\n+\t\t\t\t\t ref_update_original_update_refname(u),\n+\t\t\t\t\t u->old_target);\n+\t\t\t\tret = -1;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\n \t\t\tif (ref_update_check_old_target(referent.buf, u, err)) {\n \t\t\t\tret = -1;\n \t\t\t\tgoto done;\n-- \n2.43.GIT\n\n"},{"id":"496360","messageId":"20240605102958.716432-4-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 3/7] update-ref: add support for 'symref-verify' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:54Z","receivedAt":"2024-06-05T10:30:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nThe 'symref-verify' command allows users to verify if a provided \ncontains the provided without changing the . If\n is not provided, the command will verify that the \ndoesn't exist.\n\nThe command allows users to verify symbolic refs within a transaction,\nand this means users can perform a set of changes in a transaction only\nwhen the verification holds good.\n\nSince we're checking for symbolic refs, this command will only work with\nthe 'no-deref' mode. This is because any dereferenced symbolic ref will\npoint to an object and not a ref and the regular 'verify' command can be\nused in such situations.\n\nAdd required tests for symref support in 'verify'. Since we're here,\nalso add reflog checks for the pre-existing 'verify' tests, there is no\ndivergence from behavior, but we never tested to ensure that reflog\nwasn't affected by the 'verify' command.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 7 +++\n builtin/update-ref.c | 80 +++++++++++++++++++++++----\n refs.c | 11 +++-\n refs.h | 1 +\n t/t1400-update-ref.sh | 94 +++++++++++++++++++++++++++++++-\n t/t1416-ref-transaction-hooks.sh | 30 ++++++++++\n 6 files changed, 208 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 374a2ebd2b..9fe78b3501 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-verify SP [SP ] LF\n \toption SP LF\n \tstart LF\n \tprepare LF\n@@ -86,6 +87,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-verify SP [NUL ] NUL\n \toption SP NUL\n \tstart NUL\n \tprepare NUL\n@@ -117,6 +119,11 @@ verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\n \n+symref-verify::\n+\tVerify symbolic against but do not change it.\n+\tIf is missing, the ref must not exist. Can only be\n+\tused in `no-deref` mode.\n+\n option::\n \tModify the behavior of the next command naming a .\n \tThe only valid option is `no-deref` to avoid dereferencing\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 6cda1c08aa..50f5472160 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -76,6 +76,29 @@ static char *parse_refname(const char **next)\n \treturn strbuf_detach(&ref, NULL);\n }\n \n+/*\n+ * Wrapper around parse_refname which skips the next delimiter.\n+ */\n+static char *parse_next_refname(const char **next)\n+{\n+\tif (line_termination) {\n+\t\t/* Without -z, consume SP and use next argument */\n+\t\tif (!**next || **next == line_termination)\n+\t\t\treturn NULL;\n+\t\tif (**next != ' ')\n+\t\t\tdie(\"expected SP but got: %s\", *next);\n+\t} else {\n+\t\t/* With -z, read the next NUL-terminated line */\n+\t\tif (**next)\n+\t\t\treturn NULL;\n+\t}\n+\t/* Skip the delimiter */\n+\t(*next)++;\n+\n+\treturn parse_refname(next);\n+}\n+\n+\n /*\n * The value being parsed is (as opposed to ; the\n * difference affects which error messages are generated):\n@@ -297,11 +320,47 @@ static void parse_cmd_verify(struct ref_transaction *transaction,\n \t\tdie(\"verify %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_verify(transaction, refname, &old_oid,\n-\t\t\t\t update_flags, &err))\n+\t\t\t\t NULL, update_flags, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tstrbuf_release(&err);\n+}\n+\n+static void parse_cmd_symref_verify(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct object_id old_oid;\n+\tchar *refname, *old_target;\n+\n+\tif (!(update_flags & REF_NO_DEREF))\n+\t\tdie(\"symref-verify: cannot operate with deref mode\");\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-verify: missing \");\n+\n+\t/*\n+\t * old_ref is optional, if not provided, we need to ensure that the\n+\t * ref doesn't exist.\n+\t */\n+\told_target = parse_next_refname(&next);\n+\tif (!old_target)\n+\t\toidcpy(&old_oid, null_oid());\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-verify %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_verify(transaction, refname,\n+\t\t\t\t old_target ? NULL : &old_oid,\n+\t\t\t\t old_target, update_flags, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = default_flags;\n \tfree(refname);\n+\tfree(old_target);\n \tstrbuf_release(&err);\n }\n \n@@ -380,15 +439,16 @@ static const struct parse_cmd {\n \tunsigned args;\n \tenum update_refs_state state;\n } command[] = {\n-\t{ \"update\", parse_cmd_update, 3, UPDATE_REFS_OPEN },\n-\t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n-\t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n-\t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n-\t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\n-\t{ \"start\", parse_cmd_start, 0, UPDATE_REFS_STARTED },\n-\t{ \"prepare\", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },\n-\t{ \"abort\", parse_cmd_abort, 0, UPDATE_REFS_CLOSED },\n-\t{ \"commit\", parse_cmd_commit, 0, UPDATE_REFS_CLOSED },\n+\t{ \"update\", parse_cmd_update, 3, UPDATE_REFS_OPEN },\n+\t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n+\t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n+\t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\n+\t{ \"start\", parse_cmd_start, 0, UPDATE_REFS_STARTED },\n+\t{ \"prepare\", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },\n+\t{ \"abort\", parse_cmd_abort, 0, UPDATE_REFS_CLOSED },\n+\t{ \"commit\", parse_cmd_commit, 0, UPDATE_REFS_CLOSED },\n };\n \n static void update_refs_stdin(void)\ndiff --git a/refs.c b/refs.c\nindex 50d8d7d777..cdc4d25557 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1297,14 +1297,19 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n int ref_transaction_verify(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n+\t\t\t const char *old_target,\n \t\t\t unsigned int flags,\n \t\t\t struct strbuf *err)\n {\n-\tif (!old_oid)\n-\t\tBUG(\"verify called with old_oid set to NULL\");\n+\tif (!old_target && !old_oid)\n+\t\tBUG(\"verify called with old_oid and old_target set to NULL\");\n+\tif (old_oid && old_target)\n+\t\tBUG(\"verify called with both old_oid and old_target set\");\n+\tif (old_target && !(flags & REF_NO_DEREF))\n+\t\tBUG(\"verify cannot operate on symrefs with deref mode\");\n \treturn ref_transaction_update(transaction, refname,\n \t\t\t\t NULL, old_oid,\n-\t\t\t\t NULL, NULL,\n+\t\t\t\t NULL, old_target,\n \t\t\t\t flags, NULL, err);\n }\n \ndiff --git a/refs.h b/refs.h\nindex 34568ee1fb..906299351c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -736,6 +736,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n int ref_transaction_verify(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n+\t\t\t const char *old_target,\n \t\t\t unsigned int flags,\n \t\t\t struct strbuf *err);\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex bbee2783ab..52801be07d 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -892,17 +892,23 @@ test_expect_success 'stdin update/create/verify combination works' '\n '\n \n test_expect_success 'stdin verify succeeds for correct value' '\n+\ttest-tool ref-store main for-each-reflog-ent $m >before &&\n \tgit rev-parse $m >expect &&\n \techo \"verify $m $m\" >stdin &&\n \tgit update-ref --stdin actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\ttest-tool ref-store main for-each-reflog-ent $m >after &&\n+\ttest_cmp before after\n '\n \n test_expect_success 'stdin verify succeeds for missing reference' '\n+\ttest-tool ref-store main for-each-reflog-ent $m >before &&\n \techo \"verify refs/heads/missing $Z\" >stdin &&\n \tgit update-ref --stdin after &&\n+\ttest_cmp before after\n '\n \n test_expect_success 'stdin verify treats no value as missing' '\n@@ -1643,4 +1649,88 @@ test_expect_success PIPE 'transaction flushes status updates' '\n \ttest_cmp expected actual\n '\n \n+format_command () {\n+\tif test \"$1\" = \"-z\"\n+\tthen\n+\t\tshift\n+\t\tprintf \"$F\" \"$@\"\n+\telse\n+\t\techo \"$@\"\n+\tfi\n+}\n+\n+for type in \"\" \"-z\"\n+do\n+\n+\ttest_expect_success \"stdin $type symref-verify fails without --no-deref\" '\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: symref-verify: cannot operate with deref mode\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails with too many arguments\" '\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-verify refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify succeeds for correct value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >before &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >after &&\n+\t\ttest_cmp before after\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails with no value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref before &&\n+\t\tformat_command $type \"symref-verify refs/heads/missing\" \"refs/heads/unknown\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/missing${SQ}: unable to resolve reference ${SQ}refs/heads/missing${SQ}\" err &&\n+\t\ttest_must_fail git rev-parse --verify -q refs/heads/missing &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >after &&\n+\t\ttest_cmp before after\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails for wrong value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$b\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails for mistaken null value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$Z\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+done\n+\n test_done\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex 067fd57290..fd58b902f4 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -157,4 +157,34 @@ test_expect_success 'hook captures git-symbolic-ref updates' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'hook gets all queued symref updates' '\n+\ttest_when_finished \"rm actual\" &&\n+\n+\tgit update-ref refs/heads/branch $POST_OID &&\n+\tgit symbolic-ref refs/heads/symref refs/heads/main &&\n+\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\techo \"$*\" >>actual\n+\twhile read -r line\n+\tdo\n+\t\tprintf \"%s\\n\" \"$line\"\n+\tdone >>actual\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tprepared\n+\tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tcommitted\n+\tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tEOF\n+\n+\tgit update-ref --no-deref --stdin <<-EOF &&\n+\tstart\n+\tsymref-verify refs/heads/symref refs/heads/main\n+\tprepare\n+\tcommit\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.43.GIT\n\n"},{"id":"496361","messageId":"20240605102958.716432-5-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 4/7] update-ref: add support for 'symref-delete' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:55Z","receivedAt":"2024-06-05T10:30:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd a new command 'symref-delete' to allow deletions of symbolic refs in\na transaction via the '--stdin' mode of the 'git-update-ref' command.\nThe 'symref-delete' command can, when given an , delete the\nprovided only when it points to .\n\nThis command is only compatible with the 'no-deref' mode because we\noptionally want to check the 'old_target' of the ref being deleted.\nDe-referencing a symbolic ref would provide a regular ref and we already\nhave the 'delete' command for regular refs.\n\nWhile users can also use 'git symbolic-ref -d' to delete symbolic refs,\nthe 'symref-delete' command in 'git-update-ref' allows users to do so\nwithin a transaction, which promises atomicity of the operation and can\nbe batched with other commands.\n\nWhen no 'old_target' is provided it can also delete regular refs,\nsimilar to how the 'delete' command can delete symrefs when no 'old_oid'\nis provided.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 5 +++\n builtin/fetch.c | 4 +-\n builtin/receive-pack.c | 3 +-\n builtin/update-ref.c | 33 +++++++++++++++-\n refs.c | 14 +++++--\n refs.h | 4 +-\n t/t1400-update-ref.sh | 68 ++++++++++++++++++++++++++++++++\n t/t1416-ref-transaction-hooks.sh | 19 ++++++++-\n 8 files changed, 140 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 9fe78b3501..16e02f6979 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-delete SP [SP ] LF\n \tsymref-verify SP [SP ] LF\n \toption SP LF\n \tstart LF\n@@ -87,6 +88,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-delete SP [NUL ] NUL\n \tsymref-verify SP [NUL ] NUL\n \toption SP NUL\n \tstart NUL\n@@ -119,6 +121,9 @@ verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\n \n+symref-delete::\n+\tDelete after verifying it exists with , if given.\n+\n symref-verify::\n \tVerify symbolic against but do not change it.\n \tIf is missing, the ref must not exist. Can only be\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 75255dc600..d63100e0d3 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1386,8 +1386,8 @@ static int prune_refs(struct display_state *display_state,\n \tif (!dry_run) {\n \t\tif (transaction) {\n \t\t\tfor (ref = stale_refs; ref; ref = ref->next) {\n-\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n-\t\t\t\t\t\t\t\t\"fetch: prune\", &err);\n+\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL,\n+\t\t\t\t\t\t\t\tNULL, 0, \"fetch: prune\", &err);\n \t\t\t\tif (result)\n \t\t\t\t\tgoto cleanup;\n \t\t\t}\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 01c1f04ece..0a30fac239 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1576,7 +1576,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\tif (ref_transaction_delete(transaction,\n \t\t\t\t\t namespaced_name,\n \t\t\t\t\t old_oid,\n-\t\t\t\t\t 0, \"push\", &err)) {\n+\t\t\t\t\t NULL, 0,\n+\t\t\t\t\t \"push\", &err)) {\n \t\t\trp_error(\"%s\", err.buf);\n \t\t\tret = \"failed to delete\";\n \t\t} else {\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 50f5472160..0cb7eef3c6 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -293,7 +293,7 @@ static void parse_cmd_delete(struct ref_transaction *transaction,\n \n \tif (ref_transaction_delete(transaction, refname,\n \t\t\t\t have_old ? &old_oid : NULL,\n-\t\t\t\t update_flags, msg, &err))\n+\t\t\t\t NULL, update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = default_flags;\n@@ -301,6 +301,36 @@ static void parse_cmd_delete(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+\n+static void parse_cmd_symref_delete(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tchar *refname, *old_target;\n+\n+\tif (!(update_flags & REF_NO_DEREF))\n+\t\tdie(\"symref-delete: cannot operate with deref mode\");\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-delete: missing \");\n+\n+\told_target = parse_next_refname(&next);\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-delete %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_delete(transaction, refname, NULL,\n+\t\t\t\t old_target, update_flags, msg, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tfree(old_target);\n+\tstrbuf_release(&err);\n+}\n+\n+\n static void parse_cmd_verify(struct ref_transaction *transaction,\n \t\t\t const char *next, const char *end)\n {\n@@ -443,6 +473,7 @@ static const struct parse_cmd {\n \t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n \t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-delete\", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\n \t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\n \t{ \"start\", parse_cmd_start, 0, UPDATE_REFS_STARTED },\ndiff --git a/refs.c b/refs.c\nindex cdc4d25557..01f3188a09 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -950,7 +950,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \ttransaction = ref_store_transaction_begin(refs, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n-\t\t\t\t flags, msg, &err) ||\n+\t\t\t\t NULL, flags, msg, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n@@ -1283,14 +1283,20 @@ int ref_transaction_create(struct ref_transaction *transaction,\n int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n-\t\t\t unsigned int flags, const char *msg,\n+\t\t\t const char *old_target,\n+\t\t\t unsigned int flags,\n+\t\t\t const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tif (old_oid && is_null_oid(old_oid))\n \t\tBUG(\"delete called with old_oid set to zeros\");\n+\tif (old_oid && old_target)\n+\t\tBUG(\"delete called with both old_oid and old_target set\");\n+\tif (old_target && !(flags & REF_NO_DEREF))\n+\t\tBUG(\"delete cannot operate on symrefs with deref mode\");\n \treturn ref_transaction_update(transaction, refname,\n \t\t\t\t null_oid(), old_oid,\n-\t\t\t\t NULL, NULL, flags,\n+\t\t\t\t NULL, old_target, flags,\n \t\t\t\t msg, err);\n }\n \n@@ -2599,7 +2605,7 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \n \tfor_each_string_list_item(item, refnames) {\n \t\tret = ref_transaction_delete(transaction, item->string,\n-\t\t\t\t\t NULL, flags, msg, &err);\n+\t\t\t\t\t NULL, NULL, flags, msg, &err);\n \t\tif (ret) {\n \t\t\twarning(_(\"could not delete reference %s: %s\"),\n \t\t\t\titem->string, err.buf);\ndiff --git a/refs.h b/refs.h\nindex 906299351c..974cf4dd08 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -722,7 +722,9 @@ int ref_transaction_create(struct ref_transaction *transaction,\n int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n-\t\t\t unsigned int flags, const char *msg,\n+\t\t\t const char *old_target,\n+\t\t\t unsigned int flags,\n+\t\t\t const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 52801be07d..45111ce021 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1731,6 +1731,74 @@ do\n \t\ttest_cmp expect actual\n \t'\n \n+\ttest_expect_success \"stdin $type symref-delete fails without --no-deref\" '\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: symref-delete: cannot operate with deref mode\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails with no ref\" '\n+\t\tformat_command $type \"symref-delete \" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: symref-delete: missing \" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails deleting regular ref\" '\n+\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n+\t\tgit update-ref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-delete refs/heads/regularref\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: cannot update regular ref: ${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails with too many arguments\" '\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-delete refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails with wrong old value\" '\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$m\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected refs/heads/main\" err &&\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete works with right old value\" '\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref >actual\n@@ -171,16 +172,32 @@ test_expect_success 'hook gets all queued symref updates' '\n \tdone >>actual\n \tEOF\n \n-\tcat >expect <<-EOF &&\n+\t# In the files backend, \"delete\" also triggers an additional transaction\n+\t# update on the packed-refs backend, which constitutes additional reflog\n+\t# entries.\n+\tif test_have_prereq REFFILES\n+\tthen\n+\t\tcat >expect <<-EOF\n+\t\taborted\n+\t\t$ZERO_OID $ZERO_OID refs/heads/symrefd\n+\t\tEOF\n+\telse\n+\t\t>expect\n+\tfi &&\n+\n+\tcat >>expect <<-EOF &&\n \tprepared\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \tcommitted\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \tEOF\n \n \tgit update-ref --no-deref --stdin <<-EOF &&\n \tstart\n \tsymref-verify refs/heads/symref refs/heads/main\n+\tsymref-delete refs/heads/symrefd refs/heads/main\n \tprepare\n \tcommit\n \tEOF\n-- \n2.43.GIT\n\n"},{"id":"496362","messageId":"20240605102958.716432-6-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 5/7] update-ref: add support for 'symref-create' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:56Z","receivedAt":"2024-06-05T10:30:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd 'symref-create' command to the '--stdin' mode 'git-update-ref' to\nallow creation of symbolic refs in a transaction. The 'symref-create'\ncommand takes in a , which the created will point to.\n\nAlso, support the 'core.prefersymlinkrefs' config, wherein if the config\nis set and the filesystem supports symlinks, we create the symbolic ref\nas a symlink. We fallback to creating a regular symref if creating the\nsymlink is unsuccessful.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 6 +++\n builtin/clone.c | 2 +-\n builtin/update-ref.c | 32 +++++++++++++++-\n refs.c | 9 +++--\n refs.h | 1 +\n t/t0600-reffiles-backend.sh | 32 ++++++++++++++++\n t/t1400-update-ref.sh | 65 ++++++++++++++++++++++++++++++++\n t/t1416-ref-transaction-hooks.sh | 3 ++\n t/t5605-clone-local.sh | 2 +-\n 9 files changed, 146 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 16e02f6979..364ef78af1 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-create SP SP LF\n \tsymref-delete SP [SP ] LF\n \tsymref-verify SP [SP ] LF\n \toption SP LF\n@@ -88,6 +89,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-create SP NUL NUL\n \tsymref-delete SP [NUL ] NUL\n \tsymref-verify SP [NUL ] NUL\n \toption SP NUL\n@@ -121,6 +123,10 @@ verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\n \n+symref-create:\n+\tCreate symbolic ref with after verifying\n+\tit does not exist.\n+\n symref-delete::\n \tDelete after verifying it exists with , if given.\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 23993b905b..6ddb3084e6 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -576,7 +576,7 @@ static void write_remote_refs(const struct ref *local_refs)\n \t\tif (!r->peer_ref)\n \t\t\tcontinue;\n \t\tif (ref_transaction_create(t, r->peer_ref->name, &r->old_oid,\n-\t\t\t\t\t 0, NULL, &err))\n+\t\t\t\t\t NULL, 0, NULL, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t}\n \ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 0cb7eef3c6..9c40e94626 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -257,7 +257,7 @@ static void parse_cmd_create(struct ref_transaction *transaction,\n \tif (*next != line_termination)\n \t\tdie(\"create %s: extra input: %s\", refname, next);\n \n-\tif (ref_transaction_create(transaction, refname, &new_oid,\n+\tif (ref_transaction_create(transaction, refname, &new_oid, NULL,\n \t\t\t\t update_flags | create_reflog_flag,\n \t\t\t\t msg, &err))\n \t\tdie(\"%s\", err.buf);\n@@ -267,6 +267,35 @@ static void parse_cmd_create(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+\n+static void parse_cmd_symref_create(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tchar *refname, *new_target;\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-create: missing \");\n+\n+\tnew_target = parse_next_refname(&next);\n+\tif (!new_target)\n+\t\tdie(\"symref-create %s: missing \", refname);\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-create %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_create(transaction, refname, NULL, new_target,\n+\t\t\t\t update_flags | create_reflog_flag,\n+\t\t\t\t msg, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tfree(new_target);\n+\tstrbuf_release(&err);\n+}\n+\n static void parse_cmd_delete(struct ref_transaction *transaction,\n \t\t\t const char *next, const char *end)\n {\n@@ -473,6 +502,7 @@ static const struct parse_cmd {\n \t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n \t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-create\", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-delete\", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\n \t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\ndiff --git a/refs.c b/refs.c\nindex 01f3188a09..8807e87e3c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1268,15 +1268,18 @@ int ref_transaction_update(struct ref_transaction *transaction,\n int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *new_oid,\n+\t\t\t const char *new_target,\n \t\t\t unsigned int flags, const char *msg,\n \t\t\t struct strbuf *err)\n {\n-\tif (!new_oid || is_null_oid(new_oid)) {\n-\t\tstrbuf_addf(err, \"'%s' has a null OID\", refname);\n+\tif (new_oid && new_target)\n+\t\tBUG(\"create called with both new_oid and new_target set\");\n+\tif ((!new_oid || is_null_oid(new_oid)) && !new_target) {\n+\t\tstrbuf_addf(err, \"'%s' has neither a valid OID nor a target\", refname);\n \t\treturn 1;\n \t}\n \treturn ref_transaction_update(transaction, refname, new_oid,\n-\t\t\t\t null_oid(), NULL, NULL, flags,\n+\t\t\t\t null_oid(), new_target, NULL, flags,\n \t\t\t\t msg, err);\n }\n \ndiff --git a/refs.h b/refs.h\nindex 974cf4dd08..28e3bb8a42 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -708,6 +708,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *new_oid,\n+\t\t\t const char *new_target,\n \t\t\t unsigned int flags, const char *msg,\n \t\t\t struct strbuf *err);\n \ndiff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\nindex 92f570313d..b2a771ff2b 100755\n--- a/t/t0600-reffiles-backend.sh\n+++ b/t/t0600-reffiles-backend.sh\n@@ -468,4 +468,36 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '\n \tesac\n '\n \n+test_expect_success SYMLINKS 'symref transaction supports symlinks' '\n+\ttest_when_finished \"git symbolic-ref -d TEST_SYMREF_HEAD\" &&\n+\tgit update-ref refs/heads/new @ &&\n+\ttest_config core.prefersymlinkrefs true &&\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tsymref-create TEST_SYMREF_HEAD refs/heads/new\n+\tprepare\n+\tcommit\n+\tEOF\n+\tgit update-ref --no-deref --stdin stdin <<-EOF &&\n+\tstart\n+\tsymref-create TEST_SYMREF_HEAD refs/heads/new\n+\tprepare\n+\tcommit\n+\tEOF\n+\tgit update-ref --no-deref --stdin actual &&\n+\techo refs/heads/new >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 45111ce021..27ac6ee7cb 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1799,6 +1799,71 @@ do\n \t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-create refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create fails with no target\" '\n+\t\tformat_command $type \"symref-create refs/heads/symref\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create works with --no-deref\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-create refs/heads/symref\" \"$a\" &&\n+\t\tgit update-ref --stdin $type err\n+\t'\n+\n+\ttest_expect_success \"stdin $type create dangling symref ref works\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-create refs/heads/symref\" \"refs/heads/unkown\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\techo refs/heads/unkown >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create does not create reflogs by default\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/symref\" &&\n+\t\tformat_command $type \"symref-create refs/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest_must_fail git reflog exists refs/symref\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create reflogs with --create-reflog\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-create refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --create-reflog --stdin $type --no-deref expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\tgit reflog exists refs/heads/symref\n+\t'\n+\n done\n \n test_done\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex ccde1b944b..ff77dcca6b 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -189,15 +189,18 @@ test_expect_success 'hook gets all queued symref updates' '\n \tprepared\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n+\t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n \tcommitted\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n+\t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n \tEOF\n \n \tgit update-ref --no-deref --stdin <<-EOF &&\n \tstart\n \tsymref-verify refs/heads/symref refs/heads/main\n \tsymref-delete refs/heads/symrefd refs/heads/main\n+\tsymref-create refs/heads/symrefc refs/heads/main\n \tprepare\n \tcommit\n \tEOF\ndiff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh\nindex a3055869bc..339d8c786f 100755\n--- a/t/t5605-clone-local.sh\n+++ b/t/t5605-clone-local.sh\n@@ -163,7 +163,7 @@ test_expect_success REFFILES 'local clone from repo with corrupt refs fails grac\n \techo a >corrupt/.git/refs/heads/topic &&\n \n \ttest_must_fail git clone corrupt working 2>err &&\n-\tgrep \"has a null OID\" err\n+\tgrep \"has neither a valid OID nor a target\" err\n '\n \n test_done\n-- \n2.43.GIT\n\n"},{"id":"496363","messageId":"20240605102958.716432-7-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 6/7] reftable: pick either 'oid' or 'target' for new updates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:57Z","receivedAt":"2024-06-05T10:30:09Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nWhen creating a reference transaction update, we can provide the old/new\noid/target for the update. We have checks in place to ensure that for\neach old/new, either oid or target is set and not both.\n\nIn the reftable backend, when dealing with updates without the\n`REF_NO_DEREF` flag, we don't selectively propagate data as needed.\nSince there are no active users of the path, this is not caught. As we\nwant to introduce the 'symref-update' command in the upcoming commit,\nwhich would use this flow, correct it.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n refs/reftable-backend.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex bd89ce8d76..fee7a9904b 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -895,8 +895,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t */\n \t\t\t\tnew_update = ref_transaction_add_update(\n \t\t\t\t\ttransaction, referent.buf, new_flags,\n-\t\t\t\t\t&u->new_oid, &u->old_oid, u->new_target,\n-\t\t\t\t\tu->old_target, u->msg);\n+\t\t\t\t\tu->new_target ? NULL : &u->new_oid,\n+\t\t\t\t\tu->old_target ? NULL : &u->old_oid,\n+\t\t\t\t\tu->new_target, u->old_target, u->msg);\n \n \t\t\t\tnew_update->parent_update = u;\n \n-- \n2.43.GIT\n\n"},{"id":"496364","messageId":"20240605102958.716432-8-knayak@gitlab.com","threadId":"61588","inReplyTo":"https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com","subject":"[PATCH v4 7/7] update-ref: add support for 'symref-update' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-05T10:29:58Z","receivedAt":"2024-06-05T10:30:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd 'symref-update' command to the '--stdin' mode of 'git-update-ref' to\nallow updates of symbolic refs. The 'symref-update' command takes in a\n, which the will be updated to. If the doesn't\nexist it will be created.\n\nIt also optionally takes either an `ref ` or `oid\n`. If the is provided, it checks to see if the\n targets the before the update. If is provided\nit checks to ensure that it is a regular ref and is the\nOID before the update. This by extension also means that this when a\nzero is provided, it ensures that the ref didn't exist before.\n\nThe divergence in syntax from the regular `update` command is because if\nwe don't use a `(ref | oid)` prefix for the old_value, then there is\nambiguity around if the value provided should be treated as an oid or a\nreference. This is more so the reason, because we allow anything\ncommittish to be provided as an oid. While 'symref-verify' and\n'symref-delete' also take in `` we do not have this\ndivergence there as those commands only work with symrefs. Whereas\n'symref-update' also works with regular refs and allows users to convert\nregular refs to symrefs.\n\nThe command allows users to perform symbolic ref updates within a\ntransaction. This provides atomicity and allows users to perform a set\nof operations together.\n\nThis command supports deref mode, to ensure that we can update\ndereferenced regular refs to symrefs.\n\nHelped-by: Patrick Steinhardt \nHelped-by: Junio C Hamano \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 7 ++\n builtin/update-ref.c | 92 ++++++++++++++\n t/t1400-update-ref.sh | 203 +++++++++++++++++++++++++++++++\n t/t1416-ref-transaction-hooks.sh | 4 +\n 4 files changed, 306 insertions(+)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 364ef78af1..afcf33cf60 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-update SP SP [SP (ref SP | oid SP )] LF\n \tsymref-create SP SP LF\n \tsymref-delete SP [SP ] LF\n \tsymref-verify SP [SP ] LF\n@@ -89,6 +90,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-update SP NUL [NUL (ref NUL | oid NUL )] NUL\n \tsymref-create SP NUL NUL\n \tsymref-delete SP [NUL ] NUL\n \tsymref-verify SP [NUL ] NUL\n@@ -119,6 +121,11 @@ delete::\n \tDelete after verifying it exists with , if\n \tgiven. If given, may not be zero.\n \n+symref-update::\n+\tSet to after verifying or ,\n+\tif given. Specify a zero to ensure that the ref does not\n+\texist before the update.\n+\n verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 9c40e94626..471fa5c8d1 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -98,6 +98,42 @@ static char *parse_next_refname(const char **next)\n \treturn parse_refname(next);\n }\n \n+/*\n+ * Wrapper around parse_arg which skips the next delimiter.\n+ */\n+static char *parse_next_arg(const char **next)\n+{\n+\tstruct strbuf arg = STRBUF_INIT;\n+\n+\tif (line_termination) {\n+\t\t/* Without -z, consume SP and use next argument */\n+\t\tif (!**next || **next == line_termination)\n+\t\t\treturn NULL;\n+\t\tif (**next != ' ')\n+\t\t\tdie(\"expected SP but got: %s\", *next);\n+\t} else {\n+\t\t/* With -z, read the next NUL-terminated line */\n+\t\tif (**next)\n+\t\t\treturn NULL;\n+\t}\n+\t/* Skip the delimiter */\n+\t(*next)++;\n+\n+\tif (line_termination) {\n+\t\t/* Without -z, use the next argument */\n+\t\t*next = parse_arg(*next, &arg);\n+\t} else {\n+\t\t/* With -z, use everything up to the next NUL */\n+\t\tstrbuf_addstr(&arg, *next);\n+\t\t*next += arg.len;\n+\t}\n+\n+\tif (arg.len)\n+\t\treturn strbuf_detach(&arg, NULL);\n+\n+\tstrbuf_release(&arg);\n+\treturn NULL;\n+}\n \n /*\n * The value being parsed is (as opposed to ; the\n@@ -237,6 +273,61 @@ static void parse_cmd_update(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+static void parse_cmd_symref_update(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tchar *refname, *new_target, *old_arg;\n+\tchar *old_target = NULL;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct object_id old_oid;\n+\tint have_old_oid = 0;\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-update: missing \");\n+\n+\tnew_target = parse_next_refname(&next);\n+\tif (!new_target)\n+\t\tdie(\"symref-update %s: missing \", refname);\n+\n+\told_arg = parse_next_arg(&next);\n+\tif (old_arg) {\n+\t\told_target = parse_next_arg(&next);\n+\t\tif (!old_target)\n+\t\t\tdie(\"symref-update %s: expected old value\", refname);\n+\n+\t\tif (!strcmp(old_arg, \"oid\")) {\n+\t\t\tif (repo_get_oid(the_repository, old_target, &old_oid))\n+\t\t\t\tdie(\"symref-update %s: invalid oid: %s\", refname, old_target);\n+\n+\t\t\thave_old_oid = 1;\n+\t\t} else if (!strcmp(old_arg, \"ref\")) {\n+\t\t\tif (check_refname_format(old_target, REFNAME_ALLOW_ONELEVEL))\n+\t\t\t\tdie(\"symref-update %s: invalid ref: %s\", refname, old_target);\n+\t\t} else {\n+\t\t\tdie(\"symref-update %s: invalid arg '%s' for old value\", refname, old_arg);\n+\t\t}\n+\t}\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-update %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_update(transaction, refname, NULL,\n+\t\t\t\t have_old_oid ? &old_oid : NULL,\n+\t\t\t\t new_target,\n+\t\t\t\t have_old_oid ? NULL : old_target,\n+\t\t\t\t update_flags | create_reflog_flag,\n+\t\t\t\t msg, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tfree(old_arg);\n+\tfree(old_target);\n+\tfree(new_target);\n+\tstrbuf_release(&err);\n+}\n+\n static void parse_cmd_create(struct ref_transaction *transaction,\n \t\t\t const char *next, const char *end)\n {\n@@ -502,6 +593,7 @@ static const struct parse_cmd {\n \t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n \t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-update\", parse_cmd_symref_update, 4, UPDATE_REFS_OPEN },\n \t{ \"symref-create\", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-delete\", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 27ac6ee7cb..9bda06d2dc 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1362,6 +1362,7 @@ test_expect_success 'fails with duplicate HEAD update' '\n '\n \n test_expect_success 'fails with duplicate ref update via symref' '\n+\ttest_when_finished \"git symbolic-ref -d refs/heads/symref2\" &&\n \tgit branch target2 $A &&\n \tgit symbolic-ref refs/heads/symref2 refs/heads/target2 &&\n \tcat >stdin <<-EOF &&\n@@ -1864,6 +1865,208 @@ do\n \t\tgit reflog exists refs/heads/symref\n \t'\n \n+\ttest_expect_success \"stdin $type symref-update fails with too many arguments\" '\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"ref\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-update refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails with wrong old value argument\" '\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"foo\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: symref-update refs/heads/symref: invalid arg ${SQ}foo${SQ} for old value\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates with zero old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"oid\" \"$Z\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates with no old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates dangling\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"refs/heads/nonexistent\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails with wrong old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" \"ref\" \"$b\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected $b\" err &&\n+\t\ttest_must_fail git rev-parse --verify -q $c\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update updates dangling ref\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update updates dangling ref with old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"ref\" \"refs/heads/nonexistent\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails update dangling ref with wrong old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"ref\" \"refs/heads/wrongref\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update works with right old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" \"ref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update works with no old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails with empty old ref-target\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" \"ref\" \"\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates (with deref)\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&\n+\t\tgrep \"$Z $(git rev-parse $a)\" actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref with correct old-oid\" '\n+\t\ttest_when_finished \"git symbolic-ref -d --no-recurse refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" \"oid\" \"$(git rev-parse $a)\" >stdin &&\n+\t\tgit update-ref --stdin $type expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&\n+\t\tgrep \"$(git rev-parse $a) $(git rev-parse $a)\" actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref fails with wrong old-oid\" '\n+\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" \"oid\" \"$(git rev-parse refs/heads/target2)\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/regularref${SQ}: is at $(git rev-parse $a) but expected $(git rev-parse refs/heads/target2)\" err &&\n+\t\techo $(git rev-parse $a) >expect &&\n+\t\tgit rev-parse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref fails with invalid old-oid\" '\n+\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" \"oid\" \"not-a-ref-oid\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: symref-update refs/heads/regularref: invalid oid: not-a-ref-oid\" err &&\n+\t\techo $(git rev-parse $a) >expect &&\n+\t\tgit rev-parse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update existing symref with zero old-oid\" '\n+\t\ttest_when_finished \"git symbolic-ref -d --no-recurse refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/target2 &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"oid\" \"$Z\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/symref${SQ}: reference already exists\" err &&\n+\t\techo refs/heads/target2 >expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref (with deref)\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_when_finished \"git update-ref -d --no-deref refs/heads/symref2\" &&\n+\t\tgit update-ref refs/heads/symref2 $a &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref refs/heads/symref2 &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref $type --stdin expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref2 >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\techo refs/heads/symref2 >expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&\n+\t\tgrep \"$(git rev-parse $a) $(git rev-parse $a)\" actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref\" '\n+\t\ttest_when_finished \"git symbolic-ref -d --no-recurse refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" >stdin &&\n+\t\tgit update-ref $type --stdin expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&\n+\t\tgrep \"$(git rev-parse $a) $(git rev-parse $a)\" actual\n+\t'\n+\n done\n \n test_done\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex ff77dcca6b..5a812ca3c0 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -163,6 +163,7 @@ test_expect_success 'hook gets all queued symref updates' '\n \tgit update-ref refs/heads/branch $POST_OID &&\n \tgit symbolic-ref refs/heads/symref refs/heads/main &&\n \tgit symbolic-ref refs/heads/symrefd refs/heads/main &&\n+\tgit symbolic-ref refs/heads/symrefu refs/heads/main &&\n \n \ttest_hook reference-transaction <<-\\EOF &&\n \techo \"$*\" >>actual\n@@ -190,10 +191,12 @@ test_expect_success 'hook gets all queued symref updates' '\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n+\tref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu\n \tcommitted\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n+\tref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu\n \tEOF\n \n \tgit update-ref --no-deref --stdin <<-EOF &&\n@@ -201,6 +204,7 @@ test_expect_success 'hook gets all queued symref updates' '\n \tsymref-verify refs/heads/symref refs/heads/main\n \tsymref-delete refs/heads/symrefd refs/heads/main\n \tsymref-create refs/heads/symrefc refs/heads/main\n+\tsymref-update refs/heads/symrefu refs/heads/branch ref refs/heads/main\n \tprepare\n \tcommit\n \tEOF\n-- \n2.43.GIT\n\n"},{"id":"496462","messageId":"CAOLa=ZSkBj5V1sqW6pyraqMJOw4QUntFgo660NFoTKRTwrsvhA@mail.gmail.com","threadId":"61588","inReplyTo":"20240605102958.716432-1-knayak@gitlab.com","subject":"Re: [PATCH v4 0/7] update-ref: add symref support for --stdin","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-06T08:22:41Z","receivedAt":"2024-06-06T08:22:43Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Karthik Nayak writes:\n\n> From: Karthik Nayak \n>\n> The 'update-ref' command is used to update refs using transactions. The\n> command allows users to also utilize a '--stdin' mode to provide a\n> batch of sub-commands which can be processed in a transaction.\n>\n> Currently, the sub-commands involve {verify, delete, create, update}\n> and they allow users to work with regular refs in the repository. To\n> work with symrefs, users only have the option of using\n> 'git-symbolic-ref', which doesn't provide transaction support to the\n> users eventhough it uses the same behind the hood.\n>\n> Recently, we modified the reference backend to add symref support,\n> following which, 'git-symbolic-ref' also uses the transaction backend.\n> But, it doesn't expose this to the user. To allow users to work with\n> symrefs via transaction, this series adds support for new sub-commands\n> {symrer-verify, symref-delete, symref-create, symref-update} to the\n> '--stdin' mode of update-ref. These complement the existing\n> sub-commands.\n>\n> The patches 1, 2, & 6 fix small issues in the reference backends. The other\n> patches 3, 4, 5, & 7 each add one of the new sub-commands.\n>\n> The series is based off master, with 'kn/ref-transaction-symref' merged\n> in.\n>\n> There was some discussion [1] also about adding `old_target` support to\n> the existing `update` command. I think its worthwhile to do this with\n> some tests cleanup, will follow that up as a separate series.\n>\n> Changes since v3:\n> * Changed the position of `old_target` and `flags` in `ref_transaction_delete`\n> to make it a coherent.\n> * Added tests for deletion of regular refs using 'symref-delete', this lead to\n> adding a new commit to have specific errors for when a regular update contains\n> `old_target`.\n>\n> [1]: https://lore.kernel.org/r/CAOLa=ZQW-cCV5BP_fCvuZimfkjwAzjEiqXYRPft1Wf9kAX=_bw@mail.gmail.com\n>\n\nSomehow I added the wrong link in the reply-to and this has come out as\na thread of its own. This is a follow up of v3:\n\nhttps://lore.kernel.org/all/20240530120940.456817-1-knayak@gitlab.com/#t\n\n[snip]\n"},{"id":"496513","messageId":"ZmGXREp9Xa6qHpMC@tanuki","threadId":"61588","inReplyTo":"20240605102958.716432-3-knayak@gitlab.com","subject":"Re: [PATCH v4 2/7] refs: specify error for regular refs with `old_target`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-06T11:02:28Z","receivedAt":"2024-06-06T11:02:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jun 05, 2024 at 12:29:53PM +0200, Karthik Nayak wrote:\n> From: Karthik Nayak \n> \n> When a regular reference update contains `old_target` set, we call the\n\ns/contains/has its/\n\n> `ref_update_check_old_target` function to check the referent value. But\n> for regular refs we know that the referent value is not set and this\n\nThis is fairly technical and focussed on the implementation. Can we\nmaybe talk more about intent (\"expected a symref, but is a direct ref\")\nrather than the exact implementation to make the commit message a bit\neasier to understand for folks?\n\n> simply raises a generic error which says nothing about this being a\n> regular ref. Instead let's raise a more specific error when a regular\n> ref update contains `old_target`.\n\nIt might be helpful to include before/after in this commit message to\nshow the change. Even better would be a test of course, but I think we\ncannot add one at this point in time yet.\n\n> Signed-off-by: Karthik Nayak \n> ---\n> refs/files-backend.c | 13 +++++++------\n> refs/reftable-backend.c | 9 +++++++++\n> 2 files changed, 16 insertions(+), 6 deletions(-)\n> \n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 194e74eb4d..f516d4d82f 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2491,14 +2491,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n> \n> \t\t/*\n> \t\t * Even if the ref is a regular ref, if `old_target` is set, we\n> -\t\t * check the referent value. Ideally `old_target` should only\n> -\t\t * be set for symrefs, but we're strict about its usage.\n> +\t\t * fail with an error.\n> \t\t */\n> \t\tif (update->old_target) {\n> -\t\t\tif (ref_update_check_old_target(referent.buf, update, err)) {\n> -\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n> -\t\t\t\tgoto out;\n> -\t\t\t}\n> +\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n> +\t\t\t\t\t \"symref target '%s' set\"),\n> +\t\t\t\t ref_update_original_update_refname(update),\n> +\t\t\t\t update->old_target);\n> +\t\t\tret = TRANSACTION_GENERIC_ERROR;\n> +\t\t\tgoto out;\n\nI feel like these error messages are somewhat technical. If I were\nreading it as a user, I don't think I'd understand what it is trying to\ntell me. How about:\n\n strbuf_addf(err, _(\"cannot lock ref '%s': %s\",\n \"expected symref but is a direct ref\"));\n\nThis also matches the other error messages we have in this function more\nclosely. The same is true for the equivalent in the reftable backend.\n\nPatrick\n"},{"id":"496514","messageId":"CAOLa=ZR_EQfrjK90=zebOiiMUCAdEzPe9+SxHVoFrfkq4A+9jg@mail.gmail.com","threadId":"61588","inReplyTo":"20240605102958.716432-1-knayak@gitlab.com","subject":"Re: [PATCH v4 0/7] update-ref: add symref support for --stdin","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-06T11:03:13Z","receivedAt":"2024-06-06T11:03:14Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Karthik Nayak writes:\n\n> From: Karthik Nayak \n>\n> The 'update-ref' command is used to update refs using transactions. The\n> command allows users to also utilize a '--stdin' mode to provide a\n> batch of sub-commands which can be processed in a transaction.\n>\n> Currently, the sub-commands involve {verify, delete, create, update}\n> and they allow users to work with regular refs in the repository. To\n> work with symrefs, users only have the option of using\n> 'git-symbolic-ref', which doesn't provide transaction support to the\n> users eventhough it uses the same behind the hood.\n>\n> Recently, we modified the reference backend to add symref support,\n> following which, 'git-symbolic-ref' also uses the transaction backend.\n> But, it doesn't expose this to the user. To allow users to work with\n> symrefs via transaction, this series adds support for new sub-commands\n> {symrer-verify, symref-delete, symref-create, symref-update} to the\n> '--stdin' mode of update-ref. These complement the existing\n> sub-commands.\n>\n> The patches 1, 2, & 6 fix small issues in the reference backends. The other\n> patches 3, 4, 5, & 7 each add one of the new sub-commands.\n>\n> The series is based off master, with 'kn/ref-transaction-symref' merged\n> in.\n>\n> There was some discussion [1] also about adding `old_target` support to\n> the existing `update` command. I think its worthwhile to do this with\n> some tests cleanup, will follow that up as a separate series.\n>\n> Changes since v3:\n> * Changed the position of `old_target` and `flags` in `ref_transaction_delete`\n> to make it a coherent.\n> * Added tests for deletion of regular refs using 'symref-delete', this lead to\n> adding a new commit to have specific errors for when a regular update contains\n> `old_target`.\n>\n> [1]: https://lore.kernel.org/r/CAOLa=ZQW-cCV5BP_fCvuZimfkjwAzjEiqXYRPft1Wf9kAX=_bw@mail.gmail.com\n>\n> Karthik Nayak (7):\n> refs: create and use `ref_update_expects_existing_old_ref()`\n> refs: specify error for regular refs with `old_target`\n> update-ref: add support for 'symref-verify' command\n> update-ref: add support for 'symref-delete' command\n> update-ref: add support for 'symref-create' command\n> reftable: pick either 'oid' or 'target' for new updates\n> update-ref: add support for 'symref-update' command\n>\n> Documentation/git-update-ref.txt | 25 ++\n> builtin/clone.c | 2 +-\n> builtin/fetch.c | 4 +-\n> builtin/receive-pack.c | 3 +-\n> builtin/update-ref.c | 237 ++++++++++++++++-\n> refs.c | 40 ++-\n> refs.h | 6 +-\n> refs/files-backend.c | 16 +-\n> refs/refs-internal.h | 6 +\n> refs/reftable-backend.c | 16 +-\n> t/t0600-reffiles-backend.sh | 32 +++\n> t/t1400-update-ref.sh | 430 ++++++++++++++++++++++++++++++-\n> t/t1416-ref-transaction-hooks.sh | 54 ++++\n> t/t5605-clone-local.sh | 2 +-\n> 14 files changed, 832 insertions(+), 41 deletions(-)\n>\n> Range-diff against v3:\n> -: ---------- > 1: cab5265c3c refs: create and use `ref_update_expects_existing_old_ref()`\n> -: ---------- > 2: 57b5ff46c0 refs: specify error for regular refs with `old_target`\n> 1: ed54b0dfb9 = 3: 5710fa81bf update-ref: add support for 'symref-verify' command\n> 2: b82b86ff40 ! 4: 5f8fc4eb6e update-ref: add support for 'symref-delete' command\n> @@ Commit message\n> within a transaction, which promises atomicity of the operation and can\n> be batched with other commands.\n>\n> + When no 'old_target' is provided it can also delete regular refs,\n> + similar to how the 'delete' command can delete symrefs when no 'old_oid'\n> + is provided.\n> +\n> Helped-by: Patrick Steinhardt \n> Signed-off-by: Karthik Nayak \n>\n> @@ Documentation/git-update-ref.txt: verify::\n>\n> ## builtin/fetch.c ##\n> @@ builtin/fetch.c: static int prune_refs(struct display_state *display_state,\n> + \tif (!dry_run) {\n> \t\tif (transaction) {\n> \t\t\tfor (ref = stale_refs; ref; ref = ref->next) {\n> - \t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n> +-\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n> -\t\t\t\t\t\t\t\t\"fetch: prune\", &err);\n> -+\t\t\t\t\t\t\t\tNULL, \"fetch: prune\", &err);\n> ++\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL,\n> ++\t\t\t\t\t\t\t\tNULL, 0, \"fetch: prune\", &err);\n> \t\t\t\tif (result)\n> \t\t\t\t\tgoto cleanup;\n> \t\t\t}\n> @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh\n> \t\t\t\t\t namespaced_name,\n> \t\t\t\t\t old_oid,\n> -\t\t\t\t\t 0, \"push\", &err)) {\n> -+\t\t\t\t\t 0, NULL,\n> ++\t\t\t\t\t NULL, 0,\n> +\t\t\t\t\t \"push\", &err)) {\n> \t\t\trp_error(\"%s\", err.buf);\n> \t\t\tret = \"failed to delete\";\n> @@ builtin/update-ref.c: static void parse_cmd_delete(struct ref_transaction *trans\n> \tif (ref_transaction_delete(transaction, refname,\n> \t\t\t\t have_old ? &old_oid : NULL,\n> -\t\t\t\t update_flags, msg, &err))\n> -+\t\t\t\t update_flags, NULL, msg, &err))\n> ++\t\t\t\t NULL, update_flags, msg, &err))\n> \t\tdie(\"%s\", err.buf);\n>\n> \tupdate_flags = default_flags;\n> @@ builtin/update-ref.c: static void parse_cmd_delete(struct ref_transaction *trans\n> +\t\tdie(\"symref-delete %s: extra input: %s\", refname, next);\n> +\n> +\tif (ref_transaction_delete(transaction, refname, NULL,\n> -+\t\t\t\t update_flags, old_target, msg, &err))\n> ++\t\t\t\t old_target, update_flags, msg, &err))\n> +\t\tdie(\"%s\", err.buf);\n> +\n> +\tupdate_flags = default_flags;\n> @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,\n> \tif (!transaction ||\n> \t ref_transaction_delete(transaction, refname, old_oid,\n> -\t\t\t\t flags, msg, &err) ||\n> -+\t\t\t\t flags, NULL, msg, &err) ||\n> ++\t\t\t\t NULL, flags, msg, &err) ||\n> \t ref_transaction_commit(transaction, &err)) {\n> \t\terror(\"%s\", err.buf);\n> \t\tref_transaction_free(transaction);\n> @@ refs.c: int ref_transaction_create(struct ref_transaction *transaction,\n> \t\t\t const char *refname,\n> \t\t\t const struct object_id *old_oid,\n> -\t\t\t unsigned int flags, const char *msg,\n> -+\t\t\t unsigned int flags,\n> +\t\t\t const char *old_target,\n> ++\t\t\t unsigned int flags,\n> +\t\t\t const char *msg,\n> \t\t\t struct strbuf *err)\n> {\n> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n> \tfor_each_string_list_item(item, refnames) {\n> \t\tret = ref_transaction_delete(transaction, item->string,\n> -\t\t\t\t\t NULL, flags, msg, &err);\n> -+\t\t\t\t\t NULL, flags, NULL, msg, &err);\n> ++\t\t\t\t\t NULL, NULL, flags, msg, &err);\n> \t\tif (ret) {\n> \t\t\twarning(_(\"could not delete reference %s: %s\"),\n> \t\t\t\titem->string, err.buf);\n> @@ refs.h: int ref_transaction_create(struct ref_transaction *transaction,\n> \t\t\t const char *refname,\n> \t\t\t const struct object_id *old_oid,\n> -\t\t\t unsigned int flags, const char *msg,\n> -+\t\t\t unsigned int flags,\n> +\t\t\t const char *old_target,\n> ++\t\t\t unsigned int flags,\n> +\t\t\t const char *msg,\n> \t\t\t struct strbuf *err);\n>\n> @@ t/t1400-update-ref.sh: do\n> +\t\tgrep \"fatal: symref-delete: missing \" err\n> +\t'\n> +\n> ++\ttest_expect_success \"stdin $type symref-delete fails deleting regular ref\" '\n> ++\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n> ++\t\tgit update-ref refs/heads/regularref $a &&\n> ++\t\tformat_command $type \"symref-delete refs/heads/regularref\" \"$a\" >stdin &&\n> ++\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n> ++\t\tgrep \"fatal: cannot update regular ref: ${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set\" err\n> ++\t'\n> ++\n> +\ttest_expect_success \"stdin $type symref-delete fails with too many arguments\" '\n> +\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" \"$a\" >stdin &&\n> +\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n> @@ t/t1400-update-ref.sh: do\n> +\t\tgit update-ref --stdin $type --no-deref +\t\ttest_must_fail git symbolic-ref -d refs/heads/symref2\n> +\t'\n> ++\n> ++\ttest_expect_success \"stdin $type symref-delete fails deleting regular ref without target\" '\n> ++\t\tgit update-ref refs/heads/regularref $a &&\n> ++\t\tformat_command $type \"symref-delete refs/heads/regularref\" >stdin &&\n> ++\t\tgit update-ref --stdin $type --no-deref ++\t'\n> +\n> done\n>\n> 3: ae127f7d52 ! 5: 1542cfb806 update-ref: add support for 'symref-create' command\n> @@ t/t0600-reffiles-backend.sh: test_expect_success POSIXPERM 'git reflog expire ho\n>\n> ## t/t1400-update-ref.sh ##\n> @@ t/t1400-update-ref.sh: do\n> - \t\ttest_must_fail git symbolic-ref -d refs/heads/symref2\n> + \t\tgit update-ref --stdin $type --no-deref \t'\n>\n> +\ttest_expect_success \"stdin $type symref-create fails with too many arguments\" '\n> 4: 8889dcbf40 = 6: ec5380743d reftable: pick either 'oid' or 'target' for new updates\n> 5: 19d85d56c4 ! 7: f8d91f7fc9 update-ref: add support for 'symref-update' command\n> @@ Commit message\n> we don't use a `(ref | oid)` prefix for the old_value, then there is\n> ambiguity around if the value provided should be treated as an oid or a\n> reference. This is more so the reason, because we allow anything\n> - committish to be provided as an oid.\n> + committish to be provided as an oid. While 'symref-verify' and\n> + 'symref-delete' also take in `` we do not have this\n> + divergence there as those commands only work with symrefs. Whereas\n> + 'symref-update' also works with regular refs and allows users to convert\n> + regular refs to symrefs.\n>\n> The command allows users to perform symbolic ref updates within a\n> transaction. This provides atomicity and allows users to perform a set\n> --\n> 2.43.GIT\n\nI also noticed that the range-diff seems a bit weird. I used\n\n git format-patch -v 4 --cover-letter --range-diff=symref-cmd-v3 @~7..@\n\nto generate the patch series, but seems like when doing this it\ninternally computes the merge-base, which happens to be the first commit\nin this series, skewing the range-diff by one commit. I guess its always\nbetter to provide the commit range to `--range-diff`. Adding the range\ndiff for the same, when run using\n\n git format-patch -v 4 --cover-letter\n--range-diff=6ab505f161..symref-cmd-v3 @~7..@\n\nwhere `6ab505f161` is the common base for both v3 and v4, and is from\nmerging in `ps/leakfixes`.\n\nRange-diff against v3:\n1: cab5265c3c = 1: cab5265c3c refs: create and use\n`ref_update_expects_existing_old_ref()`\n-: ---------- > 2: 57b5ff46c0 refs: specify error for regular refs\nwith `old_target`\n2: ed54b0dfb9 = 3: 5710fa81bf update-ref: add support for\n'symref-verify' command\n3: b82b86ff40 ! 4: 5f8fc4eb6e update-ref: add support for\n'symref-delete' command\n @@ Commit message\n within a transaction, which promises atomicity of the operation and can\n be batched with other commands.\n\n + When no 'old_target' is provided it can also delete regular refs,\n + similar to how the 'delete' command can delete symrefs when\nno 'old_oid'\n + is provided.\n +\n Helped-by: Patrick Steinhardt \n Signed-off-by: Karthik Nayak \n\n @@ Documentation/git-update-ref.txt: verify::\n\n ## builtin/fetch.c ##\n @@ builtin/fetch.c: static int prune_refs(struct display_state\n*display_state,\n + \tif (!dry_run) {\n \t\tif (transaction) {\n \t\t\tfor (ref = stale_refs; ref; ref = ref->next) {\n - \t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n +-\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n -\t\t\t\t\t\t\t\t\"fetch: prune\", &err);\n -+\t\t\t\t\t\t\t\tNULL, \"fetch: prune\", &err);\n ++\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL,\n ++\t\t\t\t\t\t\t\tNULL, 0, \"fetch: prune\", &err);\n \t\t\t\tif (result)\n \t\t\t\t\tgoto cleanup;\n \t\t\t}\n @@ builtin/receive-pack.c: static const char *update(struct\ncommand *cmd, struct sh\n \t\t\t\t\t namespaced_name,\n \t\t\t\t\t old_oid,\n -\t\t\t\t\t 0, \"push\", &err)) {\n -+\t\t\t\t\t 0, NULL,\n ++\t\t\t\t\t NULL, 0,\n +\t\t\t\t\t \"push\", &err)) {\n \t\t\trp_error(\"%s\", err.buf);\n \t\t\tret = \"failed to delete\";\n @@ builtin/update-ref.c: static void parse_cmd_delete(struct\nref_transaction *trans\n \tif (ref_transaction_delete(transaction, refname,\n \t\t\t\t have_old ? &old_oid : NULL,\n -\t\t\t\t update_flags, msg, &err))\n -+\t\t\t\t update_flags, NULL, msg, &err))\n ++\t\t\t\t NULL, update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n\n \tupdate_flags = default_flags;\n @@ builtin/update-ref.c: static void parse_cmd_delete(struct\nref_transaction *trans\n +\t\tdie(\"symref-delete %s: extra input: %s\", refname, next);\n +\n +\tif (ref_transaction_delete(transaction, refname, NULL,\n -+\t\t\t\t update_flags, old_target, msg, &err))\n ++\t\t\t\t old_target, update_flags, msg, &err))\n +\t\tdie(\"%s\", err.buf);\n +\n +\tupdate_flags = default_flags;\n @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n -\t\t\t\t flags, msg, &err) ||\n -+\t\t\t\t flags, NULL, msg, &err) ||\n ++\t\t\t\t NULL, flags, msg, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n @@ refs.c: int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n -\t\t\t unsigned int flags, const char *msg,\n -+\t\t\t unsigned int flags,\n +\t\t\t const char *old_target,\n ++\t\t\t unsigned int flags,\n +\t\t\t const char *msg,\n \t\t\t struct strbuf *err)\n {\n @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \tfor_each_string_list_item(item, refnames) {\n \t\tret = ref_transaction_delete(transaction, item->string,\n -\t\t\t\t\t NULL, flags, msg, &err);\n -+\t\t\t\t\t NULL, flags, NULL, msg, &err);\n ++\t\t\t\t\t NULL, NULL, flags, msg, &err);\n \t\tif (ret) {\n \t\t\twarning(_(\"could not delete reference %s: %s\"),\n \t\t\t\titem->string, err.buf);\n @@ refs.h: int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n -\t\t\t unsigned int flags, const char *msg,\n -+\t\t\t unsigned int flags,\n +\t\t\t const char *old_target,\n ++\t\t\t unsigned int flags,\n +\t\t\t const char *msg,\n \t\t\t struct strbuf *err);\n\n @@ t/t1400-update-ref.sh: do\n +\t\tgrep \"fatal: symref-delete: missing \" err\n +\t'\n +\n ++\ttest_expect_success \"stdin $type symref-delete fails deleting\nregular ref\" '\n ++\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n ++\t\tgit update-ref refs/heads/regularref $a &&\n ++\t\tformat_command $type \"symref-delete refs/heads/regularref\"\n\"$a\" >stdin &&\n ++\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n ++\t\tgrep \"fatal: cannot update regular ref:\n${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set\" err\n ++\t'\n ++\n +\ttest_expect_success \"stdin $type symref-delete fails with too\nmany arguments\" '\n +\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\"\n\"$a\" >stdin &&\n +\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n @@ t/t1400-update-ref.sh: do\n +\t\tgit update-ref --stdin $type --no-deref stdin &&\n ++\t\tgit update-ref --stdin $type --no-deref ` we do not have this\n + divergence there as those commands only work with symrefs. Whereas\n + 'symref-update' also works with regular refs and allows users\nto convert\n + regular refs to symrefs.\n\n The command allows users to perform symbolic ref updates within a\n transaction. This provides atomicity and allows users to perform a set\n-- \n2.43.GIT\n"},{"id":"496522","messageId":"CAOLa=ZQcZ+pqhe+GbiG=UJi1a3vjQBCz4EDthpa+Rj85FQ0ecw@mail.gmail.com","threadId":"61588","inReplyTo":"ZmGXREp9Xa6qHpMC@tanuki","subject":"Re: [PATCH v4 2/7] refs: specify error for regular refs with `old_target`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-06T14:20:42Z","receivedAt":"2024-06-06T14:20:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Wed, Jun 05, 2024 at 12:29:53PM +0200, Karthik Nayak wrote:\n>> From: Karthik Nayak \n>>\n>> When a regular reference update contains `old_target` set, we call the\n>\n> s/contains/has its/\n>\n>> `ref_update_check_old_target` function to check the referent value. But\n>> for regular refs we know that the referent value is not set and this\n>\n> This is fairly technical and focussed on the implementation. Can we\n> maybe talk more about intent (\"expected a symref, but is a direct ref\")\n> rather than the exact implementation to make the commit message a bit\n> easier to understand for folks?\n>\n\nFair enough, will change this.\n\n>> simply raises a generic error which says nothing about this being a\n>> regular ref. Instead let's raise a more specific error when a regular\n>> ref update contains `old_target`.\n>\n> It might be helpful to include before/after in this commit message to\n> show the change. Even better would be a test of course, but I think we\n> cannot add one at this point in time yet.\n>\n\nYeah will do.\n\nYeah, adding a test is only possible in the commit where we introduce\n`symref-delete` and we do test it there.\n\n>> Signed-off-by: Karthik Nayak \n>> ---\n>> refs/files-backend.c | 13 +++++++------\n>> refs/reftable-backend.c | 9 +++++++++\n>> 2 files changed, 16 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>> index 194e74eb4d..f516d4d82f 100644\n>> --- a/refs/files-backend.c\n>> +++ b/refs/files-backend.c\n>> @@ -2491,14 +2491,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n>>\n>> \t\t/*\n>> \t\t * Even if the ref is a regular ref, if `old_target` is set, we\n>> -\t\t * check the referent value. Ideally `old_target` should only\n>> -\t\t * be set for symrefs, but we're strict about its usage.\n>> +\t\t * fail with an error.\n>> \t\t */\n>> \t\tif (update->old_target) {\n>> -\t\t\tif (ref_update_check_old_target(referent.buf, update, err)) {\n>> -\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n>> -\t\t\t\tgoto out;\n>> -\t\t\t}\n>> +\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n>> +\t\t\t\t\t \"symref target '%s' set\"),\n>> +\t\t\t\t ref_update_original_update_refname(update),\n>> +\t\t\t\t update->old_target);\n>> +\t\t\tret = TRANSACTION_GENERIC_ERROR;\n>> +\t\t\tgoto out;\n>\n> I feel like these error messages are somewhat technical. If I were\n> reading it as a user, I don't think I'd understand what it is trying to\n> tell me. How about:\n>\n> strbuf_addf(err, _(\"cannot lock ref '%s': %s\",\n> \"expected symref but is a direct ref\"));\n>\n> This also matches the other error messages we have in this function more\n> closely. The same is true for the equivalent in the reftable backend.\n>\n> Patrick\n>\n\nI do want to also note that there is a old_target set so perhaps\n\nmodified refs/files-backend.c\n@@ -2494,8 +2494,9 @@ static int lock_ref_for_update(struct\nfiles_ref_store *refs,\n \t\t * fail with an error.\n \t\t */\n \t\tif (update->old_target) {\n-\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n-\t\t\t\t\t \"symref target '%s' set\"),\n+\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n+\t\t\t\t\t \"expected symref with target '%s': \"\n+\t\t\t\t\t \"but is a regular ref\"),\n \t\t\t\t ref_update_original_update_refname(update),\n \t\t\t\t update->old_target);\n \t\t\tret = TRANSACTION_GENERIC_ERROR;\n"},{"id":"496638","messageId":"20240607133304.2333280-1-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240605102958.716432-1-knayak@gitlab.com","subject":"[PATCH v5 0/7] update-ref: add symref support for --stdin","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:32:57Z","receivedAt":"2024-06-07T13:33:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nThe 'update-ref' command is used to update refs using transactions. The\ncommand allows users to also utilize a '--stdin' mode to provide a\nbatch of sub-commands which can be processed in a transaction.\n\nCurrently, the sub-commands involve {verify, delete, create, update}\nand they allow users to work with regular refs in the repository. To\nwork with symrefs, users only have the option of using\n'git-symbolic-ref', which doesn't provide transaction support to the\nusers eventhough it uses the same behind the hood.\n\nRecently, we modified the reference backend to add symref support,\nfollowing which, 'git-symbolic-ref' also uses the transaction backend.\nBut, it doesn't expose this to the user. To allow users to work with\nsymrefs via transaction, this series adds support for new sub-commands\n{symrer-verify, symref-delete, symref-create, symref-update} to the\n'--stdin' mode of update-ref. These complement the existing\nsub-commands.\n\nThe patches 1, 2, & 6 fix small issues in the reference backends. The other\npatches 3, 4, 5, & 7 each add one of the new sub-commands.\n\nThe series is based off master, with 'kn/ref-transaction-symref' merged\nin.\n\nThere was some discussion [1] also about adding `old_target` support to\nthe existing `update` command. I think its worthwhile to do this with\nsome tests cleanup, will follow that up as a separate series.\n\nChanges since v4:\n* Modified the commit message for the second patch and also the error to be\nless technical and more user friendly. \n* Renamed an incorrectly named test.\n\nVersion 1-3 can be found here:\nhttps://lore.kernel.org/all/20240530120940.456817-1-knayak@gitlab.com/\n\n[1]: https://lore.kernel.org/r/CAOLa=ZQW-cCV5BP_fCvuZimfkjwAzjEiqXYRPft1Wf9kAX=_bw@mail.gmail.com\n\nKarthik Nayak (7):\n refs: create and use `ref_update_expects_existing_old_ref()`\n refs: specify error for regular refs with `old_target`\n update-ref: add support for 'symref-verify' command\n update-ref: add support for 'symref-delete' command\n update-ref: add support for 'symref-create' command\n reftable: pick either 'oid' or 'target' for new updates\n update-ref: add support for 'symref-update' command\n\n Documentation/git-update-ref.txt | 25 ++\n builtin/clone.c | 2 +-\n builtin/fetch.c | 4 +-\n builtin/receive-pack.c | 3 +-\n builtin/update-ref.c | 237 ++++++++++++++++-\n refs.c | 40 ++-\n refs.h | 6 +-\n refs/files-backend.c | 17 +-\n refs/refs-internal.h | 6 +\n refs/reftable-backend.c | 17 +-\n t/t0600-reffiles-backend.sh | 32 +++\n t/t1400-update-ref.sh | 430 ++++++++++++++++++++++++++++++-\n t/t1416-ref-transaction-hooks.sh | 54 ++++\n t/t5605-clone-local.sh | 2 +-\n 14 files changed, 834 insertions(+), 41 deletions(-)\n\nRange-diff against v4:\n1: cab5265c3c = 1: cab5265c3c refs: create and use `ref_update_expects_existing_old_ref()`\n2: 57b5ff46c0 ! 2: 94077e69cc refs: specify error for regular refs with `old_target`\n @@ Metadata\n ## Commit message ##\n refs: specify error for regular refs with `old_target`\n \n - When a regular reference update contains `old_target` set, we call the\n - `ref_update_check_old_target` function to check the referent value. But\n - for regular refs we know that the referent value is not set and this\n - simply raises a generic error which says nothing about this being a\n - regular ref. Instead let's raise a more specific error when a regular\n - ref update contains `old_target`.\n + When a reference update tries to update a symref, but the ref in\n + question is actually a regular ref, we raise an error. However the error\n + raised in this situation is:\n +\n + verifying symref target: '': reference is missing but expected \n +\n + which is very generic and doesn't indicate the mismatch of types. Let's\n + make this error more specific:\n +\n + cannot lock ref '': expected symref with target '': but is a regular ref\n +\n + so that users have a clearer understanding.\n \n Signed-off-by: Karthik Nayak \n \n @@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref\n -\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n -\t\t\t\tgoto out;\n -\t\t\t}\n -+\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n -+\t\t\t\t\t \"symref target '%s' set\"),\n ++\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n ++\t\t\t\t\t \"expected symref with target '%s': \"\n ++\t\t\t\t\t \"but is a regular ref\"),\n +\t\t\t\t ref_update_original_update_refname(update),\n +\t\t\t\t update->old_target);\n +\t\t\tret = TRANSACTION_GENERIC_ERROR;\n @@ refs/reftable-backend.c: static int reftable_be_transaction_prepare(struct ref_s\n \t\t */\n \t\tif (u->old_target) {\n +\t\t\tif (!(u->type & REF_ISSYMREF)) {\n -+\t\t\t\tstrbuf_addf(err, _(\"cannot update regular ref: '%s': \"\n -+\t\t\t\t\t \"symref target '%s' set\"),\n ++\t\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n ++\t\t\t\t\t \"expected symref with target '%s': \"\n ++\t\t\t\t\t \"but is a regular ref\"),\n +\t\t\t\t\t ref_update_original_update_refname(u),\n +\t\t\t\t\t u->old_target);\n +\t\t\t\tret = -1;\n3: 5710fa81bf = 3: 153d8cce75 update-ref: add support for 'symref-verify' command\n4: 5f8fc4eb6e ! 4: 07382bfa09 update-ref: add support for 'symref-delete' command\n @@ t/t1400-update-ref.sh: do\n +\t\tgit update-ref refs/heads/regularref $a &&\n +\t\tformat_command $type \"symref-delete refs/heads/regularref\" \"$a\" >stdin &&\n +\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n -+\t\tgrep \"fatal: cannot update regular ref: ${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set\" err\n ++\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/regularref${SQ}: expected symref with target ${SQ}$a${SQ}: but is a regular ref\" err\n +\t'\n +\n +\ttest_expect_success \"stdin $type symref-delete fails with too many arguments\" '\n @@ t/t1400-update-ref.sh: do\n +\t\ttest_must_fail git symbolic-ref -d refs/heads/symref2\n +\t'\n +\n -+\ttest_expect_success \"stdin $type symref-delete fails deleting regular ref without target\" '\n ++\ttest_expect_success \"stdin $type symref-delete deletes regular ref without target\" '\n +\t\tgit update-ref refs/heads/regularref $a &&\n +\t\tformat_command $type \"symref-delete refs/heads/regularref\" >stdin &&\n +\t\tgit update-ref --stdin $type --no-deref \n\nThe files and reftable backend, need to check if a ref must exist, so\nthat the required validation can be done. A ref must exist only when the\n`old_oid` value of the update has been explicitly set and it is not the\n`null_oid` value.\n\nSince we also support symrefs now, we need to ensure that even when\n`old_target` is set a ref must exist. While this was missed when we\nadded symref support in transactions, there are no active users of this\npath. As we introduce the 'symref-verify' command in the upcoming\ncommits, it is important to fix this.\n\nSo let's export this to a function called\n`ref_update_expects_existing_old_ref()` and expose it internally via\n'refs-internal.h'.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n refs.c | 6 ++++++\n refs/files-backend.c | 3 +--\n refs/refs-internal.h | 6 ++++++\n refs/reftable-backend.c | 2 +-\n 4 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 8260c27cde..50d8d7d777 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2679,3 +2679,9 @@ int ref_update_check_old_target(const char *referent, struct ref_update *update,\n \t\t\t referent, update->old_target);\n \treturn -1;\n }\n+\n+int ref_update_expects_existing_old_ref(struct ref_update *update)\n+{\n+\treturn (update->flags & REF_HAVE_OLD) &&\n+\t\t(!is_null_oid(&update->old_oid) || update->old_target);\n+}\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5f3089d947..194e74eb4d 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2412,8 +2412,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t struct strbuf *err)\n {\n \tstruct strbuf referent = STRBUF_INIT;\n-\tint mustexist = (update->flags & REF_HAVE_OLD) &&\n-\t\t!is_null_oid(&update->old_oid);\n+\tint mustexist = ref_update_expects_existing_old_ref(update);\n \tint ret = 0;\n \tstruct ref_lock *lock;\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 53a6c5d842..ee298ec0d5 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -765,4 +765,10 @@ int ref_update_has_null_new_value(struct ref_update *update);\n int ref_update_check_old_target(const char *referent, struct ref_update *update,\n \t\t\t\tstruct strbuf *err);\n \n+/*\n+ * Check if the ref must exist, this means that the old_oid or\n+ * old_target is non NULL.\n+ */\n+int ref_update_expects_existing_old_ref(struct ref_update *update);\n+\n #endif /* REFS_REFS_INTERNAL_H */\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 1af86bbdec..b838cf8f00 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -824,7 +824,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t\t ¤t_oid, &referent, &u->type);\n \t\tif (ret < 0)\n \t\t\tgoto done;\n-\t\tif (ret > 0 && (!(u->flags & REF_HAVE_OLD) || is_null_oid(&u->old_oid))) {\n+\t\tif (ret > 0 && !ref_update_expects_existing_old_ref(u)) {\n \t\t\t/*\n \t\t\t * The reference does not exist, and we either have no\n \t\t\t * old object ID or expect the reference to not exist.\n-- \n2.43.GIT\n\n"},{"id":"496639","messageId":"20240607133304.2333280-3-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240607133304.2333280-1-knayak@gitlab.com","subject":"[PATCH v5 2/7] refs: specify error for regular refs with `old_target`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:32:59Z","receivedAt":"2024-06-07T13:33:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nWhen a reference update tries to update a symref, but the ref in\nquestion is actually a regular ref, we raise an error. However the error\nraised in this situation is:\n\n verifying symref target: '': reference is missing but expected \n\nwhich is very generic and doesn't indicate the mismatch of types. Let's\nmake this error more specific:\n\n cannot lock ref '': expected symref with target '': but is a regular ref\n\nso that users have a clearer understanding.\n\nSigned-off-by: Karthik Nayak \n---\n refs/files-backend.c | 14 ++++++++------\n refs/reftable-backend.c | 10 ++++++++++\n 2 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 194e74eb4d..fc57c9d220 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2491,14 +2491,16 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \n \t\t/*\n \t\t * Even if the ref is a regular ref, if `old_target` is set, we\n-\t\t * check the referent value. Ideally `old_target` should only\n-\t\t * be set for symrefs, but we're strict about its usage.\n+\t\t * fail with an error.\n \t\t */\n \t\tif (update->old_target) {\n-\t\t\tif (ref_update_check_old_target(referent.buf, update, err)) {\n-\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n-\t\t\t\tgoto out;\n-\t\t\t}\n+\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n+\t\t\t\t\t \"expected symref with target '%s': \"\n+\t\t\t\t\t \"but is a regular ref\"),\n+\t\t\t\t ref_update_original_update_refname(update),\n+\t\t\t\t update->old_target);\n+\t\t\tret = TRANSACTION_GENERIC_ERROR;\n+\t\t\tgoto out;\n \t\t} else if (check_old_oid(update, &lock->old_oid, err)) {\n \t\t\tret = TRANSACTION_GENERIC_ERROR;\n \t\t\tgoto out;\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex b838cf8f00..c66ab9ecd8 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -928,6 +928,16 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t * backend returns, which keeps our tests happy.\n \t\t */\n \t\tif (u->old_target) {\n+\t\t\tif (!(u->type & REF_ISSYMREF)) {\n+\t\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n+\t\t\t\t\t \"expected symref with target '%s': \"\n+\t\t\t\t\t \"but is a regular ref\"),\n+\t\t\t\t\t ref_update_original_update_refname(u),\n+\t\t\t\t\t u->old_target);\n+\t\t\t\tret = -1;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\n \t\t\tif (ref_update_check_old_target(referent.buf, u, err)) {\n \t\t\t\tret = -1;\n \t\t\t\tgoto done;\n-- \n2.43.GIT\n\n"},{"id":"496641","messageId":"20240607133304.2333280-4-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240607133304.2333280-1-knayak@gitlab.com","subject":"[PATCH v5 3/7] update-ref: add support for 'symref-verify' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:33:00Z","receivedAt":"2024-06-07T13:33:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nThe 'symref-verify' command allows users to verify if a provided \ncontains the provided without changing the . If\n is not provided, the command will verify that the \ndoesn't exist.\n\nThe command allows users to verify symbolic refs within a transaction,\nand this means users can perform a set of changes in a transaction only\nwhen the verification holds good.\n\nSince we're checking for symbolic refs, this command will only work with\nthe 'no-deref' mode. This is because any dereferenced symbolic ref will\npoint to an object and not a ref and the regular 'verify' command can be\nused in such situations.\n\nAdd required tests for symref support in 'verify'. Since we're here,\nalso add reflog checks for the pre-existing 'verify' tests, there is no\ndivergence from behavior, but we never tested to ensure that reflog\nwasn't affected by the 'verify' command.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 7 +++\n builtin/update-ref.c | 80 +++++++++++++++++++++++----\n refs.c | 11 +++-\n refs.h | 1 +\n t/t1400-update-ref.sh | 94 +++++++++++++++++++++++++++++++-\n t/t1416-ref-transaction-hooks.sh | 30 ++++++++++\n 6 files changed, 208 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 374a2ebd2b..9fe78b3501 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-verify SP [SP ] LF\n \toption SP LF\n \tstart LF\n \tprepare LF\n@@ -86,6 +87,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-verify SP [NUL ] NUL\n \toption SP NUL\n \tstart NUL\n \tprepare NUL\n@@ -117,6 +119,11 @@ verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\n \n+symref-verify::\n+\tVerify symbolic against but do not change it.\n+\tIf is missing, the ref must not exist. Can only be\n+\tused in `no-deref` mode.\n+\n option::\n \tModify the behavior of the next command naming a .\n \tThe only valid option is `no-deref` to avoid dereferencing\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 6cda1c08aa..50f5472160 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -76,6 +76,29 @@ static char *parse_refname(const char **next)\n \treturn strbuf_detach(&ref, NULL);\n }\n \n+/*\n+ * Wrapper around parse_refname which skips the next delimiter.\n+ */\n+static char *parse_next_refname(const char **next)\n+{\n+\tif (line_termination) {\n+\t\t/* Without -z, consume SP and use next argument */\n+\t\tif (!**next || **next == line_termination)\n+\t\t\treturn NULL;\n+\t\tif (**next != ' ')\n+\t\t\tdie(\"expected SP but got: %s\", *next);\n+\t} else {\n+\t\t/* With -z, read the next NUL-terminated line */\n+\t\tif (**next)\n+\t\t\treturn NULL;\n+\t}\n+\t/* Skip the delimiter */\n+\t(*next)++;\n+\n+\treturn parse_refname(next);\n+}\n+\n+\n /*\n * The value being parsed is (as opposed to ; the\n * difference affects which error messages are generated):\n@@ -297,11 +320,47 @@ static void parse_cmd_verify(struct ref_transaction *transaction,\n \t\tdie(\"verify %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_verify(transaction, refname, &old_oid,\n-\t\t\t\t update_flags, &err))\n+\t\t\t\t NULL, update_flags, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tstrbuf_release(&err);\n+}\n+\n+static void parse_cmd_symref_verify(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct object_id old_oid;\n+\tchar *refname, *old_target;\n+\n+\tif (!(update_flags & REF_NO_DEREF))\n+\t\tdie(\"symref-verify: cannot operate with deref mode\");\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-verify: missing \");\n+\n+\t/*\n+\t * old_ref is optional, if not provided, we need to ensure that the\n+\t * ref doesn't exist.\n+\t */\n+\told_target = parse_next_refname(&next);\n+\tif (!old_target)\n+\t\toidcpy(&old_oid, null_oid());\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-verify %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_verify(transaction, refname,\n+\t\t\t\t old_target ? NULL : &old_oid,\n+\t\t\t\t old_target, update_flags, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = default_flags;\n \tfree(refname);\n+\tfree(old_target);\n \tstrbuf_release(&err);\n }\n \n@@ -380,15 +439,16 @@ static const struct parse_cmd {\n \tunsigned args;\n \tenum update_refs_state state;\n } command[] = {\n-\t{ \"update\", parse_cmd_update, 3, UPDATE_REFS_OPEN },\n-\t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n-\t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n-\t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n-\t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\n-\t{ \"start\", parse_cmd_start, 0, UPDATE_REFS_STARTED },\n-\t{ \"prepare\", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },\n-\t{ \"abort\", parse_cmd_abort, 0, UPDATE_REFS_CLOSED },\n-\t{ \"commit\", parse_cmd_commit, 0, UPDATE_REFS_CLOSED },\n+\t{ \"update\", parse_cmd_update, 3, UPDATE_REFS_OPEN },\n+\t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n+\t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n+\t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\n+\t{ \"start\", parse_cmd_start, 0, UPDATE_REFS_STARTED },\n+\t{ \"prepare\", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },\n+\t{ \"abort\", parse_cmd_abort, 0, UPDATE_REFS_CLOSED },\n+\t{ \"commit\", parse_cmd_commit, 0, UPDATE_REFS_CLOSED },\n };\n \n static void update_refs_stdin(void)\ndiff --git a/refs.c b/refs.c\nindex 50d8d7d777..cdc4d25557 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1297,14 +1297,19 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n int ref_transaction_verify(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n+\t\t\t const char *old_target,\n \t\t\t unsigned int flags,\n \t\t\t struct strbuf *err)\n {\n-\tif (!old_oid)\n-\t\tBUG(\"verify called with old_oid set to NULL\");\n+\tif (!old_target && !old_oid)\n+\t\tBUG(\"verify called with old_oid and old_target set to NULL\");\n+\tif (old_oid && old_target)\n+\t\tBUG(\"verify called with both old_oid and old_target set\");\n+\tif (old_target && !(flags & REF_NO_DEREF))\n+\t\tBUG(\"verify cannot operate on symrefs with deref mode\");\n \treturn ref_transaction_update(transaction, refname,\n \t\t\t\t NULL, old_oid,\n-\t\t\t\t NULL, NULL,\n+\t\t\t\t NULL, old_target,\n \t\t\t\t flags, NULL, err);\n }\n \ndiff --git a/refs.h b/refs.h\nindex 34568ee1fb..906299351c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -736,6 +736,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n int ref_transaction_verify(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n+\t\t\t const char *old_target,\n \t\t\t unsigned int flags,\n \t\t\t struct strbuf *err);\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex bbee2783ab..52801be07d 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -892,17 +892,23 @@ test_expect_success 'stdin update/create/verify combination works' '\n '\n \n test_expect_success 'stdin verify succeeds for correct value' '\n+\ttest-tool ref-store main for-each-reflog-ent $m >before &&\n \tgit rev-parse $m >expect &&\n \techo \"verify $m $m\" >stdin &&\n \tgit update-ref --stdin actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp expect actual &&\n+\ttest-tool ref-store main for-each-reflog-ent $m >after &&\n+\ttest_cmp before after\n '\n \n test_expect_success 'stdin verify succeeds for missing reference' '\n+\ttest-tool ref-store main for-each-reflog-ent $m >before &&\n \techo \"verify refs/heads/missing $Z\" >stdin &&\n \tgit update-ref --stdin after &&\n+\ttest_cmp before after\n '\n \n test_expect_success 'stdin verify treats no value as missing' '\n@@ -1643,4 +1649,88 @@ test_expect_success PIPE 'transaction flushes status updates' '\n \ttest_cmp expected actual\n '\n \n+format_command () {\n+\tif test \"$1\" = \"-z\"\n+\tthen\n+\t\tshift\n+\t\tprintf \"$F\" \"$@\"\n+\telse\n+\t\techo \"$@\"\n+\tfi\n+}\n+\n+for type in \"\" \"-z\"\n+do\n+\n+\ttest_expect_success \"stdin $type symref-verify fails without --no-deref\" '\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: symref-verify: cannot operate with deref mode\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails with too many arguments\" '\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-verify refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify succeeds for correct value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >before &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >after &&\n+\t\ttest_cmp before after\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails with no value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref before &&\n+\t\tformat_command $type \"symref-verify refs/heads/missing\" \"refs/heads/unknown\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/missing${SQ}: unable to resolve reference ${SQ}refs/heads/missing${SQ}\" err &&\n+\t\ttest_must_fail git rev-parse --verify -q refs/heads/missing &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >after &&\n+\t\ttest_cmp before after\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails for wrong value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$b\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-verify fails for mistaken null value\" '\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\tformat_command $type \"symref-verify refs/heads/symref\" \"$Z\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+done\n+\n test_done\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex 067fd57290..fd58b902f4 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -157,4 +157,34 @@ test_expect_success 'hook captures git-symbolic-ref updates' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'hook gets all queued symref updates' '\n+\ttest_when_finished \"rm actual\" &&\n+\n+\tgit update-ref refs/heads/branch $POST_OID &&\n+\tgit symbolic-ref refs/heads/symref refs/heads/main &&\n+\n+\ttest_hook reference-transaction <<-\\EOF &&\n+\techo \"$*\" >>actual\n+\twhile read -r line\n+\tdo\n+\t\tprintf \"%s\\n\" \"$line\"\n+\tdone >>actual\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tprepared\n+\tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tcommitted\n+\tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tEOF\n+\n+\tgit update-ref --no-deref --stdin <<-EOF &&\n+\tstart\n+\tsymref-verify refs/heads/symref refs/heads/main\n+\tprepare\n+\tcommit\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.43.GIT\n\n"},{"id":"496642","messageId":"20240607133304.2333280-5-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240607133304.2333280-1-knayak@gitlab.com","subject":"[PATCH v5 4/7] update-ref: add support for 'symref-delete' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:33:01Z","receivedAt":"2024-06-07T13:33:12Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd a new command 'symref-delete' to allow deletions of symbolic refs in\na transaction via the '--stdin' mode of the 'git-update-ref' command.\nThe 'symref-delete' command can, when given an , delete the\nprovided only when it points to .\n\nThis command is only compatible with the 'no-deref' mode because we\noptionally want to check the 'old_target' of the ref being deleted.\nDe-referencing a symbolic ref would provide a regular ref and we already\nhave the 'delete' command for regular refs.\n\nWhile users can also use 'git symbolic-ref -d' to delete symbolic refs,\nthe 'symref-delete' command in 'git-update-ref' allows users to do so\nwithin a transaction, which promises atomicity of the operation and can\nbe batched with other commands.\n\nWhen no 'old_target' is provided it can also delete regular refs,\nsimilar to how the 'delete' command can delete symrefs when no 'old_oid'\nis provided.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 5 +++\n builtin/fetch.c | 4 +-\n builtin/receive-pack.c | 3 +-\n builtin/update-ref.c | 33 +++++++++++++++-\n refs.c | 14 +++++--\n refs.h | 4 +-\n t/t1400-update-ref.sh | 68 ++++++++++++++++++++++++++++++++\n t/t1416-ref-transaction-hooks.sh | 19 ++++++++-\n 8 files changed, 140 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 9fe78b3501..16e02f6979 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-delete SP [SP ] LF\n \tsymref-verify SP [SP ] LF\n \toption SP LF\n \tstart LF\n@@ -87,6 +88,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-delete SP [NUL ] NUL\n \tsymref-verify SP [NUL ] NUL\n \toption SP NUL\n \tstart NUL\n@@ -119,6 +121,9 @@ verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\n \n+symref-delete::\n+\tDelete after verifying it exists with , if given.\n+\n symref-verify::\n \tVerify symbolic against but do not change it.\n \tIf is missing, the ref must not exist. Can only be\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 75255dc600..d63100e0d3 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1386,8 +1386,8 @@ static int prune_refs(struct display_state *display_state,\n \tif (!dry_run) {\n \t\tif (transaction) {\n \t\t\tfor (ref = stale_refs; ref; ref = ref->next) {\n-\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL, 0,\n-\t\t\t\t\t\t\t\t\"fetch: prune\", &err);\n+\t\t\t\tresult = ref_transaction_delete(transaction, ref->name, NULL,\n+\t\t\t\t\t\t\t\tNULL, 0, \"fetch: prune\", &err);\n \t\t\t\tif (result)\n \t\t\t\t\tgoto cleanup;\n \t\t\t}\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 01c1f04ece..0a30fac239 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1576,7 +1576,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\tif (ref_transaction_delete(transaction,\n \t\t\t\t\t namespaced_name,\n \t\t\t\t\t old_oid,\n-\t\t\t\t\t 0, \"push\", &err)) {\n+\t\t\t\t\t NULL, 0,\n+\t\t\t\t\t \"push\", &err)) {\n \t\t\trp_error(\"%s\", err.buf);\n \t\t\tret = \"failed to delete\";\n \t\t} else {\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 50f5472160..0cb7eef3c6 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -293,7 +293,7 @@ static void parse_cmd_delete(struct ref_transaction *transaction,\n \n \tif (ref_transaction_delete(transaction, refname,\n \t\t\t\t have_old ? &old_oid : NULL,\n-\t\t\t\t update_flags, msg, &err))\n+\t\t\t\t NULL, update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = default_flags;\n@@ -301,6 +301,36 @@ static void parse_cmd_delete(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+\n+static void parse_cmd_symref_delete(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tchar *refname, *old_target;\n+\n+\tif (!(update_flags & REF_NO_DEREF))\n+\t\tdie(\"symref-delete: cannot operate with deref mode\");\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-delete: missing \");\n+\n+\told_target = parse_next_refname(&next);\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-delete %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_delete(transaction, refname, NULL,\n+\t\t\t\t old_target, update_flags, msg, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tfree(old_target);\n+\tstrbuf_release(&err);\n+}\n+\n+\n static void parse_cmd_verify(struct ref_transaction *transaction,\n \t\t\t const char *next, const char *end)\n {\n@@ -443,6 +473,7 @@ static const struct parse_cmd {\n \t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n \t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-delete\", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\n \t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\n \t{ \"start\", parse_cmd_start, 0, UPDATE_REFS_STARTED },\ndiff --git a/refs.c b/refs.c\nindex cdc4d25557..01f3188a09 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -950,7 +950,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,\n \ttransaction = ref_store_transaction_begin(refs, &err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_oid,\n-\t\t\t\t flags, msg, &err) ||\n+\t\t\t\t NULL, flags, msg, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n@@ -1283,14 +1283,20 @@ int ref_transaction_create(struct ref_transaction *transaction,\n int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n-\t\t\t unsigned int flags, const char *msg,\n+\t\t\t const char *old_target,\n+\t\t\t unsigned int flags,\n+\t\t\t const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tif (old_oid && is_null_oid(old_oid))\n \t\tBUG(\"delete called with old_oid set to zeros\");\n+\tif (old_oid && old_target)\n+\t\tBUG(\"delete called with both old_oid and old_target set\");\n+\tif (old_target && !(flags & REF_NO_DEREF))\n+\t\tBUG(\"delete cannot operate on symrefs with deref mode\");\n \treturn ref_transaction_update(transaction, refname,\n \t\t\t\t null_oid(), old_oid,\n-\t\t\t\t NULL, NULL, flags,\n+\t\t\t\t NULL, old_target, flags,\n \t\t\t\t msg, err);\n }\n \n@@ -2599,7 +2605,7 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,\n \n \tfor_each_string_list_item(item, refnames) {\n \t\tret = ref_transaction_delete(transaction, item->string,\n-\t\t\t\t\t NULL, flags, msg, &err);\n+\t\t\t\t\t NULL, NULL, flags, msg, &err);\n \t\tif (ret) {\n \t\t\twarning(_(\"could not delete reference %s: %s\"),\n \t\t\t\titem->string, err.buf);\ndiff --git a/refs.h b/refs.h\nindex 906299351c..974cf4dd08 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -722,7 +722,9 @@ int ref_transaction_create(struct ref_transaction *transaction,\n int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *old_oid,\n-\t\t\t unsigned int flags, const char *msg,\n+\t\t\t const char *old_target,\n+\t\t\t unsigned int flags,\n+\t\t\t const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 52801be07d..9dbe28bd13 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1731,6 +1731,74 @@ do\n \t\ttest_cmp expect actual\n \t'\n \n+\ttest_expect_success \"stdin $type symref-delete fails without --no-deref\" '\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: symref-delete: cannot operate with deref mode\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails with no ref\" '\n+\t\tformat_command $type \"symref-delete \" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: symref-delete: missing \" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails deleting regular ref\" '\n+\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n+\t\tgit update-ref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-delete refs/heads/regularref\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/regularref${SQ}: expected symref with target ${SQ}$a${SQ}: but is a regular ref\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails with too many arguments\" '\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-delete refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete fails with wrong old value\" '\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$m\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected refs/heads/main\" err &&\n+\t\tgit symbolic-ref refs/heads/symref >expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-delete works with right old value\" '\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\tformat_command $type \"symref-delete refs/heads/symref\" \"\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref >actual\n@@ -171,16 +172,32 @@ test_expect_success 'hook gets all queued symref updates' '\n \tdone >>actual\n \tEOF\n \n-\tcat >expect <<-EOF &&\n+\t# In the files backend, \"delete\" also triggers an additional transaction\n+\t# update on the packed-refs backend, which constitutes additional reflog\n+\t# entries.\n+\tif test_have_prereq REFFILES\n+\tthen\n+\t\tcat >expect <<-EOF\n+\t\taborted\n+\t\t$ZERO_OID $ZERO_OID refs/heads/symrefd\n+\t\tEOF\n+\telse\n+\t\t>expect\n+\tfi &&\n+\n+\tcat >>expect <<-EOF &&\n \tprepared\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \tcommitted\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n+\tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \tEOF\n \n \tgit update-ref --no-deref --stdin <<-EOF &&\n \tstart\n \tsymref-verify refs/heads/symref refs/heads/main\n+\tsymref-delete refs/heads/symrefd refs/heads/main\n \tprepare\n \tcommit\n \tEOF\n-- \n2.43.GIT\n\n"},{"id":"496643","messageId":"20240607133304.2333280-6-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240607133304.2333280-1-knayak@gitlab.com","subject":"[PATCH v5 5/7] update-ref: add support for 'symref-create' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:33:02Z","receivedAt":"2024-06-07T13:33:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd 'symref-create' command to the '--stdin' mode 'git-update-ref' to\nallow creation of symbolic refs in a transaction. The 'symref-create'\ncommand takes in a , which the created will point to.\n\nAlso, support the 'core.prefersymlinkrefs' config, wherein if the config\nis set and the filesystem supports symlinks, we create the symbolic ref\nas a symlink. We fallback to creating a regular symref if creating the\nsymlink is unsuccessful.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 6 +++\n builtin/clone.c | 2 +-\n builtin/update-ref.c | 32 +++++++++++++++-\n refs.c | 9 +++--\n refs.h | 1 +\n t/t0600-reffiles-backend.sh | 32 ++++++++++++++++\n t/t1400-update-ref.sh | 65 ++++++++++++++++++++++++++++++++\n t/t1416-ref-transaction-hooks.sh | 3 ++\n t/t5605-clone-local.sh | 2 +-\n 9 files changed, 146 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 16e02f6979..364ef78af1 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-create SP SP LF\n \tsymref-delete SP [SP ] LF\n \tsymref-verify SP [SP ] LF\n \toption SP LF\n@@ -88,6 +89,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-create SP NUL NUL\n \tsymref-delete SP [NUL ] NUL\n \tsymref-verify SP [NUL ] NUL\n \toption SP NUL\n@@ -121,6 +123,10 @@ verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\n \n+symref-create:\n+\tCreate symbolic ref with after verifying\n+\tit does not exist.\n+\n symref-delete::\n \tDelete after verifying it exists with , if given.\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 23993b905b..6ddb3084e6 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -576,7 +576,7 @@ static void write_remote_refs(const struct ref *local_refs)\n \t\tif (!r->peer_ref)\n \t\t\tcontinue;\n \t\tif (ref_transaction_create(t, r->peer_ref->name, &r->old_oid,\n-\t\t\t\t\t 0, NULL, &err))\n+\t\t\t\t\t NULL, 0, NULL, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t}\n \ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 0cb7eef3c6..9c40e94626 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -257,7 +257,7 @@ static void parse_cmd_create(struct ref_transaction *transaction,\n \tif (*next != line_termination)\n \t\tdie(\"create %s: extra input: %s\", refname, next);\n \n-\tif (ref_transaction_create(transaction, refname, &new_oid,\n+\tif (ref_transaction_create(transaction, refname, &new_oid, NULL,\n \t\t\t\t update_flags | create_reflog_flag,\n \t\t\t\t msg, &err))\n \t\tdie(\"%s\", err.buf);\n@@ -267,6 +267,35 @@ static void parse_cmd_create(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+\n+static void parse_cmd_symref_create(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\tchar *refname, *new_target;\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-create: missing \");\n+\n+\tnew_target = parse_next_refname(&next);\n+\tif (!new_target)\n+\t\tdie(\"symref-create %s: missing \", refname);\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-create %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_create(transaction, refname, NULL, new_target,\n+\t\t\t\t update_flags | create_reflog_flag,\n+\t\t\t\t msg, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tfree(new_target);\n+\tstrbuf_release(&err);\n+}\n+\n static void parse_cmd_delete(struct ref_transaction *transaction,\n \t\t\t const char *next, const char *end)\n {\n@@ -473,6 +502,7 @@ static const struct parse_cmd {\n \t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n \t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-create\", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-delete\", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\n \t{ \"option\", parse_cmd_option, 1, UPDATE_REFS_OPEN },\ndiff --git a/refs.c b/refs.c\nindex 01f3188a09..8807e87e3c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1268,15 +1268,18 @@ int ref_transaction_update(struct ref_transaction *transaction,\n int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *new_oid,\n+\t\t\t const char *new_target,\n \t\t\t unsigned int flags, const char *msg,\n \t\t\t struct strbuf *err)\n {\n-\tif (!new_oid || is_null_oid(new_oid)) {\n-\t\tstrbuf_addf(err, \"'%s' has a null OID\", refname);\n+\tif (new_oid && new_target)\n+\t\tBUG(\"create called with both new_oid and new_target set\");\n+\tif ((!new_oid || is_null_oid(new_oid)) && !new_target) {\n+\t\tstrbuf_addf(err, \"'%s' has neither a valid OID nor a target\", refname);\n \t\treturn 1;\n \t}\n \treturn ref_transaction_update(transaction, refname, new_oid,\n-\t\t\t\t null_oid(), NULL, NULL, flags,\n+\t\t\t\t null_oid(), new_target, NULL, flags,\n \t\t\t\t msg, err);\n }\n \ndiff --git a/refs.h b/refs.h\nindex 974cf4dd08..28e3bb8a42 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -708,6 +708,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const struct object_id *new_oid,\n+\t\t\t const char *new_target,\n \t\t\t unsigned int flags, const char *msg,\n \t\t\t struct strbuf *err);\n \ndiff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh\nindex 92f570313d..b2a771ff2b 100755\n--- a/t/t0600-reffiles-backend.sh\n+++ b/t/t0600-reffiles-backend.sh\n@@ -468,4 +468,36 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '\n \tesac\n '\n \n+test_expect_success SYMLINKS 'symref transaction supports symlinks' '\n+\ttest_when_finished \"git symbolic-ref -d TEST_SYMREF_HEAD\" &&\n+\tgit update-ref refs/heads/new @ &&\n+\ttest_config core.prefersymlinkrefs true &&\n+\tcat >stdin <<-EOF &&\n+\tstart\n+\tsymref-create TEST_SYMREF_HEAD refs/heads/new\n+\tprepare\n+\tcommit\n+\tEOF\n+\tgit update-ref --no-deref --stdin stdin <<-EOF &&\n+\tstart\n+\tsymref-create TEST_SYMREF_HEAD refs/heads/new\n+\tprepare\n+\tcommit\n+\tEOF\n+\tgit update-ref --no-deref --stdin actual &&\n+\techo refs/heads/new >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 9dbe28bd13..253263320d 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1799,6 +1799,71 @@ do\n \t\tgit update-ref --stdin $type --no-deref stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-create refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create fails with no target\" '\n+\t\tformat_command $type \"symref-create refs/heads/symref\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create works with --no-deref\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-create refs/heads/symref\" \"$a\" &&\n+\t\tgit update-ref --stdin $type err\n+\t'\n+\n+\ttest_expect_success \"stdin $type create dangling symref ref works\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-create refs/heads/symref\" \"refs/heads/unkown\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\techo refs/heads/unkown >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create does not create reflogs by default\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/symref\" &&\n+\t\tformat_command $type \"symref-create refs/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest_must_fail git reflog exists refs/symref\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-create reflogs with --create-reflog\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-create refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --create-reflog --stdin $type --no-deref expect &&\n+\t\techo $a >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\tgit reflog exists refs/heads/symref\n+\t'\n+\n done\n \n test_done\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex ccde1b944b..ff77dcca6b 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -189,15 +189,18 @@ test_expect_success 'hook gets all queued symref updates' '\n \tprepared\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n+\t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n \tcommitted\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n+\t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n \tEOF\n \n \tgit update-ref --no-deref --stdin <<-EOF &&\n \tstart\n \tsymref-verify refs/heads/symref refs/heads/main\n \tsymref-delete refs/heads/symrefd refs/heads/main\n+\tsymref-create refs/heads/symrefc refs/heads/main\n \tprepare\n \tcommit\n \tEOF\ndiff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh\nindex a3055869bc..339d8c786f 100755\n--- a/t/t5605-clone-local.sh\n+++ b/t/t5605-clone-local.sh\n@@ -163,7 +163,7 @@ test_expect_success REFFILES 'local clone from repo with corrupt refs fails grac\n \techo a >corrupt/.git/refs/heads/topic &&\n \n \ttest_must_fail git clone corrupt working 2>err &&\n-\tgrep \"has a null OID\" err\n+\tgrep \"has neither a valid OID nor a target\" err\n '\n \n test_done\n-- \n2.43.GIT\n\n"},{"id":"496644","messageId":"20240607133304.2333280-7-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240607133304.2333280-1-knayak@gitlab.com","subject":"[PATCH v5 6/7] reftable: pick either 'oid' or 'target' for new updates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:33:03Z","receivedAt":"2024-06-07T13:33:14Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nWhen creating a reference transaction update, we can provide the old/new\noid/target for the update. We have checks in place to ensure that for\neach old/new, either oid or target is set and not both.\n\nIn the reftable backend, when dealing with updates without the\n`REF_NO_DEREF` flag, we don't selectively propagate data as needed.\nSince there are no active users of the path, this is not caught. As we\nwant to introduce the 'symref-update' command in the upcoming commit,\nwhich would use this flow, correct it.\n\nHelped-by: Patrick Steinhardt \nSigned-off-by: Karthik Nayak \n---\n refs/reftable-backend.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex c66ab9ecd8..1c46fc87f8 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -895,8 +895,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,\n \t\t\t\t */\n \t\t\t\tnew_update = ref_transaction_add_update(\n \t\t\t\t\ttransaction, referent.buf, new_flags,\n-\t\t\t\t\t&u->new_oid, &u->old_oid, u->new_target,\n-\t\t\t\t\tu->old_target, u->msg);\n+\t\t\t\t\tu->new_target ? NULL : &u->new_oid,\n+\t\t\t\t\tu->old_target ? NULL : &u->old_oid,\n+\t\t\t\t\tu->new_target, u->old_target, u->msg);\n \n \t\t\t\tnew_update->parent_update = u;\n \n-- \n2.43.GIT\n\n"},{"id":"496645","messageId":"20240607133304.2333280-8-knayak@gitlab.com","threadId":"61588","inReplyTo":"20240607133304.2333280-1-knayak@gitlab.com","subject":"[PATCH v5 7/7] update-ref: add support for 'symref-update' command","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-07T13:33:04Z","receivedAt":"2024-06-07T13:33:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak \n\nAdd 'symref-update' command to the '--stdin' mode of 'git-update-ref' to\nallow updates of symbolic refs. The 'symref-update' command takes in a\n, which the will be updated to. If the doesn't\nexist it will be created.\n\nIt also optionally takes either an `ref ` or `oid\n`. If the is provided, it checks to see if the\n targets the before the update. If is provided\nit checks to ensure that it is a regular ref and is the\nOID before the update. This by extension also means that this when a\nzero is provided, it ensures that the ref didn't exist before.\n\nThe divergence in syntax from the regular `update` command is because if\nwe don't use a `(ref | oid)` prefix for the old_value, then there is\nambiguity around if the value provided should be treated as an oid or a\nreference. This is more so the reason, because we allow anything\ncommittish to be provided as an oid. While 'symref-verify' and\n'symref-delete' also take in `` we do not have this\ndivergence there as those commands only work with symrefs. Whereas\n'symref-update' also works with regular refs and allows users to convert\nregular refs to symrefs.\n\nThe command allows users to perform symbolic ref updates within a\ntransaction. This provides atomicity and allows users to perform a set\nof operations together.\n\nThis command supports deref mode, to ensure that we can update\ndereferenced regular refs to symrefs.\n\nHelped-by: Patrick Steinhardt \nHelped-by: Junio C Hamano \nSigned-off-by: Karthik Nayak \n---\n Documentation/git-update-ref.txt | 7 ++\n builtin/update-ref.c | 92 ++++++++++++++\n t/t1400-update-ref.sh | 203 +++++++++++++++++++++++++++++++\n t/t1416-ref-transaction-hooks.sh | 4 +\n 4 files changed, 306 insertions(+)\n\ndiff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt\nindex 364ef78af1..afcf33cf60 100644\n--- a/Documentation/git-update-ref.txt\n+++ b/Documentation/git-update-ref.txt\n@@ -65,6 +65,7 @@ performs all modifications together. Specify commands of the form:\n \tcreate SP SP LF\n \tdelete SP [SP ] LF\n \tverify SP [SP ] LF\n+\tsymref-update SP SP [SP (ref SP | oid SP )] LF\n \tsymref-create SP SP LF\n \tsymref-delete SP [SP ] LF\n \tsymref-verify SP [SP ] LF\n@@ -89,6 +90,7 @@ quoting:\n \tcreate SP NUL NUL\n \tdelete SP NUL [] NUL\n \tverify SP NUL [] NUL\n+\tsymref-update SP NUL [NUL (ref NUL | oid NUL )] NUL\n \tsymref-create SP NUL NUL\n \tsymref-delete SP [NUL ] NUL\n \tsymref-verify SP [NUL ] NUL\n@@ -119,6 +121,11 @@ delete::\n \tDelete after verifying it exists with , if\n \tgiven. If given, may not be zero.\n \n+symref-update::\n+\tSet to after verifying or ,\n+\tif given. Specify a zero to ensure that the ref does not\n+\texist before the update.\n+\n verify::\n \tVerify against but do not change it. If\n \t is zero or missing, the ref must not exist.\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 9c40e94626..471fa5c8d1 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -98,6 +98,42 @@ static char *parse_next_refname(const char **next)\n \treturn parse_refname(next);\n }\n \n+/*\n+ * Wrapper around parse_arg which skips the next delimiter.\n+ */\n+static char *parse_next_arg(const char **next)\n+{\n+\tstruct strbuf arg = STRBUF_INIT;\n+\n+\tif (line_termination) {\n+\t\t/* Without -z, consume SP and use next argument */\n+\t\tif (!**next || **next == line_termination)\n+\t\t\treturn NULL;\n+\t\tif (**next != ' ')\n+\t\t\tdie(\"expected SP but got: %s\", *next);\n+\t} else {\n+\t\t/* With -z, read the next NUL-terminated line */\n+\t\tif (**next)\n+\t\t\treturn NULL;\n+\t}\n+\t/* Skip the delimiter */\n+\t(*next)++;\n+\n+\tif (line_termination) {\n+\t\t/* Without -z, use the next argument */\n+\t\t*next = parse_arg(*next, &arg);\n+\t} else {\n+\t\t/* With -z, use everything up to the next NUL */\n+\t\tstrbuf_addstr(&arg, *next);\n+\t\t*next += arg.len;\n+\t}\n+\n+\tif (arg.len)\n+\t\treturn strbuf_detach(&arg, NULL);\n+\n+\tstrbuf_release(&arg);\n+\treturn NULL;\n+}\n \n /*\n * The value being parsed is (as opposed to ; the\n@@ -237,6 +273,61 @@ static void parse_cmd_update(struct ref_transaction *transaction,\n \tstrbuf_release(&err);\n }\n \n+static void parse_cmd_symref_update(struct ref_transaction *transaction,\n+\t\t\t\t const char *next, const char *end)\n+{\n+\tchar *refname, *new_target, *old_arg;\n+\tchar *old_target = NULL;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tstruct object_id old_oid;\n+\tint have_old_oid = 0;\n+\n+\trefname = parse_refname(&next);\n+\tif (!refname)\n+\t\tdie(\"symref-update: missing \");\n+\n+\tnew_target = parse_next_refname(&next);\n+\tif (!new_target)\n+\t\tdie(\"symref-update %s: missing \", refname);\n+\n+\told_arg = parse_next_arg(&next);\n+\tif (old_arg) {\n+\t\told_target = parse_next_arg(&next);\n+\t\tif (!old_target)\n+\t\t\tdie(\"symref-update %s: expected old value\", refname);\n+\n+\t\tif (!strcmp(old_arg, \"oid\")) {\n+\t\t\tif (repo_get_oid(the_repository, old_target, &old_oid))\n+\t\t\t\tdie(\"symref-update %s: invalid oid: %s\", refname, old_target);\n+\n+\t\t\thave_old_oid = 1;\n+\t\t} else if (!strcmp(old_arg, \"ref\")) {\n+\t\t\tif (check_refname_format(old_target, REFNAME_ALLOW_ONELEVEL))\n+\t\t\t\tdie(\"symref-update %s: invalid ref: %s\", refname, old_target);\n+\t\t} else {\n+\t\t\tdie(\"symref-update %s: invalid arg '%s' for old value\", refname, old_arg);\n+\t\t}\n+\t}\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"symref-update %s: extra input: %s\", refname, next);\n+\n+\tif (ref_transaction_update(transaction, refname, NULL,\n+\t\t\t\t have_old_oid ? &old_oid : NULL,\n+\t\t\t\t new_target,\n+\t\t\t\t have_old_oid ? NULL : old_target,\n+\t\t\t\t update_flags | create_reflog_flag,\n+\t\t\t\t msg, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tupdate_flags = default_flags;\n+\tfree(refname);\n+\tfree(old_arg);\n+\tfree(old_target);\n+\tfree(new_target);\n+\tstrbuf_release(&err);\n+}\n+\n static void parse_cmd_create(struct ref_transaction *transaction,\n \t\t\t const char *next, const char *end)\n {\n@@ -502,6 +593,7 @@ static const struct parse_cmd {\n \t{ \"create\", parse_cmd_create, 2, UPDATE_REFS_OPEN },\n \t{ \"delete\", parse_cmd_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"verify\", parse_cmd_verify, 2, UPDATE_REFS_OPEN },\n+\t{ \"symref-update\", parse_cmd_symref_update, 4, UPDATE_REFS_OPEN },\n \t{ \"symref-create\", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-delete\", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },\n \t{ \"symref-verify\", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 253263320d..eb1691860d 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -1362,6 +1362,7 @@ test_expect_success 'fails with duplicate HEAD update' '\n '\n \n test_expect_success 'fails with duplicate ref update via symref' '\n+\ttest_when_finished \"git symbolic-ref -d refs/heads/symref2\" &&\n \tgit branch target2 $A &&\n \tgit symbolic-ref refs/heads/symref2 refs/heads/target2 &&\n \tcat >stdin <<-EOF &&\n@@ -1864,6 +1865,208 @@ do\n \t\tgit reflog exists refs/heads/symref\n \t'\n \n+\ttest_expect_success \"stdin $type symref-update fails with too many arguments\" '\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"ref\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tif test \"$type\" = \"-z\"\n+\t\tthen\n+\t\t\tgrep \"fatal: unknown command: $a\" err\n+\t\telse\n+\t\t\tgrep \"fatal: symref-update refs/heads/symref: extra input: $a\" err\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails with wrong old value argument\" '\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"foo\" \"$a\" \"$a\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: symref-update refs/heads/symref: invalid arg ${SQ}foo${SQ} for old value\" err\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates with zero old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"oid\" \"$Z\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates with no old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates dangling\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"refs/heads/nonexistent\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails with wrong old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" \"ref\" \"$b\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref err &&\n+\t\tgrep \"fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected $b\" err &&\n+\t\ttest_must_fail git rev-parse --verify -q $c\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update updates dangling ref\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update updates dangling ref with old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"ref\" \"refs/heads/nonexistent\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails update dangling ref with wrong old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_must_fail git rev-parse refs/heads/nonexistent &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/nonexistent &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"ref\" \"refs/heads/wrongref\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update works with right old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" \"ref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update works with no old value\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" >stdin &&\n+\t\tgit update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update fails with empty old ref-target\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$m\" \"ref\" \"\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type --no-deref expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update creates (with deref)\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref --stdin $type expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&\n+\t\tgrep \"$Z $(git rev-parse $a)\" actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref with correct old-oid\" '\n+\t\ttest_when_finished \"git symbolic-ref -d --no-recurse refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" \"oid\" \"$(git rev-parse $a)\" >stdin &&\n+\t\tgit update-ref --stdin $type expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&\n+\t\tgrep \"$(git rev-parse $a) $(git rev-parse $a)\" actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref fails with wrong old-oid\" '\n+\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" \"oid\" \"$(git rev-parse refs/heads/target2)\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/regularref${SQ}: is at $(git rev-parse $a) but expected $(git rev-parse refs/heads/target2)\" err &&\n+\t\techo $(git rev-parse $a) >expect &&\n+\t\tgit rev-parse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref fails with invalid old-oid\" '\n+\t\ttest_when_finished \"git update-ref -d refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" \"oid\" \"not-a-ref-oid\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: symref-update refs/heads/regularref: invalid oid: not-a-ref-oid\" err &&\n+\t\techo $(git rev-parse $a) >expect &&\n+\t\tgit rev-parse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update existing symref with zero old-oid\" '\n+\t\ttest_when_finished \"git symbolic-ref -d --no-recurse refs/heads/symref\" &&\n+\t\tgit symbolic-ref refs/heads/symref refs/heads/target2 &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" \"oid\" \"$Z\" >stdin &&\n+\t\ttest_must_fail git update-ref --stdin $type err &&\n+\t\tgrep \"fatal: cannot lock ref ${SQ}refs/heads/symref${SQ}: reference already exists\" err &&\n+\t\techo refs/heads/target2 >expect &&\n+\t\tgit symbolic-ref refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref (with deref)\" '\n+\t\ttest_when_finished \"git symbolic-ref -d refs/heads/symref\" &&\n+\t\ttest_when_finished \"git update-ref -d --no-deref refs/heads/symref2\" &&\n+\t\tgit update-ref refs/heads/symref2 $a &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref refs/heads/symref2 &&\n+\t\tformat_command $type \"symref-update refs/heads/symref\" \"$a\" >stdin &&\n+\t\tgit update-ref $type --stdin expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref2 >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\techo refs/heads/symref2 >expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/symref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&\n+\t\tgrep \"$(git rev-parse $a) $(git rev-parse $a)\" actual\n+\t'\n+\n+\ttest_expect_success \"stdin $type symref-update regular ref to symref\" '\n+\t\ttest_when_finished \"git symbolic-ref -d --no-recurse refs/heads/regularref\" &&\n+\t\tgit update-ref --no-deref refs/heads/regularref $a &&\n+\t\tformat_command $type \"symref-update refs/heads/regularref\" \"$a\" >stdin &&\n+\t\tgit update-ref $type --stdin expect &&\n+\t\tgit symbolic-ref --no-recurse refs/heads/regularref >actual &&\n+\t\ttest_cmp expect actual &&\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&\n+\t\tgrep \"$(git rev-parse $a) $(git rev-parse $a)\" actual\n+\t'\n+\n done\n \n test_done\ndiff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh\nindex ff77dcca6b..5a812ca3c0 100755\n--- a/t/t1416-ref-transaction-hooks.sh\n+++ b/t/t1416-ref-transaction-hooks.sh\n@@ -163,6 +163,7 @@ test_expect_success 'hook gets all queued symref updates' '\n \tgit update-ref refs/heads/branch $POST_OID &&\n \tgit symbolic-ref refs/heads/symref refs/heads/main &&\n \tgit symbolic-ref refs/heads/symrefd refs/heads/main &&\n+\tgit symbolic-ref refs/heads/symrefu refs/heads/main &&\n \n \ttest_hook reference-transaction <<-\\EOF &&\n \techo \"$*\" >>actual\n@@ -190,10 +191,12 @@ test_expect_success 'hook gets all queued symref updates' '\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n+\tref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu\n \tcommitted\n \tref:refs/heads/main $ZERO_OID refs/heads/symref\n \tref:refs/heads/main $ZERO_OID refs/heads/symrefd\n \t$ZERO_OID ref:refs/heads/main refs/heads/symrefc\n+\tref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu\n \tEOF\n \n \tgit update-ref --no-deref --stdin <<-EOF &&\n@@ -201,6 +204,7 @@ test_expect_success 'hook gets all queued symref updates' '\n \tsymref-verify refs/heads/symref refs/heads/main\n \tsymref-delete refs/heads/symrefd refs/heads/main\n \tsymref-create refs/heads/symrefc refs/heads/main\n+\tsymref-update refs/heads/symrefu refs/heads/branch ref refs/heads/main\n \tprepare\n \tcommit\n \tEOF\n-- \n2.43.GIT\n\n"},{"id":"496754","messageId":"ZmajCzuQftZlmlmS@tanuki","threadId":"61588","inReplyTo":"20240607133304.2333280-3-knayak@gitlab.com","subject":"Re: [PATCH v5 2/7] refs: specify error for regular refs with `old_target`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-06-10T06:54:03Z","receivedAt":"2024-06-10T06:54:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jun 07, 2024 at 03:32:59PM +0200, Karthik Nayak wrote:\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index 194e74eb4d..fc57c9d220 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -2491,14 +2491,16 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n> \n> \t\t/*\n> \t\t * Even if the ref is a regular ref, if `old_target` is set, we\n> -\t\t * check the referent value. Ideally `old_target` should only\n> -\t\t * be set for symrefs, but we're strict about its usage.\n> +\t\t * fail with an error.\n> \t\t */\n> \t\tif (update->old_target) {\n> -\t\t\tif (ref_update_check_old_target(referent.buf, update, err)) {\n> -\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n> -\t\t\t\tgoto out;\n> -\t\t\t}\n> +\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n> +\t\t\t\t\t \"expected symref with target '%s': \"\n> +\t\t\t\t\t \"but is a regular ref\"),\n> +\t\t\t\t ref_update_original_update_refname(update),\n> +\t\t\t\t update->old_target);\n> +\t\t\tret = TRANSACTION_GENERIC_ERROR;\n> +\t\t\tgoto out;\n\nShouldn't the second colon be a comma? \"expected symref, but is a\nregular ref\" reads way more natural to me than \"expected symref: but is\na regular ref\".\n\nOther than that this series looks good to me now, thanks!\n\nPatrick\n"},{"id":"496759","messageId":"CAOLa=ZSum=gWbxMyekwjfxqJ38EPTbQ-d4ts-xGxKaiiDgUTNw@mail.gmail.com","threadId":"61588","inReplyTo":"ZmajCzuQftZlmlmS@tanuki","subject":"Re: [PATCH v5 2/7] refs: specify error for regular refs with `old_target`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-06-10T08:26:17Z","receivedAt":"2024-06-10T08:26:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Fri, Jun 07, 2024 at 03:32:59PM +0200, Karthik Nayak wrote:\n>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>> index 194e74eb4d..fc57c9d220 100644\n>> --- a/refs/files-backend.c\n>> +++ b/refs/files-backend.c\n>> @@ -2491,14 +2491,16 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n>>\n>> \t\t/*\n>> \t\t * Even if the ref is a regular ref, if `old_target` is set, we\n>> -\t\t * check the referent value. Ideally `old_target` should only\n>> -\t\t * be set for symrefs, but we're strict about its usage.\n>> +\t\t * fail with an error.\n>> \t\t */\n>> \t\tif (update->old_target) {\n>> -\t\t\tif (ref_update_check_old_target(referent.buf, update, err)) {\n>> -\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n>> -\t\t\t\tgoto out;\n>> -\t\t\t}\n>> +\t\t\tstrbuf_addf(err, _(\"cannot lock ref '%s': \"\n>> +\t\t\t\t\t \"expected symref with target '%s': \"\n>> +\t\t\t\t\t \"but is a regular ref\"),\n>> +\t\t\t\t ref_update_original_update_refname(update),\n>> +\t\t\t\t update->old_target);\n>> +\t\t\tret = TRANSACTION_GENERIC_ERROR;\n>> +\t\t\tgoto out;\n>\n> Shouldn't the second colon be a comma? \"expected symref, but is a\n> regular ref\" reads way more natural to me than \"expected symref: but is\n> a regular ref\".\n>\n\nIt makes sense the way you put it, but we also print the 'target', so it\nis something like \"cannot lock ref 'refs/heads/branch1': expected symref with\ntarget 'refs/heads/master': but is a regular ref\". I felt here the colon\ndivides the error into three segments\n1. States that we couldn't lock the file\n2. States that we were expecting a symref with a particular target\n3. States that the ref in question is actually a regular ref\n\nHaving said that, I'm not too bullish on this and happy to change it :)\n\n> Other than that this series looks good to me now, thanks!\n>\n> Patrick\n>\n\nThanks for all the reviews. Since this is the only change, I'm inclined\nto leave it as is.\n\nKarthik\n"}]}