Re: [PATCH 7/8] refs: add 'update-symref' command to 'update-ref'

Karthik Nayak <karthik.188@gmail.com> writes:

In pre-context, there is this entry.

	update SP <ref> SP <newvalue> [SP <oldvalue>] LF

Unrelated to this patch, we probably should say <new-value> and <old-value> to follow the (recent) documentation guideline to write multiple words concatenated with hyphen.

If we are updating these newvalue and oldvalue anyway, we may want to update them to <new-oid> and <old-oid>, as the existing commands on refs are about object names, and what we are adding here are not.

I would prefer to see such an update of existing documentation as a separate preliminary clean-up patch, so that we can cleanly add the "update-symref" that takes <new-symref-target> on top of a cleaned-up base.

The <newvalue> ...

... we are giving to "update-symref" is not an object name, but a refname (i.e. the target of the symref at <ref> points at), so "newvalue" -> "new-ref" or something is needed.

The semantics of this new command is not quite in line with the existing commands. Existing "update" and "delete" allow you to optionally specify <old-oid> to make sure you do not overwrite somebody else's update in the meantime. Your "update-symref" lacks such safety completely, which I doubt is a good idea in the context of adding it to an existing set that has such safety features. We should at least offer the same "optional" safety. That makes the syntax more like

	update-symref SP <ref> SP <new-ref> [SP <old-ref>] LF

probably.

Existing "update" command can be used to create (by having "zero" <old-oid>) and to delete (by having "zero" <new-oid>), but we still have "create" and "delete" separately. Given that we are teaching the command to also work with symrefs by adding command(s) for symrefs to an existing vocabulary, we should offer "create-symref" and "delete-symref" for consistency between refs and symrefs.

It probably is a good idea to allow "update-symref" to also create and delete a symref in similar ways with the existing "update" allows creation and deletion of a ref. An "zero" <old-ref> may be an update that can happen only when the <ref> does not exist, i.e., creation, and an "zero" <new-ref> may be an update that makes <ref> disappear.

"zero" value in the context of refs is a 0{40} object name (side note: we have an explicit mention of 40 in the doc, which needs to be updated eventually, probably outside this series). But in the new context of symrefs, a safe "zero" value needs to be picked carefully. An empty string may not work well syntactically especially in the `-z` format, because you cannot tell if

	update-symref NUL HEAD NUL refs/heads/main NUL NUL

wants to say that <old-ref> is an empty string, or if it is missing. As a refname cannot have a path component that begins with a dot, a usable "zero" value for <new-ref> and <old-ref> may be a string like ".missing", ".detached", and perhaps ".detached-f78d7...f2d4".

To summarize:

	update-symref SP HEAD SP refs/heads/main LF

forcibly sets HEAD to point at refs/heads/main without regard to the current state of HEAD.

	update-symref SP HEAD SP .missing LF

forcibly removes HEAD symref without regard to the current state of HEAD.

	update-symref SP HEAD SP refs/heads/main SP .missing LF

creates HEAD symref to point at the 'main' branch but fails if HEAD already exists.

	update-symref SP HEAD SP refs/heads/main SP .detached LF
	update-symref SP HEAD SP refs/heads/main SP .detached-f78d7...f2d4 LF

creates HEAD symref to point at the 'main' branch but fails if HEAD is not detached (or detached at the specified commit).

	update-symref SP HEAD SP refs/heads/main SP refs/heads/next LF

creates HEAD symref to point at the 'main' branch but fails if HEAD is not pointing at the 'next' branch.

Hmm?