update-ref: add support for 'symref-delete' command

Add a new command 'symref-delete' to allow deletions of symbolic refs in
a transaction via the '--stdin' mode of the 'git-update-ref' command.
The 'symref-delete' command can, when given an <old-target>, delete the
provided <ref> only when it points to <old-target>.

This command is only compatible with the 'no-deref' mode because we
optionally want to check the 'old_target' of the ref being deleted.
De-referencing a symbolic ref would provide a regular ref and we already
have the 'delete' command for regular refs.

While users can also use 'git symbolic-ref -d' to delete symbolic refs,
the 'symref-delete' command in 'git-update-ref' allows users to do so
within a transaction, which promises atomicity of the operation and can
be batched with other commands.

When no 'old_target' is provided it can also delete regular refs,
similar to how the 'delete' command can delete symrefs when no 'old_oid'
is provided.

Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 9fe78b350188b3fbf8fd6a1834717e8c45fffb7f..16e02f6979fdabf878b02e2c1b9b6c0bc24479ba 100644 (file)
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -65,6 +65,7 @@ performs all modifications together.  Specify commands of the form:
        create SP <ref> SP <new-oid> LF
        delete SP <ref> [SP <old-oid>] LF
        verify SP <ref> [SP <old-oid>] LF
+       symref-delete SP <ref> [SP <old-target>] LF
        symref-verify SP <ref> [SP <old-target>] LF
        option SP <opt> LF
        start LF
@@ -87,6 +88,7 @@ quoting:
        create SP <ref> NUL <new-oid> NUL
        delete SP <ref> NUL [<old-oid>] NUL
        verify SP <ref> NUL [<old-oid>] NUL
+       symref-delete SP <ref> [NUL <old-target>] NUL
        symref-verify SP <ref> [NUL <old-target>] NUL
        option SP <opt> NUL
        start NUL
@@ -119,6 +121,9 @@ verify::
        Verify <ref> against <old-oid> but do not change it.  If
        <old-oid> is zero or missing, the ref must not exist.
 
