Subject: Re: [PATCH v3 03/16] builtin/update-ref: skip ambiguity checks when parsing object IDs

From: Junio C Hamano <[email-masked]>
Date: Wed, 26 Feb 2025 14:26:59 -0800

Patrick Steinhardt <[email-masked]> writes:

> Most of the commands in git-update-ref(1) accept an old and/or new
> object ID to update a specific reference to. These object IDs get parsed
> via `repo_get_oid()`, which not only handles plain object IDs, but also
> those that have a suffix like "~" or "^2". More surprisingly though, it
> even knows to resolve references, despite the fact that its manpage does
> not mention this fact even once.

Are you referring to <new-oid> and other placeholders with "oid" in
their names?  I do think "oid" in our documentation implies that
only full hexadecimal object names are allowed.  The glossary agrees
by saying that <object id> is a synonym for <object name> that is
usually 40-hex SHA-1.  However, that is not strictly enforced and we
say <object> (or its typed variants like <commit-ish>) even when a
command takes any extended SHA-1 expression, as described in
Documentation/revisions.{txt,adoc}, not limited to full hexadecimal
object name.

So, I am somewhat sympathetic to your confusion, but not that much.
When we wrote the command and documented it back in 2005, we did
mean to take any object name that is spelled in any way, not just
full hexadecimal.  You may want to update the manual to emphasize
that we encourage the use of full hexadecimal for this command and
elsewhere where it is more appropriate.

> One consequence of this is that we also check for ambiguous references:
> when parsing a full object ID where the DWIM mechanism would also cause
> us to resolve it as a branch, we'd end up printing a warning. While this
> check makes sense to have in general, it is arguably less useful in the
> context of git-update-ref(1).
>
>   - The manpage is explicitly structured around object IDs. So if we see
>     a fully blown object ID, the intent should be quite clear in
>     general.
>
>   - The command is part of our plumbing layer and not a tool that users
>     would generally use in interactive workflows. As such, the warning
>     will likely not be visible to anybody in the first place.

In addition, if the user meant to refer to a ref, it is possible to
disambiguate by prefixing refs/tags/ or whatever.  So squelching the
warning unconditionally might make sense.  We will yield the value
of the full hexadecimal object name, instead of the value of the ref
that is confusingly named, so there is no material change in the
behaviour here.

OK.