Re: rev-list and "ambiguous" IDs

[Jeff King <peff@peff.net>]

On Wed, Nov 13, 2019 at 08:35:47PM -0800, Bryan Turner wrote:

> When using a command like `git rev-list dc41e --`, it's possible to
> get output like this: (from newer Git versions)
> error: short SHA1 dc41e is ambiguous
> hint: The candidates are:
> hint:   dc41eeb01ba commit 2012-11-23 - Stuff from the commit message
> hint:   dc41e0d508b tree
> hint:   dc41e5bef41 tree
> hint:   dc41e11ee18 blob
> fatal: bad revision 'dc41e'
>
> Is there any way to ask rev-list to be a little...pickier about what
> it considers a candidate? Almost without question the two trees and
> the blob aren't what I'm asking for, which means there's actually only
> one real candidate.

Try "dc41e^{commit}", which will realize that trees and blobs cannot
peel to a commit (there would still be an ambiguity with a tag).

I think one could argue that without "--objects" in play, rev-list
should automatically disambiguate in favor of a committish. But that's
not true for every command.

You can also set core.disambiguate to "committish" (or even "commit").
At the time we added that option (and started reporting the list of
candidates), we pondered whether it might make sense to make that the
default. That would probably help in a lot of cases, but the argument
against it is that when it goes wrong, it may be quite confusing (so
we're better off with the current message, which punts back to the
user).

I think it also comes up fairly rarely these days, as short sha1s we
print have some headroom built in (as you can see above; the one you've
input is really quite short compared to anything Git would have printed
in that repo).

> Also, while considering this, I noticed that `git rev-list
> dc41e11ee18` (the blob from the output above) doesn't fail. It
> silently exits, nothing written to stdout or stderr, with 0 status. A
> little surprising; I would have expected rev-list to complain that
> dc41e11ee18 isn't a valid commit-ish value.

Yeah, this is a separate issue. If the revision machinery has pending
trees or blobs but isn't asked to show them via "--objects", then it
just ignores them.

I've been running with the patch below for several years; it just adds a
warning when we ignore such an object. I've been tempted to send it for
inclusion, but it has some rough edges:

  - there are some fast-export calls in the test scripts that trigger
    this. I don't remember the details, and what the fix would look
    like.

  - it makes wildcards like "rev-list --all" complain, because they may
    add a tag-of-blob, for example (in git.git, junio-gpg-pub triggers
    this). Things like "--all" would probably need to get smarter, and
    avoid adding non-commits in the first place (when --objects is not
    in use, of course)

---
 revision.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 0e39b2b8a5..7dc2d9a822 100644
--- a/revision.c
+++ b/revision.c
@@ -393,6 +393,16 @@ void add_pending_oid(struct rev_info *revs, const char *name,
 	add_pending_object(revs, object, name);
 }
 
+static void warn_ignored_object(struct object *object, const char *name)
+{
+	if (object->flags & UNINTERESTING)
+		return;
+
+	warning(_("ignoring %s object in traversal: %s"),
+		type_name(object->type),
+		(name && *name) ? name : oid_to_hex(&object->oid));
+}
+
 static struct commit *handle_commit(struct rev_info *revs,
 				    struct object_array_entry *entry)
 {
@@ -458,8 +468,10 @@ static struct commit *handle_commit(struct rev_info *revs,
 	 */
 	if (object->type == OBJ_TREE) {
 		struct tree *tree = (struct tree *)object;
-		if (!revs->tree_objects)
+		if (!revs->tree_objects) {
+			warn_ignored_object(object, name);
 			return NULL;
+		}
 		if (flags & UNINTERESTING) {
 			mark_tree_contents_uninteresting(revs->repo, tree);
 			return NULL;
@@ -472,8 +484,10 @@ static struct commit *handle_commit(struct rev_info *revs,
 	 * Blob object? You know the drill by now..
 	 */
 	if (object->type == OBJ_BLOB) {
-		if (!revs->blob_objects)
+		if (!revs->blob_objects) {
+			warn_ignored_object(object, name);
 			return NULL;
+		}
 		if (flags & UNINTERESTING)
 			return NULL;
 		add_pending_object_with_path(revs, object, name, mode, path);
-- 
2.24.0.739.gb5632e4929