diff mbox series

[v2,6/7] update-ref: add support for symref-update

Message ID 20240412095908.1134387-7-knayak@gitlab.com (mailing list archive)
State New
Headers show
Series update-ref: add symref oriented commands | expand

Commit Message

Karthik Nayak April 12, 2024, 9:59 a.m. UTC 
From: Karthik Nayak <karthik.188@gmail.com>

Add 'symref-update' to allow updates of symbolic refs in a transaction
via the 'git-update-ref' command. The 'symref-update' command takes in a
<new-ref>, which the <ref> will be updated to. If the <ref> doesn't
exist it will be created.

It also optionally takes either an <old-ref> or <old-oid>. If the
<old-ref> is provided, it checks to see if the <ref> ponints to the
<old-ref> before the update. If <old-oid> is provided it checks <ref> to
ensure that it is a regular ref and <old-oid> is the OID before the
update. This by extension also means that this when a zero <old-oid> is
provided, it ensures that the ref didn't exist before.

This command will also support deref mode, to ensure that we can update
dereferenced regular refs to symrefs.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-update-ref.txt |   6 ++
 builtin/update-ref.c             |  49 +++++++++++
 refs.c                           |  24 ++----
 refs/files-backend.c             |  15 ++--
 refs/reftable-backend.c          |   7 +-
 t/t1400-update-ref.sh            | 143 +++++++++++++++++++++++++++++++
 6 files changed, 220 insertions(+), 24 deletions(-)

Comments

Patrick Steinhardt April 19, 2024, 9:40 a.m. UTC | #1 
On Fri, Apr 12, 2024 at 11:59:07AM +0200, Karthik Nayak wrote:
> From: Karthik Nayak <karthik.188@gmail.com>
> 
> Add 'symref-update' to allow updates of symbolic refs in a transaction
> via the 'git-update-ref' command. The 'symref-update' command takes in a
> <new-ref>, which the <ref> will be updated to. If the <ref> doesn't
> exist it will be created.
> 
> It also optionally takes either an <old-ref> or <old-oid>. If the
> <old-ref> is provided, it checks to see if the <ref> ponints to the
> <old-ref> before the update. If <old-oid> is provided it checks <ref> to
> ensure that it is a regular ref and <old-oid> is the OID before the
> update. This by extension also means that this when a zero <old-oid> is
> provided, it ensures that the ref didn't exist before.
> 
> This command will also support deref mode, to ensure that we can update
> dereferenced regular refs to symrefs.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git-update-ref.txt |   6 ++
>  builtin/update-ref.c             |  49 +++++++++++
>  refs.c                           |  24 ++----
>  refs/files-backend.c             |  15 ++--
>  refs/reftable-backend.c          |   7 +-
>  t/t1400-update-ref.sh            | 143 +++++++++++++++++++++++++++++++
>  6 files changed, 220 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
> index a5b1f42728..9710c9bc78 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-update SP <ref> SP <new-ref> [SP (<old-ref> | <old-oid>)] LF
>  	symref-create SP <ref> SP <new-ref> LF
>  	symref-delete SP <ref> [SP <old-ref>] LF
>  	symref-verify SP <ref> [SP <old-ref>] LF
> @@ -89,6 +90,7 @@ quoting:
>  	create SP <ref> NUL <new-oid> NUL
>  	delete SP <ref> NUL [<old-oid>] NUL
>  	verify SP <ref> NUL [<old-oid>] NUL
> +	symref-update SP <ref> NUL <new-ref> [NUL (<old-ref> | <old-oid>)] NUL
>  	symref-create SP <ref> NUL <new-ref> NUL
>  	symref-delete SP <ref> [NUL <old-ref>] NUL
>  	symref-verify SP <ref> [NUL <old-ref>] NUL
> @@ -123,6 +125,10 @@ verify::
>  	Verify <ref> against <old-oid> but do not change it.  If
>  	<old-oid> is zero or missing, the ref must not exist.
>  
> +symref-update::
> +	Set <ref> to <new-ref> after verifying <old-ref> or <old-oid>,
> +	if given. Can be used to delete or create symrefs too.
> +
>  symref-create::
>  	Create symbolic ref <ref> with <new-ref> after verifying
>  	it does not exist.  Can only be used in `no-deref` mode.
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 24556a28a8..809c1c7a76 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -238,6 +238,54 @@ static void parse_cmd_update(struct ref_transaction *transaction,
>  	strbuf_release(&err);
>  }
>  
> +static void parse_cmd_symref_update(struct ref_transaction *transaction,
> +				    const char *next, const char *end)
> +{
> +	struct strbuf err = STRBUF_INIT;
> +	char *refname, *new_ref, *old_ref;
> +	struct object_id old_oid;
> +	int have_old = 0;
> +
> +	refname = parse_refname(&next);
> +	if (!refname)
> +		die("symref-update: missing <ref>");
> +
> +	new_ref = parse_next_refname(&next);
> +	if (!new_ref)
> +		die("symref-update %s: missing <new-ref>", refname);
> +	if (read_ref(new_ref, NULL))
> +		die("symref-update %s: invalid <new-ref>", refname);
> +
> +	old_ref = parse_next_refname(&next);
> +	/*
> +	 * Since the user can also send in an old-oid, we try to parse
> +	 * it as such too.
> +	 */
> +	if (old_ref && read_ref(old_ref, NULL)) {
> +		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
> +			old_ref = NULL;
> +			have_old = 1;
> +		} else
> +			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
> +	}