+symref-delete::
+       Delete <ref> after verifying it exists with <old-target>, if given.
+
 symref-verify::
        Verify symbolic <ref> against <old-target> but do not change it.
        If <old-target> is missing, the ref must not exist.  Can only be

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 66840b7c5b8c3aa3f5fb0d79ee98ae96754979da..ef286a4a4c340aa0161a491d4b7c3a3ada4222a9 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1382,8 +1382,8 @@ static int prune_refs(struct display_state *display_state,
        if (!dry_run) {
                if (transaction) {
                        for (ref = stale_refs; ref; ref = ref->next) {
-                               result = ref_transaction_delete(transaction, ref->name, NULL, 0,
-                                                               "fetch: prune", &err);
+                               result = ref_transaction_delete(transaction, ref->name, NULL,
+                                                               NULL, 0, "fetch: prune", &err);
                                if (result)
                                        goto cleanup;
                        }

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b150ef39a891106c4e2a5fe0d990ab7201ea0484..64f82612c75b6606a37a02bf776359d420d9525f 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1576,7 +1576,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
                if (ref_transaction_delete(transaction,
                                           namespaced_name,
                                           old_oid,
-                                          0, "push", &err)) {
+                                          NULL, 0,
+                                          "push", &err)) {
                        rp_error("%s", err.buf);
                        ret = "failed to delete";
                } else {

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 6dce1cd663ad189be31d214b047a4ffe0cc66d61..833ebbdd42ed2773320334fc71c5a7729aeb8231 100644 (file)
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -293,7 +293,7 @@ static void parse_cmd_delete(struct ref_transaction *transaction,
 
        if (ref_transaction_delete(transaction, refname,
                                   have_old ? &old_oid : NULL,
-                                  update_flags, msg, &err))
+                                  NULL, update_flags, msg, &err))
                die("%s", err.buf);
 
        update_flags = default_flags;
@@ -301,6 +301,36 @@ static void parse_cmd_delete(struct ref_transaction *transaction,
        strbuf_release(&err);
 }
 
+
+static void parse_cmd_symref_delete(struct ref_transaction *transaction,
+                                   const char *next, const char *end)
+{
+       struct strbuf err = STRBUF_INIT;
+       char *refname, *old_target;
+
+       if (!(update_flags & REF_NO_DEREF))
+               die("symref-delete: cannot operate with deref mode");
+
+       refname = parse_refname(&next);
+       if (!refname)
+               die("symref-delete: missing <ref>");
+
+       old_target = parse_next_refname(&next);
+
+       if (*next != line_termination)
+               die("symref-delete %s: extra input: %s", refname, next);
+
+       if (ref_transaction_delete(transaction, refname, NULL,
+                                  old_target, update_flags, msg, &err))
+               die("%s", err.buf);
+
+       update_flags = default_flags;
+       free(refname);
+       free(old_target);
+       strbuf_release(&err);
+}
+
+
 static void parse_cmd_verify(struct ref_transaction *transaction,
                             const char *next, const char *end)
 {
@@ -443,6 +473,7 @@ static const struct parse_cmd {
        { "create",        parse_cmd_create,        2, UPDATE_REFS_OPEN },
        { "delete",        parse_cmd_delete,        2, UPDATE_REFS_OPEN },
        { "verify",        parse_cmd_verify,        2, UPDATE_REFS_OPEN },
+       { "symref-delete", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },
        { "symref-verify", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },
        { "option",        parse_cmd_option,        1, UPDATE_REFS_OPEN },
        { "start",         parse_cmd_start,         0, UPDATE_REFS_STARTED },

diff --git a/refs.c b/refs.c
index 865264d487e7b5e07ef87912abca69615f0a2323..8114bf65e0ada33ac3eadc74d1e655162426af6e 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -979,7 +979,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
        transaction = ref_store_transaction_begin(refs, &err);
        if (!transaction ||
            ref_transaction_delete(transaction, refname, old_oid,
-                                  flags, msg, &err) ||
+                                  NULL, flags, msg, &err) ||
            ref_transaction_commit(transaction, &err)) {
                error("%s", err.buf);
                ref_transaction_free(transaction);
@@ -1317,14 +1317,20 @@ int ref_transaction_create(struct ref_transaction *transaction,
 int ref_transaction_delete(struct ref_transaction *transaction,
                           const char *refname,
                           const struct object_id *old_oid,
-                          unsigned int flags, const char *msg,
+                          const char *old_target,
+                          unsigned int flags,
+                          const char *msg,
                           struct strbuf *err)
 {
        if (old_oid && is_null_oid(old_oid))
                BUG("delete called with old_oid set to zeros");
+       if (old_oid && old_target)
+               BUG("delete called with both old_oid and old_target set");
+       if (old_target && !(flags & REF_NO_DEREF))
+               BUG("delete cannot operate on symrefs with deref mode");
        return ref_transaction_update(transaction, refname,
                                      null_oid(), old_oid,
-                                     NULL, NULL, flags,
+                                     NULL, old_target, flags,
                                      msg, err);
 }
 
@@ -2767,7 +2773,7 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg,
 
        for_each_string_list_item(item, refnames) {
                ret = ref_transaction_delete(transaction, item->string,
-                                            NULL, flags, msg, &err);
+                                            NULL, NULL, flags, msg, &err);
                if (ret) {
                        warning(_("could not delete reference %s: %s"),
                                item->string, err.buf);

diff --git a/refs.h b/refs.h
index 48cec1ba72d438aff1f769d0324a05b92f0aa1e2..5b84958528620d9687062a862dc0c6d77829a1b4 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -767,7 +767,9 @@ int ref_transaction_create(struct ref_transaction *transaction,
 int ref_transaction_delete(struct ref_transaction *transaction,
                           const char *refname,
                           const struct object_id *old_oid,
-                          unsigned int flags, const char *msg,
+                          const char *old_target,
+                          unsigned int flags,
+                          const char *msg,
                           struct strbuf *err);
 
 /*

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 07e111b063bec85ca6314ed44bb312e769a5ec4e..2ddc586d5d66a17833980eb43f27e7cb875afb76 100755 (executable)
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1729,6 +1729,74 @@ do
                test_cmp expect actual
        '
 
+       test_expect_success "stdin $type symref-delete fails without --no-deref" '
+               git symbolic-ref refs/heads/symref $a &&
+               format_command $type "symref-delete refs/heads/symref" "$a" >stdin &&
+               test_must_fail git update-ref --stdin $type <stdin 2>err &&
+               grep "fatal: symref-delete: cannot operate with deref mode" err
+       '
+
+       test_expect_success "stdin $type symref-delete fails with no ref" '
+               format_command $type "symref-delete " >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
+               grep "fatal: symref-delete: missing <ref>" err
+       '
+
+       test_expect_success "stdin $type symref-delete fails deleting regular ref" '
+               test_when_finished "git update-ref -d refs/heads/regularref" &&
+               git update-ref refs/heads/regularref $a &&
+               format_command $type "symref-delete refs/heads/regularref" "$a" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
+               grep "fatal: cannot lock ref ${SQ}refs/heads/regularref${SQ}: expected symref with target ${SQ}$a${SQ}: but is a regular ref" err
+       '
+
+       test_expect_success "stdin $type symref-delete fails with too many arguments" '
+               format_command $type "symref-delete refs/heads/symref" "$a" "$a" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
+               if test "$type" = "-z"
+               then
+                       grep "fatal: unknown command: $a" err
+               else
+                       grep "fatal: symref-delete refs/heads/symref: extra input:  $a" err
+               fi
+       '
+
+       test_expect_success "stdin $type symref-delete fails with wrong old value" '
+               format_command $type "symref-delete refs/heads/symref" "$m" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
+               grep "fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected refs/heads/main" err &&
+               git symbolic-ref refs/heads/symref >expect &&
+               echo $a >actual &&
+               test_cmp expect actual
+       '
+
+       test_expect_success "stdin $type symref-delete works with right old value" '
+               format_command $type "symref-delete refs/heads/symref" "$a" >stdin &&
+               git update-ref --stdin $type --no-deref <stdin &&
+               test_must_fail git rev-parse --verify -q refs/heads/symref
+       '
+
+       test_expect_success "stdin $type symref-delete works with empty old value" '
+               git symbolic-ref refs/heads/symref $a >stdin &&
+               format_command $type "symref-delete refs/heads/symref" "" >stdin &&
+               git update-ref --stdin $type --no-deref <stdin &&
+               test_must_fail git rev-parse --verify -q $b
+       '
+
+       test_expect_success "stdin $type symref-delete succeeds for dangling reference" '
+               test_must_fail git symbolic-ref refs/heads/nonexistent &&
+               git symbolic-ref refs/heads/symref2 refs/heads/nonexistent &&
+               format_command $type "symref-delete refs/heads/symref2" "refs/heads/nonexistent" >stdin &&
+               git update-ref --stdin $type --no-deref <stdin &&
+               test_must_fail git symbolic-ref -d refs/heads/symref2
+       '
+
+       test_expect_success "stdin $type symref-delete deletes regular ref without target" '
+               git update-ref refs/heads/regularref $a &&
+               format_command $type "symref-delete refs/heads/regularref" >stdin &&
+               git update-ref --stdin $type --no-deref <stdin
+       '
+
 done
 
 test_done

diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index fd58b902f4b511a97c58fe2e4ba5bf22663b9de0..ccde1b944be6f0055e34d17195a1bc21f1e11e3b 100755 (executable)
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -162,6 +162,7 @@ test_expect_success 'hook gets all queued symref updates' '
 
        git update-ref refs/heads/branch $POST_OID &&
        git symbolic-ref refs/heads/symref refs/heads/main &&
+       git symbolic-ref refs/heads/symrefd refs/heads/main &&
 
        test_hook reference-transaction <<-\EOF &&
        echo "$*" >>actual
@@ -171,16 +172,32 @@ test_expect_success 'hook gets all queued symref updates' '
        done >>actual
        EOF
 
-       cat >expect <<-EOF &&
+       # In the files backend, "delete" also triggers an additional transaction
+       # update on the packed-refs backend, which constitutes additional reflog
+       # entries.
+       if test_have_prereq REFFILES
+       then
+               cat >expect <<-EOF
+               aborted
+               $ZERO_OID $ZERO_OID refs/heads/symrefd
+               EOF
+       else
+               >expect
+       fi &&
+
+       cat >>expect <<-EOF &&
        prepared
        ref:refs/heads/main $ZERO_OID refs/heads/symref
+       ref:refs/heads/main $ZERO_OID refs/heads/symrefd
        committed
        ref:refs/heads/main $ZERO_OID refs/heads/symref
+       ref:refs/heads/main $ZERO_OID refs/heads/symrefd
        EOF
 
        git update-ref --no-deref --stdin <<-EOF &&
        start
        symref-verify refs/heads/symref refs/heads/main
+       symref-delete refs/heads/symrefd refs/heads/main
        prepare
        commit
        EOF