Re: [PATCH 3/4] builtin/refs: add "update" subcommand

[Patrick Steinhardt <ps@pks.im>]

On Tue, Jun 16, 2026 at 07:52:59AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >  git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]
> > +git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
> 
> "<old-value> vs <new-value>" is good, we should update "delete" to
> use "<old-value>" to match.

Good catch.

> > @@ -58,6 +59,12 @@ delete::
> >  	reference is only deleted after verifying that it currently contains
> >  	`<oldvalue>`.
> >  
> > +update::
> > +	Update the given reference to point at `<new-value>`. This subcommand
> > +	mirrors `git update-ref` (see linkgit:git-update-ref[1]). When
> > +	`<old-value>` is given, the reference is only updated after verifying
> > +	that it currently contains `<old-value>`.
> 
> As to the lack of "create", among the two potential changes, I have
> a slight preference for adding "create" and failing "update" that
> does not refer to an existing ref.  If we go that route, the
> "--create-reflog" option should move to "create", as "update" will
> never be used to create a new ref.

I agree that we should add "create", but I also think that we should
keep the special syntax of:

  - "git refs update $NULL_OID [$OLD_OID]" to delete a branch.

  - "git refs update $NEW_OID $NULL_OID" to create a branch.

While the other commands are more ergonomic, I don't see a strong reason
to restrict the "update" subcommand. Also, this ensures that it's in
line with git-update-ref(1), which also allows for these use cases.

> > +	if (repo_get_oid_with_flags(repo, argv[1], &newoid,
> > +				    GET_OID_SKIP_AMBIGUITY_CHECK))
> > +		die(_("invalid new object ID: %s"), argv[1]);
> > +	if (argc == 3 &&
> > +	    repo_get_oid_with_flags(repo, argv[2], &oldoid,
> > +				    GET_OID_SKIP_AMBIGUITY_CHECK))
> > +		die(_("invalid old object ID: %s"), argv[2]);
> 
> On the "delete" side, these messages quote the object name, i.e.,
> 
> 			die(_("invalid old object ID: '%s'"), argv[1]);
> 
> We should be consistent.

Yup, fixed now.

Patrick