So we first try to parse it as a ref, and then as an object ID? Wouldn't
it preferable to try it the other way round and first check whether it
is a valid object ID? That would likely be cheaper, even though it may
be premature optimization.

Patrick
Karthik Nayak April 21, 2024, 7 p.m. UTC | #2 
Patrick Steinhardt <ps@pks.im> writes:
[snip]
>> +	/*
>> +	 * Since the user can also send in an old-oid, we try to parse
>> +	 * it as such too.
>> +	 */
>> +	if (old_ref && read_ref(old_ref, NULL)) {
>> +		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
>> +			old_ref = NULL;
>> +			have_old = 1;
>> +		} else
>> +			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
>> +	}
>
> So we first try to parse it as a ref, and then as an object ID? Wouldn't
> it preferable to try it the other way round and first check whether it
> is a valid object ID? That would likely be cheaper, even though it may
> be premature optimization.
>
> Patrick

I think the issue is `repo_get_oid` would also parse a refname to an
OID. Whereas we want to first check and keep refnames and only if it
isn't a refname, we'd want to parse it as an OID.

Also, why do you say it is cheaper?
Patrick Steinhardt April 23, 2024, 6:49 a.m. UTC | #3 
On Sun, Apr 21, 2024 at 03:00:11PM -0400, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> [snip]
> >> +	/*
> >> +	 * Since the user can also send in an old-oid, we try to parse
> >> +	 * it as such too.
> >> +	 */
> >> +	if (old_ref && read_ref(old_ref, NULL)) {
> >> +		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
> >> +			old_ref = NULL;
> >> +			have_old = 1;
> >> +		} else
> >> +			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
> >> +	}
> >
> > So we first try to parse it as a ref, and then as an object ID? Wouldn't
> > it preferable to try it the other way round and first check whether it
> > is a valid object ID? That would likely be cheaper, even though it may
> > be premature optimization.
> >
> > Patrick
> 
> I think the issue is `repo_get_oid` would also parse a refname to an
> OID. Whereas we want to first check and keep refnames and only if it
> isn't a refname, we'd want to parse it as an OID.

Okay. The question is whether this matches precedence rules that we have
in other places. Namely, whether a reference name that looks like an
object ID overrides an object with the same name. Testing it:

```
$ rm -rf repo/
$ git init --ref-format=files repo
Initialized empty Git repository in /tmp/repo/.git/
$ cd repo/
$ git commit --allow-empty --message first
[main (root-commit) 09293d8] first
$ git commit --allow-empty --message second
[main 1588e76] second

$ git update-ref $(git rev-parse HEAD~) HEAD
$ cat .git/09293d82c434cdc1f7f286cf7b90cf35a6e57c43
1588e76ce7ef1ab25ee6f846a7b5d7032f83a69e

$ git rev-parse 09293d82c434cdc1f7f286cf7b90cf35a6e57c43
warning: refname '09293d82c434cdc1f7f286cf7b90cf35a6e57c43' is ambiguous.
Git normally never creates a ref that ends with 40 hex characters
because it will be ignored when you just specify 40-hex. These refs
may be created by mistake. For example,

  git switch -c $br $(git rev-parse ...)

where "$br" is somehow empty and a 40-hex ref is created. Please
examine these refs and maybe delete them. Turn this message off by
running "git config advice.objectNameWarning false"
09293d82c434cdc1f7f286cf7b90cf35a6e57c43
```

