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

The 'symref-verify' command allows users to verify if a provided <ref>
contains the provided <old-target> without changing the <ref>. If
<old-target> is not provided, the command will verify that the <ref>
doesn't exist.

The command allows users to verify symbolic refs within a transaction,
and this means users can perform a set of changes in a transaction only
when the verification holds good.

Since we're checking for symbolic refs, this command will only work with
the 'no-deref' mode. This is because any dereferenced symbolic ref will
point to an object and not a ref and the regular 'verify' command can be
used in such situations.

Add required tests for symref support in 'verify'. Since we're here,
also add reflog checks for the pre-existing 'verify' tests, there is no
divergence from behavior, but we never tested to ensure that reflog
wasn't affected by the 'verify' command.

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 374a2ebd2b0bdaa1e0a8cb09e172f27d1fb5fada..9fe78b350188b3fbf8fd6a1834717e8c45fffb7f 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-verify SP <ref> [SP <old-target>] LF
        option SP <opt> LF
        start LF
        prepare LF
@@ -86,6 +87,7 @@ quoting:
        create SP <ref> NUL <new-oid> NUL
        delete SP <ref> NUL [<old-oid>] NUL
        verify SP <ref> NUL [<old-oid>] NUL
+       symref-verify SP <ref> [NUL <old-target>] NUL
        option SP <opt> NUL
        start NUL
        prepare NUL
@@ -117,6 +119,11 @@ verify::
        Verify <ref> against <old-oid> but do not change it.  If
        <old-oid> is zero or missing, the ref must not exist.
 
+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
+       used in `no-deref` mode.
+
 option::
        Modify the behavior of the next command naming a <ref>.
        The only valid option is `no-deref` to avoid dereferencing

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 21fdbf6ac8a4a45138205f98454bc3d4c52ba8d0..6dce1cd663ad189be31d214b047a4ffe0cc66d61 100644 (file)
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -76,6 +76,29 @@ static char *parse_refname(const char **next)
        return strbuf_detach(&ref, NULL);
 }
 
+/*
+ * Wrapper around parse_refname which skips the next delimiter.
+ */
+static char *parse_next_refname(const char **next)
+{
+       if (line_termination) {
+               /* Without -z, consume SP and use next argument */
+               if (!**next || **next == line_termination)
+                       return NULL;
+               if (**next != ' ')
+                       die("expected SP but got: %s", *next);
+       } else {
+               /* With -z, read the next NUL-terminated line */
+               if (**next)
+                       return NULL;
+       }
+       /* Skip the delimiter */
+       (*next)++;
+
+       return parse_refname(next);
+}
+
+
 /*
  * The value being parsed is <old-oid> (as opposed to <new-oid>; the
  * difference affects which error messages are generated):
@@ -297,11 +320,47 @@ static void parse_cmd_verify(struct ref_transaction *transaction,
                die("verify %s: extra input: %s", refname, next);
 
        if (ref_transaction_verify(transaction, refname, &old_oid,
-                                  update_flags, &err))
+                                  NULL, update_flags, &err))
+               die("%s", err.buf);
+
+       update_flags = default_flags;
+       free(refname);
+       strbuf_release(&err);
+}
+
+static void parse_cmd_symref_verify(struct ref_transaction *transaction,
+                                   const char *next, const char *end)
+{
+       struct strbuf err = STRBUF_INIT;
+       struct object_id old_oid;
+       char *refname, *old_target;
+
+       if (!(update_flags & REF_NO_DEREF))
+               die("symref-verify: cannot operate with deref mode");
+
+       refname = parse_refname(&next);
+       if (!refname)
+               die("symref-verify: missing <ref>");
+
+       /*
+        * old_ref is optional, if not provided, we need to ensure that the
+        * ref doesn't exist.
+        */
+       old_target = parse_next_refname(&next);
+       if (!old_target)
+               oidcpy(&old_oid, null_oid());
+
+       if (*next != line_termination)
+               die("symref-verify %s: extra input: %s", refname, next);
+
+       if (ref_transaction_verify(transaction, refname,
+                                  old_target ? NULL : &old_oid,
+                                  old_target, update_flags, &err))
                die("%s", err.buf);
 
        update_flags = default_flags;
        free(refname);
+       free(old_target);
        strbuf_release(&err);
 }
 
