Re: [PATCH v2 1/2] object-name: fix resolution of object names containing curly braces

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 5, 2025, 16:14 UTC
Message-ID: <xmqqmsg52q7l.fsf@gitster.g>
In-Reply-To: <13f68bebe90549ba19452f12abb6fea41c2517fb.1735949870.git.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

> Given a branch name of 'foo{bar', commands like
>
>     git cat-file -p foo{bar:README.md
>
> should succeed (assuming that branch had a README.md file, of course).
> However, the change in cce91a2caef9 (Change 'master@noon' syntax to
> 'master@{noon}'., 2006-05-19) presumed that curly braces would always
> come after an '@' or '^' and be paired, causing e.g. 'foo{bar:README.md'
> to entirely miss the ':' and assume there's no object being referenced.
> In short, git would report:
>
>     fatal: Not a valid object name foo{bar:README.md

Naïvely, it seems that a solution is to parse from left to right, i.e., (1) notice there is a colon, (2) see if everything before that colon resolves to a treeish, and (3) see if everything after it is a path that appears in the treeish.

 - When we are given foo@{some:thing}, if we did that, we realize
   that "foo@{some" is not a valid tree-ish object name (since "@{"
   cannot appear in a refname) and then can backtrack by realizing
   "foo" is a ref, and @{...} could be a reflog reference (most
   likely a way to spell some sort of timestamp), and try that.

 - Similarly, for foo:path-gaffed, we would notice "foo" is a valid
   tree-ish object name, and if path-gaffed is a path in it, we'd be
   happy.  Or foo may not be a tree-ish, or path-gaffed may not
   exist in that tree-ish.  In which case, we can backtrack and see
   foo:path-g is an allowed prefix in a desribe name.

Now in the above description, I have assumed that an alternative interpretation kicks in only as a fallback when we backtrack, but we could make sure we try all possibilities and notice ambiguity if we wanted to.

In any case, such an updated structure of the parsing code paths (whether alternative interpretations are treated as fallbacks or equally plausible candidates subject to disambiguation) would be a vast departure from what we currently have, so a targeted "fix" like these two patches attempt would be more appropriate as an initial approach, I think.

Thanks, will queue, but probably we'd look at in any seriousness after the 2.48 final gets tagged.