From: Karthik Nayak <karthik.188@xxxxxxxxx>
In the previous commits, we added the required base for adding symref
commands to the '--stdin' mode provided by 'git-update-ref(1)'. Using
them, add a new 'symref-verify' command to verify symrefs.
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. 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 and use `ref_update_is_null_new_value`, a helper function which is
used to check if there is a new_value in a reference update. The new
value could either be a symref target `new_target` or a OID `new_oid`.
We also add tests to test the command in both the regular stdin mode and
also with the '-z' flag.
We also disable the reference-transaction hook for symref-updates which
will be tackled in its own commit.
Add required tests for symref support in 'verify' while also adding
reflog checks for the pre-existing 'verify' tests.
Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
---
Documentation/git-update-ref.txt | 7 +++
builtin/update-ref.c | 80 +++++++++++++++++++++++----
refs.c | 30 +++++++++--
refs.h | 1 +
refs/files-backend.c | 43 +++++++++++++++
refs/refs-internal.h | 7 +++
refs/reftable-backend.c | 21 +++++++-
t/t1400-update-ref.sh | 93 +++++++++++++++++++++++++++++++-
8 files changed, 264 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 374a2ebd2b..9fe78b3501 100644
--- 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 21fdbf6ac8..419b28169b 100644
--- 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, but we want to differentiate between
+ * a NULL and zero value.
+ */
+ old_target = parse_next_refname(&next);
+ if (!old_target)
+ 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 060a31616d..0e1013b5ab 100644
--- a/refs.c
+++ b/refs.c
@@ -1217,6 +1217,8 @@ void ref_transaction_free(struct ref_transaction *transaction)
for (i = 0; i < transaction->nr; i++) {
free(transaction->updates[i]->msg);
+ free((void *)transaction->updates[i]->old_target);
+ free((void *)transaction->updates[i]->new_target);
free(transaction->updates[i]);
}
free(transaction->updates);
@@ -1247,9 +1249,13 @@ struct ref_update *ref_transaction_add_update(
update->flags = flags;
- if (flags & REF_HAVE_NEW)
+ if (new_target)
+ update->new_target = xstrdup(new_target);
+ if (old_target)
+ update->old_target = xstrdup(old_target);
+ if (new_oid && flags & REF_HAVE_NEW)
oidcpy(&update->new_oid, new_oid);
- if (flags & REF_HAVE_OLD)
+ if (old_oid && flags & REF_HAVE_OLD)
oidcpy(&update->old_oid, old_oid);
update->msg = normalize_reflog_message(msg);
return update;
@@ -1286,6 +1292,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
flags &= REF_TRANSACTION_UPDATE_ALLOWED_FLAGS;
flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
+ flags |= (new_target ? REF_HAVE_NEW : 0) | (old_target ? REF_HAVE_OLD : 0);
ref_transaction_add_update(transaction, refname, flags,
new_oid, old_oid, new_target,
@@ -1325,14 +1332,17 @@ 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_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);
}
@@ -2349,6 +2359,12 @@ static int run_transaction_hook(struct ref_transaction *transaction,
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];
+ /*
+ * Skip reference transaction for symbolic refs.
+ */
+ if (update->new_target || update->old_target)
+ continue;
+
strbuf_reset(&buf);
strbuf_addf(&buf, "%s %s %s\n",
oid_to_hex(&update->old_oid),
@@ -2802,3 +2818,7 @@ int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg
{
return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
}
+
+int ref_update_is_null_new_value(struct ref_update *update) {
+ return !update->new_target && is_null_oid(&update->new_oid);
+}
diff --git a/refs.h b/refs.h
index c792e13a64..27b9aeaf54 100644
--- a/refs.h
+++ b/refs.h
@@ -780,6 +780,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/refs/files-backend.c b/refs/files-backend.c
index 2420dac2aa..53197fa3af 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2425,6 +2425,37 @@ static const char *original_update_refname(struct ref_update *update)
return update->refname;
}
+/*
+ * Check whether the REF_HAVE_OLD and old_target values stored in
+ * update are consistent with ref, which is the symbolic reference's
+ * current value. If everything is OK, return 0; otherwise, write an
+ * error message to err and return -1.
+ */
+static int check_old_target(struct ref_update *update, char *ref,
+ struct strbuf *err)
+{
+ if (!(update->flags & REF_HAVE_OLD) ||
+ !strcmp(update->old_target, ref))
+ return 0;
+
+ if (!strcmp(update->old_target, ""))
+ strbuf_addf(err, "cannot lock ref '%s': "
+ "reference already exists",
+ original_update_refname(update));
+ else if (!strcmp(ref, ""))
+ strbuf_addf(err, "cannot lock ref '%s': "
+ "reference is missing but expected %s",
+ original_update_refname(update),
+ update->old_target);
+ else
+ strbuf_addf(err, "cannot lock ref '%s': "
+ "is at %s but expected %s",
+ original_update_refname(update),
+ ref, update->old_target);
+
+ return -1;
+}
+
/*
* Check whether the REF_HAVE_OLD and old_oid values stored in update
* are consistent with oid, which is the reference's current value. If
@@ -2528,6 +2559,18 @@ static int lock_ref_for_update(struct files_ref_store *refs,
ret = TRANSACTION_GENERIC_ERROR;
goto out;
}
+ }
+
+ /*
+ * For symref verification, we need to check the reference value
+ * rather than the oid. If we're dealing with regular refs or we're
+ * verifying a dereferenced symref, we then check the oid.
+ */
+ if (update->old_target) {
+ if (check_old_target(update, referent.buf, err)) {
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto out;
+ }
} else if (check_old_oid(update, &lock->old_oid, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto out;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3040d4797c..23e65f65e8 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -748,4 +748,11 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
*/
struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_store *store);
+/*
+ * Helper function to check if the new value is null, this
+ * takes into consideration that the update could be a regular
+ * ref or a symbolic ref.
+ */
+int ref_update_is_null_new_value(struct ref_update *update);
+
#endif /* REFS_REFS_INTERNAL_H */
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 6104471199..a2474245aa 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -938,7 +938,26 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
* individual refs. But the error messages match what the files
* backend returns, which keeps our tests happy.
*/
- if (u->flags & REF_HAVE_OLD && !oideq(¤t_oid, &u->old_oid)) {
+ if ((u->flags & REF_HAVE_OLD) && u->old_target) {
+ if (strcmp(referent.buf, u->old_target)) {
+ if (!strcmp(u->old_target, ""))
+ strbuf_addf(err, "verifying symref target: '%s': "
+ "provided target is empty",
+ original_update_refname(u));
+ else if (!strcmp(referent.buf, ""))
+ strbuf_addf(err, "verifying symref target: '%s': "
+ "reference is missing but expected %s",
+ original_update_refname(u),
+ u->old_target);
+ else
+ strbuf_addf(err, "verifying symref target: '%s': "
+ "is at %s but expected %s",
+ original_update_refname(u),
+ referent.buf, u->old_target);
+ ret = -1;
+ goto done;
+ }
+ } else if (u->flags & REF_HAVE_OLD && !oideq(¤t_oid, &u->old_oid)) {
if (is_null_oid(&u->old_oid))
strbuf_addf(err, _("cannot lock ref '%s': "
"reference already exists"),
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index ec3443cc87..34b29eeac8 100755
--- 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,87 @@ test_expect_success PIPE 'transaction flushes status updates' '
test_cmp expected actual
'
+create_stdin_buf () {
+ if test "$1" = "-z"
+ then
+ shift
+ printf "$F" "$@" >stdin
+ else
+ echo "$@" >stdin
+ fi
+}
+
+for type in "" "-z"
+do
+
+ test_expect_success "stdin ${type} symref-verify fails without --no-deref" '
+ git symbolic-ref refs/heads/symref $a &&
+ create_stdin_buf ${type} "symref-verify refs/heads/symref" "$a" &&
+ 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" '
+ create_stdin_buf ${type} "symref-verify refs/heads/symref" "$a" "$a" &&
+ 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 &&
+ create_stdin_buf ${type} "symref-verify refs/heads/symref" "$a" &&
+ 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 no value is treated as zero value" '
+ git symbolic-ref refs/heads/symref >expect &&
+ create_stdin_buf ${type} "symref-verify refs/heads/symref" "" &&
+ 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 &&
+ create_stdin_buf ${type} "symref-verify refs/heads/symref2" "refs/heads/nonexistent" &&
+ git update-ref --stdin ${type} --no-deref <stdin
+ '
+
+ test_expect_success "stdin ${type} symref-verify succeeds for missing reference" '
+ test-tool ref-store main for-each-reflog-ent refs/heads/symref >before &&
+ create_stdin_buf ${type} "symref-verify refs/heads/missing" "$Z" &&
+ git update-ref --stdin ${type} --no-deref <stdin &&
+ 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 &&
+ create_stdin_buf ${type} "symref-verify refs/heads/symref" "$b" &&
+ 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 &&
+ create_stdin_buf ${type} "symref-verify refs/heads/symref" "$Z" &&
+ 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
--
2.43.GIT
