[PATCH 4/5] interpret_branch_name: avoid @{upstream} past colon

get_sha1() cannot currently parse a valid object name like "HEAD:@{upstream}" (assuming that such an oddly named file exists in the HEAD commit). It takes two passes to parse the string:

  1. It first considers the whole thing as a ref, which
     results in looking for the upstream of "HEAD:".

  2. It finds the colon, parses "HEAD" as a tree-ish, and then
     finds the path "@{upstream}" in the tree.

For a path that looks like a normal reflog (e.g., "HEAD:@{yesterday}"), the first pass is a no-op. We try to dwim_ref("HEAD:"), that returns zero refs, and we proceed with colon-parsing.

For "HEAD:@{upstream}", though, the first pass ends up in interpret_upstream_mark, which tries to find the branch "HEAD:". When it sees that the branch does not exist, it actually dies rather than returning an error to the caller. As a result, we never make it to the second pass.

One obvious way of fixing this would be to teach interpret_upstream_mark to simply report "no, this isn't an upstream" in such a case. However, that would make the error-reporting for legitimate upstream cases significantly worse. Something like "bogus@{upstream}" would simply report "unknown revision: bogus@{upstream}", while the current code diagnoses a wide variety of possible misconfigurations (no such branch, branch exists but does not have upstream, etc).

However, we can take advantage of the fact that a branch name cannot contain a colon. Therefore even if we find an upstream mark, any prefix with a colon must mean that the upstream mark we found is actually a pathname, and should be disregarded completely. This patch implements that logic.

Signed-off-by: Jeff King <peff@peff.net>
---
I think this would actually be cleaner if get_sha1() simply did the
colon-parsing first, and omitted the first pass completely. Then
sub-functions would not have to care about arbitrary junk that can come
in paths; they would always be parsing just the revision-specifier.

However, given the way this code has developed over the years and its general fragility, I would be entirely unsurprised if there is some case that relies on the current scheme. So I went with the simple fix here, which should be much less likely to have any fallout. And I could not come up with an example that is actually broken under the current code (we just do some suboptimal things, like looking for "foo:bar" as a ref in the filesystem, even though it is syntactically bogus).

 sha1_name.c                   |  3 +++
 t/t1507-rev-parse-upstream.sh | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)