So the object ID has precedence over the reference with the same name.
Unless I'm mistaken, your proposed order would reverse that, wouldn't
it?

> Also, why do you say it is cheaper?

Checking whether a string can be parsed as an object ID should be faster
than having to ask the ref backend whether it knows any reference with
that name. So it should be fast for `repo_get_oid()` to bail out in case
the provided string doesn't look like an object ID.

Patrick
Karthik Nayak April 23, 2024, 11:30 a.m. UTC | #4 
Patrick Steinhardt <ps@pks.im> writes:

> On Sun, Apr 21, 2024 at 03:00:11PM -0400, Karthik Nayak wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> [snip]
>> >> +	/*
>> >> +	 * Since the user can also send in an old-oid, we try to parse
>> >> +	 * it as such too.
>> >> +	 */
>> >> +	if (old_ref && read_ref(old_ref, NULL)) {
>> >> +		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
>> >> +			old_ref = NULL;
>> >> +			have_old = 1;
>> >> +		} else
>> >> +			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
>> >> +	}
>> >
>> > So we first try to parse it as a ref, and then as an object ID? Wouldn't
>> > it preferable to try it the other way round and first check whether it
>> > is a valid object ID? That would likely be cheaper, even though it may
>> > be premature optimization.
>> >
>> > Patrick
>>
>> I think the issue is `repo_get_oid` would also parse a refname to an
>> OID. Whereas we want to first check and keep refnames and only if it
>> isn't a refname, we'd want to parse it as an OID.
>
> Okay. The question is whether this matches precedence rules that we have
> in other places. Namely, whether a reference name that looks like an
> object ID overrides an object with the same name. Testing it:
>
> ```
> $ rm -rf repo/
> $ git init --ref-format=files repo
> Initialized empty Git repository in /tmp/repo/.git/
> $ cd repo/
> $ git commit --allow-empty --message first
> [main (root-commit) 09293d8] first
> $ git commit --allow-empty --message second
> [main 1588e76] second
>
> $ git update-ref $(git rev-parse HEAD~) HEAD
> $ cat .git/09293d82c434cdc1f7f286cf7b90cf35a6e57c43
> 1588e76ce7ef1ab25ee6f846a7b5d7032f83a69e
>
> $ git rev-parse 09293d82c434cdc1f7f286cf7b90cf35a6e57c43
> warning: refname '09293d82c434cdc1f7f286cf7b90cf35a6e57c43' is ambiguous.
> Git normally never creates a ref that ends with 40 hex characters
> because it will be ignored when you just specify 40-hex. These refs
> may be created by mistake. For example,
>
>   git switch -c $br $(git rev-parse ...)
>
> where "$br" is somehow empty and a 40-hex ref is created. Please
> examine these refs and maybe delete them. Turn this message off by
> running "git config advice.objectNameWarning false"
> 09293d82c434cdc1f7f286cf7b90cf35a6e57c43
> ```
>
> So the object ID has precedence over the reference with the same name.
> Unless I'm mistaken, your proposed order would reverse that, wouldn't
> it?
>

I wasn't talking about a reference being mistaken for a object ID,
rather I was talking about how a reference will be `rev-parse`'d into an
object ID.

So instead if we did something like:

```
if (old_target) {
	if (!repo_get_oid(the_repository, old_target, &old_oid)) {
		old_target = NULL;
		have_old = 1;
	} else if (read_ref(old_target, NULL)) {
	} else {
		die("symref-update %s: invalid <old-target> or <old-oid>", refname);
	}
}
```

The problem is that now:

$ git init repo && cd repo
$ git commit --allow-empty -m"c1"
[master (root-commit) af416de] c1
$ git commit --allow-empty -m"c2"
[master 52e95b2] c2
$ git symbolic-ref refs/heads/symref refs/heads/master
$ git branch b1 master~1
$ git branch b2 master
$ cat .git/refs/heads/symref
ref: refs/heads/master
$ git update-ref --no-deref --stdin
symref-update refs/heads/symref refs/heads/b1 refs/heads/b2
$ cat .git/refs/heads/symref
ref: refs/heads/b1

