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

[Patrick Steinhardt <ps@pks.im>]

On Tue, Jun 16, 2026 at 07:53:02AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > +	if (!refs_ref_exists(get_main_ref_store(repo), oldref))
> > +		die(_("reference does not exist: '%s'"), oldref);
> > +	if (refs_ref_exists(get_main_ref_store(repo), newref))
> > +		die(_("reference already exists: '%s'"), newref);
> > +
> > +	return refs_rename_ref(get_main_ref_store(repo), oldref, newref, message);
> > +}
> 
> I suspect that my version shared the same issue, but doesn't
> refs_rename_ref() return -1 for failure, which we may want to turn
> to positive 1 before returning?
> 
> This is a tangent but git.c:handle_builtin() that calls
> git.c:run_builtin() may want to do the "negative return? flip the
> polarity" conversion to make this worry go away.  I dunno what such
> a change would break, though.

Fair. The other subcommands also suffer from the same problem, so I'll
update all of them to return 1 explicitly.

> If we rename a ref that does not have a reflog, would it leave the
> ref under the new name without reflog, or would we get a reflog with
> a single entry that marks the fact the old ref was renamed into the
> new ref?  Should that be controlled via --create-reflog option?

It would leave it without a reflog. In theory I agree that it might make
sense to introduce a "--create-reflog" option, but that would require
some new plumbing in `refs_rename_ref()`. So I'd say that we can add it
at a later point as needed.

Patrick