Re: [PATCH v2 6/7] update-ref: add support for symref-update
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 19, 2024, 09:40 UTC
- Message-ID
- <ZiI8IFdfBHEqr02C@tanuki>
- In-Reply-To
- <20240412095908.1134387-7-knayak@gitlab.com>
On Fri, Apr 12, 2024 at 11:59:07AM +0200, Karthik Nayak wrote:
Show 96 quoted lines
> 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