This shouldn't have worked because symref was pointing to master, but
this did work, because `refs/heads/b2` was rev-parse'd to '52e95b2'
which is also what master is at.

The issue is that we call 'repo_get_oid', which takes in a commitish and
this could even be a reference. But ideally in 'symref' commands we want
to first treat refnames as refnames and not have them parsed as an OID.

Since, I'm not moving to 'refs:<target>' with the existing commands.
This is no longer an issue.

>> Also, why do you say it is cheaper?
>
> Checking whether a string can be parsed as an object ID should be faster
> than having to ask the ref backend whether it knows any reference with
> that name. So it should be fast for `repo_get_oid()` to bail out in case
> the provided string doesn't look like an object ID.
>
> Patrick

Yes. But for a valid refname, the `repo_get_oid()` is querying the
reference backend similar to `read_ref()`. Also `read_ref` also does
initial checks with `check_refname_format` which I think doesn't check
the ref backend.

Thanks
diff mbox series

Patch

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index a5b1f42728..9710c9bc78 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-update SP <ref> SP <new-ref> [SP (<old-ref> | <old-oid>)] LF
 	symref-create SP <ref> SP <new-ref> LF
 	symref-delete SP <ref> [SP <old-ref>] LF
 	symref-verify SP <ref> [SP <old-ref>] LF
@@ -89,6 +90,7 @@  quoting:
 	create SP <ref> NUL <new-oid> NUL
 	delete SP <ref> NUL [<old-oid>] NUL
 	verify SP <ref> NUL [<old-oid>] NUL
+	symref-update SP <ref> NUL <new-ref> [NUL (<old-ref> | <old-oid>)] NUL
 	symref-create SP <ref> NUL <new-ref> NUL
 	symref-delete SP <ref> [NUL <old-ref>] NUL
 	symref-verify SP <ref> [NUL <old-ref>] NUL
@@ -123,6 +125,10 @@  verify::
 	Verify <ref> against <old-oid> but do not change it.  If
 	<old-oid> is zero or missing, the ref must not exist.
 
+symref-update::
+	Set <ref> to <new-ref> after verifying <old-ref> or <old-oid>,
+	if given. Can be used to delete or create symrefs too.
+
 symref-create::
 	Create symbolic ref <ref> with <new-ref> after verifying
 	it does not exist.  Can only be used in `no-deref` mode.
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 24556a28a8..809c1c7a76 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -238,6 +238,54 @@  static void parse_cmd_update(struct ref_transaction *transaction,
 	strbuf_release(&err);
 }
 
