On Mon, Aug 18, 2025 at 12:48:07AM -0400, Jeff King wrote:
> And indeed, the current code does then segfault on "git describe foo" at
> the spot I mentioned. Even though the repository state here is
> unexpected and corrupt, I do think we should probably be more defensive
> and avoid the segfault.
So you almost nerd-sniped me into making a series. But the more I dug
into the rabbit-hole, the more I turned away in disgust. :)
The set of problems I found are:
1. What should happen when traversing from HEAD does not find the blob
in question? Right now we print a blank line, which is...weird.
Probably we should either print nothing, or return an error. If we
return an error, should we respect --always? Are we stuck with the
current dumb behavior because it's a plumbing command?
2. When we are on an unborn branch, we print a confusing message:
$ git init
$ git commit --allow-empty -m foo
$ git tag foo
$ git symbolic-ref HEAD refs/heads/unborn
$ git describe $(echo blob | git hash-object -w --stdin)
fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
We should probably resolve HEAD ourselves and either bail with an
empty output or an error (depending on what we do for (1) above).
3. When we do traverse, if process_object() sees that we didn't find a
commit, we should detect that and either return an empty result or
an error (again, depending on the behavior of (2) above). This is
done by checking is_null_oid(&pcd->current_commit) there.
4. Then we can teach describe_commit() to take a commit rather than an
oid (and the is_null_oid() check becomes a NULL check).
So it all depends on what to do with (1), and for a feature that IMHO
should not even exist in the first place, I had trouble summoning the
will-power to make this 4-patch series.
-Peff
