Re: [PATCH v2 0/7] update-ref: add symref oriented commands

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 20, 2024, 06:16 UTC
Message-ID: <ZiNdvEhuhtH87Mw6@framework>
In-Reply-To: <20240412095908.1134387-1-knayak@gitlab.com>

On Fri, Apr 12, 2024 at 11:59:01AM +0200, Karthik Nayak wrote:

Show 20 quoted lines

> From: Karthik Nayak <karthik.188@gmail.com>
> 
> The 'git-update-ref(1)' command allows transactional reference updates.
> But currently only supports regular reference updates. Meaning, if one
> wants to update HEAD (symbolic ref) in a transaction, there is no tool
> to do so.
> 
> One option to obtain transactional updates for the HEAD ref is to
> manually create the HEAD.lock file and commit. This is intrusive, where
> the user needs to mimic internal git behavior. Also, this only works
> when using the files backend.
> 
> At GitLab, we've been using the manual process till date, to allow users
> to set and change their default branch. But with the introduction of
> reftables as a reference backend, this becomes a necessity to be solved
> within git.
> 
> This patch series goes about introducing a set of commands
> symref-{create,verify,delete,update} to work with symrefs complimenting
> the existing commands for the regular refs within 'git-update-ref(1)'.

One more thought crossed my mind this night: is it even necessary to introduce new commands for this? As far as I can see, it should be feasible to introduce symref support for all existing commands without breaking backwards compatibility. This can be achieved by using a prefix that cannot ever be an object ID, like for example "ref:".

Thus, all of the following should work (with 1234 being an OID and 0000 being the null OID):

    update HEAD ref:refs/heads/main ref:refs/heads/master
    update HEAD ref:refs/heads/main 1234
    update HEAD ref:refs/heads/main 0000
    update HEAD 1234 ref:refs/heads/main
    update HEAD 0000 ref:refs/heads/main
    create HEAD ref:refs/heads/main
    delete HEAD ref:refs/heads/main
    verify HEAD ref:refs/heads/mains

Parsing is unambiguous because we can use `starts_with("ref:")` for an otherwise invalid object ID. Furthermore, because refs cannot have spaces, we also don't have an issue with the SP separator.

I have a hunch that this variant might lead to less code duplication, lead to less confusing behaviour and also makes for an easier user interface.

Patrick