@@ -380,15 +439,16 @@ static const struct parse_cmd {
        unsigned args;
        enum update_refs_state state;
 } command[] = {
-       { "update",  parse_cmd_update,  3, UPDATE_REFS_OPEN },
-       { "create",  parse_cmd_create,  2, UPDATE_REFS_OPEN },
-       { "delete",  parse_cmd_delete,  2, UPDATE_REFS_OPEN },
-       { "verify",  parse_cmd_verify,  2, UPDATE_REFS_OPEN },
-       { "option",  parse_cmd_option,  1, UPDATE_REFS_OPEN },
-       { "start",   parse_cmd_start,   0, UPDATE_REFS_STARTED },
-       { "prepare", parse_cmd_prepare, 0, UPDATE_REFS_PREPARED },
-       { "abort",   parse_cmd_abort,   0, UPDATE_REFS_CLOSED },
-       { "commit",  parse_cmd_commit,  0, UPDATE_REFS_CLOSED },
+       { "update",        parse_cmd_update,        3, UPDATE_REFS_OPEN },
+       { "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-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 },
+       { "prepare",       parse_cmd_prepare,       0, UPDATE_REFS_PREPARED },
+       { "abort",         parse_cmd_abort,         0, UPDATE_REFS_CLOSED },
+       { "commit",        parse_cmd_commit,        0, UPDATE_REFS_CLOSED },
 };
 
 static void update_refs_stdin(void)

diff --git a/refs.c b/refs.c
index 00e87f8ee3bf8a69d74ed8da89e405968f5b264a..865264d487e7b5e07ef87912abca69615f0a2323 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1331,14 +1331,19 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 int ref_transaction_verify(struct ref_transaction *transaction,
                           const char *refname,
                           const struct object_id *old_oid,
+                          const char *old_target,
                           unsigned int flags,
                           struct strbuf *err)
 {
-       if (!old_oid)
-               BUG("verify called with old_oid set to NULL");
+       if (!old_target && !old_oid)
+               BUG("verify called with old_oid and old_target set to NULL");
+       if (old_oid && old_target)
+               BUG("verify called with both old_oid and old_target set");
+       if (old_target && !(flags & REF_NO_DEREF))
+               BUG("verify cannot operate on symrefs with deref mode");
        return ref_transaction_update(transaction, refname,
                                      NULL, old_oid,
-                                     NULL, NULL,
+                                     NULL, old_target,
                                      flags, NULL, err);
 }
 

diff --git a/refs.h b/refs.h
index 71cc1c58e000b963ea6f8538a28ed3e1a88f12b3..48cec1ba72d438aff1f769d0324a05b92f0aa1e2 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -781,6 +781,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 int ref_transaction_verify(struct ref_transaction *transaction,
                           const char *refname,
                           const struct object_id *old_oid,
+                          const char *old_target,
                           unsigned int flags,
                           struct strbuf *err);
 

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index ec3443cc8786d126cbc0085de184b2e9af019566..07e111b063bec85ca6314ed44bb312e769a5ec4e 100755 (executable)
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -890,17 +890,23 @@ test_expect_success 'stdin update/create/verify combination works' '
 '
 
 test_expect_success 'stdin verify succeeds for correct value' '
+       test-tool ref-store main for-each-reflog-ent $m >before &&
        git rev-parse $m >expect &&
        echo "verify $m $m" >stdin &&
        git update-ref --stdin <stdin &&
        git rev-parse $m >actual &&
-       test_cmp expect actual
+       test_cmp expect actual &&
+       test-tool ref-store main for-each-reflog-ent $m >after &&
+       test_cmp before after
 '
 
 test_expect_success 'stdin verify succeeds for missing reference' '
+       test-tool ref-store main for-each-reflog-ent $m >before &&
        echo "verify refs/heads/missing $Z" >stdin &&
        git update-ref --stdin <stdin &&
-       test_must_fail git rev-parse --verify -q refs/heads/missing
+       test_must_fail git rev-parse --verify -q refs/heads/missing &&
+       test-tool ref-store main for-each-reflog-ent $m >after &&
+       test_cmp before after
 '
 
 test_expect_success 'stdin verify treats no value as missing' '
@@ -1641,4 +1647,88 @@ test_expect_success PIPE 'transaction flushes status updates' '
        test_cmp expected actual
 '
 
+format_command () {
+       if test "$1" = "-z"
+       then
+               shift
+               printf "$F" "$@"
+       else
+               echo "$@"
+       fi
+}
+
+for type in "" "-z"
+do
+
+       test_expect_success "stdin $type symref-verify fails without --no-deref" '
+               git symbolic-ref refs/heads/symref $a &&
+               format_command $type "symref-verify refs/heads/symref" "$a" >stdin &&
+               test_must_fail git update-ref --stdin $type <stdin 2>err &&
+               grep "fatal: symref-verify: cannot operate with deref mode" err
+       '
+
+       test_expect_success "stdin $type symref-verify fails with too many arguments" '
+               format_command $type "symref-verify 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-verify refs/heads/symref: extra input:  $a" err
+               fi
+       '
+
+       test_expect_success "stdin $type symref-verify succeeds for correct value" '
+               git symbolic-ref refs/heads/symref >expect &&
+               test-tool ref-store main for-each-reflog-ent refs/heads/symref >before &&
+               format_command $type "symref-verify refs/heads/symref" "$a" >stdin &&
+               git update-ref --stdin $type --no-deref <stdin &&
+               git symbolic-ref refs/heads/symref >actual &&
+               test_cmp expect actual &&
+               test-tool ref-store main for-each-reflog-ent refs/heads/symref >after &&
+               test_cmp before after
+       '
+
+       test_expect_success "stdin $type symref-verify fails with no value" '
+               git symbolic-ref refs/heads/symref >expect &&
+               format_command $type "symref-verify refs/heads/symref" "" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin
+       '
+
+       test_expect_success "stdin $type symref-verify succeeds for dangling reference" '
+               test_when_finished "git symbolic-ref -d refs/heads/symref2" &&
+               test_must_fail git symbolic-ref refs/heads/nonexistent &&
+               git symbolic-ref refs/heads/symref2 refs/heads/nonexistent &&
+               format_command $type "symref-verify refs/heads/symref2" "refs/heads/nonexistent" >stdin &&
+               git update-ref --stdin $type --no-deref <stdin
+       '
+
+       test_expect_success "stdin $type symref-verify fails for missing reference" '
+               test-tool ref-store main for-each-reflog-ent refs/heads/symref >before &&
+               format_command $type "symref-verify refs/heads/missing" "refs/heads/unknown" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
+               grep "fatal: cannot lock ref ${SQ}refs/heads/missing${SQ}: unable to resolve reference ${SQ}refs/heads/missing${SQ}" err &&
+               test_must_fail git rev-parse --verify -q refs/heads/missing &&
+               test-tool ref-store main for-each-reflog-ent refs/heads/symref >after &&
+               test_cmp before after
+       '
+
+       test_expect_success "stdin $type symref-verify fails for wrong value" '
+               git symbolic-ref refs/heads/symref >expect &&
+               format_command $type "symref-verify refs/heads/symref" "$b" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin &&
+               git symbolic-ref refs/heads/symref >actual &&
+               test_cmp expect actual
+       '
+
+       test_expect_success "stdin $type symref-verify fails for mistaken null value" '
+               git symbolic-ref refs/heads/symref >expect &&
+               format_command $type "symref-verify refs/heads/symref" "$Z" >stdin &&
+               test_must_fail git update-ref --stdin $type --no-deref <stdin &&
+               git symbolic-ref refs/heads/symref >actual &&
+               test_cmp expect actual
+       '
+
+done
+
 test_done

diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 067fd57290c375a90cee03eed02c08d99d9aed68..fd58b902f4b511a97c58fe2e4ba5bf22663b9de0 100755 (executable)
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -157,4 +157,34 @@ test_expect_success 'hook captures git-symbolic-ref updates' '
        test_cmp expect actual
 '
 
+test_expect_success 'hook gets all queued symref updates' '
+       test_when_finished "rm actual" &&
+
+       git update-ref refs/heads/branch $POST_OID &&
+       git symbolic-ref refs/heads/symref refs/heads/main &&
+
+       test_hook reference-transaction <<-\EOF &&
+       echo "$*" >>actual
+       while read -r line
+       do
+               printf "%s\n" "$line"
+       done >>actual
+       EOF
+
+       cat >expect <<-EOF &&
+       prepared
+       ref:refs/heads/main $ZERO_OID refs/heads/symref
+       committed
+       ref:refs/heads/main $ZERO_OID refs/heads/symref
+       EOF
+
+       git update-ref --no-deref --stdin <<-EOF &&
+       start
+       symref-verify refs/heads/symref refs/heads/main
+       prepare
+       commit
+       EOF
+       test_cmp expect actual
+'
+
 test_done