+static void parse_cmd_symref_update(struct ref_transaction *transaction,
+				    const char *next, const char *end)
+{
+	struct strbuf err = STRBUF_INIT;
+	char *refname, *new_ref, *old_ref;
+	struct object_id old_oid;
+	int have_old = 0;
+
+	refname = parse_refname(&next);
+	if (!refname)
+		die("symref-update: missing <ref>");
+
+	new_ref = parse_next_refname(&next);
+	if (!new_ref)
+		die("symref-update %s: missing <new-ref>", refname);
+	if (read_ref(new_ref, NULL))
+		die("symref-update %s: invalid <new-ref>", refname);
+
+	old_ref = parse_next_refname(&next);
+	/*
+	 * Since the user can also send in an old-oid, we try to parse
+	 * it as such too.
+	 */
+	if (old_ref && read_ref(old_ref, NULL)) {
+		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
+			old_ref = NULL;
+			have_old = 1;
+		} else
+			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
+	}
+
+	if (*next != line_termination)
+		die("symref-update %s: extra input: %s", refname, next);
+
+	update_flags |= create_reflog_flag | REF_SYMREF_UPDATE;
+	if (ref_transaction_update(transaction, refname, NULL,
+				   have_old ? &old_oid : NULL,
+				   new_ref, old_ref, update_flags,
+				   msg, &err))
+		die("%s", err.buf);
+
+	update_flags = default_flags;
+	free(refname);
+	free(old_ref);
+	free(new_ref);
+	strbuf_release(&err);
+}
+
 static void parse_cmd_create(struct ref_transaction *transaction,
 			     const char *next, const char *end)
 {
@@ -509,6 +557,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-update", parse_cmd_symref_update, 3, UPDATE_REFS_OPEN },
 	{ "symref-create", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },
 	{ "symref-delete", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },
 	{ "symref-verify", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },
diff --git a/refs.c b/refs.c
index e62c0f4aca..31c09c3317 100644
--- a/refs.c
+++ b/refs.c
@@ -1246,21 +1246,15 @@  struct ref_update *ref_transaction_add_update(
 
 	update->flags = flags;
 
-	/*
-	 * The ref values are to be considered over the oid values when we're
-	 * doing symref operations.
-	 */
-	if (update->flags & REF_SYMREF_UPDATE) {
-		if (old_ref)
-			update->old_ref = xstrdup(old_ref);
-		if (new_ref)
-			update->new_ref = xstrdup(new_ref);
-	} else {
-		if (flags & REF_HAVE_NEW)
-			oidcpy(&update->new_oid, new_oid);
-		if (flags & REF_HAVE_OLD)
-			oidcpy(&update->old_oid, old_oid);
-	}
+	if (old_ref)
+		update->old_ref = xstrdup(old_ref);
+	if (new_ref)
+		update->new_ref = xstrdup(new_ref);
+	if (new_oid && flags & REF_HAVE_NEW)
+		oidcpy(&update->new_oid, new_oid);
+	if (old_oid && flags & REF_HAVE_OLD)
+		oidcpy(&update->old_oid, old_oid);
+
 	update->msg = normalize_reflog_message(msg);
 	return update;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 59d438878a..fb9886484c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2386,7 +2386,7 @@  static int split_symref_update(struct ref_update *update,
 	new_update = ref_transaction_add_update(
 			transaction, referent, new_flags,
 			&update->new_oid, &update->old_oid,
-			NULL, NULL, update->msg);
+			update->new_ref, update->old_ref, update->msg);
 
 	new_update->parent_update = update;
 
@@ -2609,7 +2609,9 @@  static int lock_ref_for_update(struct files_ref_store *refs,
 		}
 	}
 
-	if (update->flags & REF_SYMREF_UPDATE && update->new_ref) {
+	if (update->flags & REF_SYMREF_UPDATE &&
+	    !(update->flags & REF_LOG_ONLY) &&
+	    update->new_ref) {
 		if (create_symref_lock(refs, lock, update->refname, update->new_ref)) {
 			ret = TRANSACTION_GENERIC_ERROR;
 			goto out;
@@ -2627,12 +2629,9 @@  static int lock_ref_for_update(struct files_ref_store *refs,
 		 * phase of the transaction only needs to commit the lock.
 		 */
 		update->flags |= REF_NEEDS_COMMIT;
-	}
-
-
-	if ((update->flags & REF_HAVE_NEW) &&
-	    !(update->flags & REF_DELETING) &&
-	    !(update->flags & REF_LOG_ONLY)) {
+	} else if ((update->flags & REF_HAVE_NEW) &&
+		   !(update->flags & REF_DELETING) &&
+		   !(update->flags & REF_LOG_ONLY)) {
 		if (!(update->type & REF_ISSYMREF) &&
 		    oideq(&lock->old_oid, &update->new_oid)) {
 			/*
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 6d42838e15..4bc6a369c7 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -908,7 +908,7 @@  static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 				 */
 				new_update = ref_transaction_add_update(
 						transaction, referent.buf, new_flags,
-						&u->new_oid, &u->old_oid, NULL, NULL, u->msg);
+						&u->new_oid, &u->old_oid, u->new_ref, u->old_ref, u->msg);
 				new_update->parent_update = u;
 
 				/*
@@ -1106,6 +1106,11 @@  static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 			    should_write_log(&arg->refs->base, u->refname))) {
 			struct reftable_log_record *log;
 
+			if (u->flags & REF_SYMREF_UPDATE && u->new_ref)
+				if (!refs_resolve_ref_unsafe(&arg->refs->base, u->new_ref,
+				     RESOLVE_REF_READING, &u->new_oid, NULL))
+					goto done;
+
 			ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
 			log = &logs[logs_nr++];
 			memset(log, 0, sizeof(*log));
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f4e63fae6e..eeb3ee7952 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1360,6 +1360,7 @@  test_expect_success 'fails with duplicate HEAD update' '
 '
 
 test_expect_success 'fails with duplicate ref update via symref' '
+	test_when_finished "git symbolic-ref -d refs/heads/symref2" &&
 	git branch target2 $A &&
 	git symbolic-ref refs/heads/symref2 refs/heads/target2 &&
 	cat >stdin <<-EOF &&
@@ -1812,6 +1813,148 @@  test_expect_success "stdin ${type} symref-create reflogs with --create-reflog" '
 	git reflog exists refs/heads/symref
 '
 
+test_expect_success "stdin ${type} fails symref-update with no ref" '
+	create_stdin_buf ${type} "symref-update " >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	grep "fatal: symref-update: missing <ref>" err
+'
+
+test_expect_success "stdin ${type} fails symref-update with no new value" '
+	create_stdin_buf ${type} "symref-update refs/heads/symref" >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	grep "fatal: symref-update refs/heads/symref: missing <new-ref>" err
+'
+
+test_expect_success "stdin ${type} fails symref-update with too many arguments" '
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$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-update refs/heads/symref: extra input:  $a" err
+	fi
+'
+
+test_expect_success "stdin ${type} symref-update ref creates with zero old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$Z" >stdin &&
+	git update-ref --stdin ${type} --no-deref <stdin &&
+	echo $a >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update ref creates with empty old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" >stdin &&
+	git update-ref --stdin ${type} --no-deref <stdin &&
+	echo $a >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update ref fails with wrong old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	git symbolic-ref refs/heads/symref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$m" "$b" >stdin &&
+	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
+	grep "fatal: symref-update refs/heads/symref: invalid <old-ref> or <old-oid>" err &&
+	test_must_fail git rev-parse --verify -q $c
+'
+
+test_expect_success "stdin ${type} symref-update ref works with right old value" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	git symbolic-ref refs/heads/symref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$m" "$a" >stdin &&
+	git update-ref --stdin ${type} --no-deref <stdin &&
+	echo $m >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update creates symref (with deref)" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/symref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&
+	grep "$Z $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update updates symref (with deref)" '
+	test_when_finished "git symbolic-ref -d refs/heads/symref" &&
+	test_when_finished "git update-ref -d --no-deref refs/heads/symref2" &&
+	git update-ref refs/heads/symref2 $a &&
+	git symbolic-ref --no-recurse refs/heads/symref refs/heads/symref2 &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/symref2 >actual &&
+	test_cmp expect actual &&
+	echo refs/heads/symref2 >expect &&
+	git symbolic-ref --no-recurse refs/heads/symref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/symref >actual &&
+	grep "$(git rev-parse $a) $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update regular ref" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/regularref" "$a" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/regularref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&
+	grep "$(git rev-parse $a) $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update regular ref with correct old-oid" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/regularref" "$a" "$(git rev-parse $a)" >stdin &&
+	git update-ref --stdin ${type} <stdin &&
+	echo $a >expect &&
+	git symbolic-ref --no-recurse refs/heads/regularref >actual &&
+	test_cmp expect actual &&
+	test-tool ref-store main for-each-reflog-ent refs/heads/regularref >actual &&
+	grep "$(git rev-parse $a) $(git rev-parse $a)" actual
+'
+
+test_expect_success "stdin ${type} symref-update regular ref fails with wrong old-oid" '
+	test_when_finished "git update-ref -d refs/heads/regularref" &&
+	git update-ref --no-deref refs/heads/regularref $a &&
+	create_stdin_buf ${type} "symref-update refs/heads/regularref" "$a" "$(git rev-parse refs/heads/target2)" >stdin &&
+	test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
+	echo $(git rev-parse $a) >expect &&
+	git rev-parse refs/heads/regularref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update with zero old-oid" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/symref" &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$Z" >stdin &&
+	git update-ref --stdin ${type} <stdin 2>err &&
+	echo $a >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "stdin ${type} symref-update ref with zero old-oid" '
+	test_when_finished "git symbolic-ref -d --no-recurse refs/heads/symref" &&
+	git symbolic-ref refs/heads/symref refs/heads/target2 &&
+	create_stdin_buf ${type} "symref-update refs/heads/symref" "$a" "$Z" >stdin &&
+	test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
+	grep "fatal: cannot lock ref '"'"'refs/heads/symref'"'"': reference already exists" err &&
+	echo refs/heads/target2 >expect &&
+	git symbolic-ref refs/heads/symref >actual &&
+	test_cmp expect actual
+'
+
 done
 
 test_done