{"thread":{"id":"52264","subject":"rev-list and \"ambiguous\" IDs","startedAt":"2019-11-14T04:36:01Z","lastAt":"2019-11-19T01:24:42Z","messageCount":13,"participants":["Bryan Turner","Jeff King","Thomas Braun","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"386160","messageId":"CAGyf7-EXOUWYUZXmww2+NyD1OuWEG18n221MPojVSCCu=19JNA@mail.gmail.com","threadId":"52264","inReplyTo":null,"subject":"rev-list and \"ambiguous\" IDs","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-11-14T04:35:47Z","receivedAt":"2019-11-14T04:36:01Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"When using a command like `git rev-list dc41e --`, it's possible to\nget output like this: (from newer Git versions)\nerror: short SHA1 dc41e is ambiguous\nhint: The candidates are:\nhint: dc41eeb01ba commit 2012-11-23 - Stuff from the commit message\nhint: dc41e0d508b tree\nhint: dc41e5bef41 tree\nhint: dc41e11ee18 blob\nfatal: bad revision 'dc41e'\n\nIs there any way to ask rev-list to be a little...pickier about what\nit considers a candidate? Almost without question the two trees and\nthe blob aren't what I'm asking for, which means there's actually only\none real candidate.\n\nAlso, while considering this, I noticed that `git rev-list\ndc41e11ee18` (the blob from the output above) doesn't fail. It\nsilently exits, nothing written to stdout or stderr, with 0 status. A\nlittle surprising; I would have expected rev-list to complain that\ndc41e11ee18 isn't a valid commit-ish value.\n\nBryan\n"},{"id":"386162","messageId":"20191114055906.GA10643@sigill.intra.peff.net","threadId":"52264","inReplyTo":"CAGyf7-EXOUWYUZXmww2+NyD1OuWEG18n221MPojVSCCu=19JNA@mail.gmail.com","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-14T05:59:06Z","receivedAt":"2019-11-14T05:59:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 13, 2019 at 08:35:47PM -0800, Bryan Turner wrote:\n\n> When using a command like `git rev-list dc41e --`, it's possible to\n> get output like this: (from newer Git versions)\n> error: short SHA1 dc41e is ambiguous\n> hint: The candidates are:\n> hint: dc41eeb01ba commit 2012-11-23 - Stuff from the commit message\n> hint: dc41e0d508b tree\n> hint: dc41e5bef41 tree\n> hint: dc41e11ee18 blob\n> fatal: bad revision 'dc41e'\n>\n> Is there any way to ask rev-list to be a little...pickier about what\n> it considers a candidate? Almost without question the two trees and\n> the blob aren't what I'm asking for, which means there's actually only\n> one real candidate.\n\nTry \"dc41e^{commit}\", which will realize that trees and blobs cannot\npeel to a commit (there would still be an ambiguity with a tag).\n\nI think one could argue that without \"--objects\" in play, rev-list\nshould automatically disambiguate in favor of a committish. But that's\nnot true for every command.\n\nYou can also set core.disambiguate to \"committish\" (or even \"commit\").\nAt the time we added that option (and started reporting the list of\ncandidates), we pondered whether it might make sense to make that the\ndefault. That would probably help in a lot of cases, but the argument\nagainst it is that when it goes wrong, it may be quite confusing (so\nwe're better off with the current message, which punts back to the\nuser).\n\nI think it also comes up fairly rarely these days, as short sha1s we\nprint have some headroom built in (as you can see above; the one you've\ninput is really quite short compared to anything Git would have printed\nin that repo).\n\n> Also, while considering this, I noticed that `git rev-list\n> dc41e11ee18` (the blob from the output above) doesn't fail. It\n> silently exits, nothing written to stdout or stderr, with 0 status. A\n> little surprising; I would have expected rev-list to complain that\n> dc41e11ee18 isn't a valid commit-ish value.\n\nYeah, this is a separate issue. If the revision machinery has pending\ntrees or blobs but isn't asked to show them via \"--objects\", then it\njust ignores them.\n\nI've been running with the patch below for several years; it just adds a\nwarning when we ignore such an object. I've been tempted to send it for\ninclusion, but it has some rough edges:\n\n - there are some fast-export calls in the test scripts that trigger\n this. I don't remember the details, and what the fix would look\n like.\n\n - it makes wildcards like \"rev-list --all\" complain, because they may\n add a tag-of-blob, for example (in git.git, junio-gpg-pub triggers\n this). Things like \"--all\" would probably need to get smarter, and\n avoid adding non-commits in the first place (when --objects is not\n in use, of course)\n\n---\n revision.c | 18 ++++++++++++++++--\n 1 file changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0e39b2b8a5..7dc2d9a822 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -393,6 +393,16 @@ void add_pending_oid(struct rev_info *revs, const char *name,\n \tadd_pending_object(revs, object, name);\n }\n \n+static void warn_ignored_object(struct object *object, const char *name)\n+{\n+\tif (object->flags & UNINTERESTING)\n+\t\treturn;\n+\n+\twarning(_(\"ignoring %s object in traversal: %s\"),\n+\t\ttype_name(object->type),\n+\t\t(name && *name) ? name : oid_to_hex(&object->oid));\n+}\n+\n static struct commit *handle_commit(struct rev_info *revs,\n \t\t\t\t struct object_array_entry *entry)\n {\n@@ -458,8 +468,10 @@ static struct commit *handle_commit(struct rev_info *revs,\n \t */\n \tif (object->type == OBJ_TREE) {\n \t\tstruct tree *tree = (struct tree *)object;\n-\t\tif (!revs->tree_objects)\n+\t\tif (!revs->tree_objects) {\n+\t\t\twarn_ignored_object(object, name);\n \t\t\treturn NULL;\n+\t\t}\n \t\tif (flags & UNINTERESTING) {\n \t\t\tmark_tree_contents_uninteresting(revs->repo, tree);\n \t\t\treturn NULL;\n@@ -472,8 +484,10 @@ static struct commit *handle_commit(struct rev_info *revs,\n \t * Blob object? You know the drill by now..\n \t */\n \tif (object->type == OBJ_BLOB) {\n-\t\tif (!revs->blob_objects)\n+\t\tif (!revs->blob_objects) {\n+\t\t\twarn_ignored_object(object, name);\n \t\t\treturn NULL;\n+\t\t}\n \t\tif (flags & UNINTERESTING)\n \t\t\treturn NULL;\n \t\tadd_pending_object_with_path(revs, object, name, mode, path);\n-- \n2.24.0.739.gb5632e4929\n\n"},{"id":"386215","messageId":"ab4dcc9c-4416-aef8-c8c4-38bb5ec97990@virtuell-zuhause.de","threadId":"52264","inReplyTo":"20191114055906.GA10643@sigill.intra.peff.net","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Thomas Braun","fromEmail":"thomas.braun@virtuell-zuhause.de","sentAt":"2019-11-15T00:12:47Z","receivedAt":"2019-11-15T00:13:32Z","isPatch":false,"sender":{"key":"thomas.braun@virtuell-zuhause.de","avatar":"https://avatars.githubusercontent.com/u/1185677?v=4"},"body":"Am 14.11.2019 um 06:59 schrieb Jeff King:\n\n[...]\n\n> You can also set core.disambiguate to \"committish\" (or even \"commit\").\n> At the time we added that option (and started reporting the list of\n> candidates), we pondered whether it might make sense to make that the\n> default.\n\nI did not know this setting. Thanks!\n\n> That would probably help in a lot of cases, but the argument\n> against it is that when it goes wrong, it may be quite confusing (so\n> we're better off with the current message, which punts back to the\n> user).\n\nJust out of curiosity: Is there a use case for inspecting non-commit\nobjects with git log?\n\nIf I do (in the git repo)\n\n$ git log 1231\n\nI get\n\nerror: short SHA1 1231 is ambiguous\nhint: The candidates are:\nhint: 123139fc89 tree\nhint: 12316a1673 tree\nhint: 123144fe8a blob\nfatal: ambiguous argument '1231': unknown revision or path not in the\nworking tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n\nwith\n$ git --version\ngit version 2.24.0.windows.2\n\nand all of these candidates are no commits.\n\n> I think it also comes up fairly rarely these days, as short sha1s we\n> print have some headroom built in (as you can see above; the one you've\n> input is really quite short compared to anything Git would have printed\n> in that repo).\n> \n>> Also, while considering this, I noticed that `git rev-list\n>> dc41e11ee18` (the blob from the output above) doesn't fail. It\n>> silently exits, nothing written to stdout or stderr, with 0 status. A\n>> little surprising; I would have expected rev-list to complain that\n>> dc41e11ee18 isn't a valid commit-ish value.\n> \n> Yeah, this is a separate issue. If the revision machinery has pending\n> trees or blobs but isn't asked to show them via \"--objects\", then it\n> just ignores them.\n> \n> I've been running with the patch below for several years; it just adds a\n> warning when we ignore such an object. I've been tempted to send it for\n> inclusion, but it has some rough edges:\n> \n> - there are some fast-export calls in the test scripts that trigger\n> this. I don't remember the details, and what the fix would look\n> like.\n> \n> - it makes wildcards like \"rev-list --all\" complain, because they may\n> add a tag-of-blob, for example (in git.git, junio-gpg-pub triggers\n> this). Things like \"--all\" would probably need to get smarter, and\n> avoid adding non-commits in the first place (when --objects is not\n> in use, of course)\n> \n> ---\n> revision.c | 18 ++++++++++++++++--\n> 1 file changed, 16 insertions(+), 2 deletions(-)\n> \n> diff --git a/revision.c b/revision.c\n> index 0e39b2b8a5..7dc2d9a822 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -393,6 +393,16 @@ void add_pending_oid(struct rev_info *revs, const char *name,\n> \tadd_pending_object(revs, object, name);\n> }\n> \n> +static void warn_ignored_object(struct object *object, const char *name)\n> +{\n> +\tif (object->flags & UNINTERESTING)\n> +\t\treturn;\n> +\n> +\twarning(_(\"ignoring %s object in traversal: %s\"),\n> +\t\ttype_name(object->type),\n> +\t\t(name && *name) ? name : oid_to_hex(&object->oid));\n> +}\n> +\n> static struct commit *handle_commit(struct rev_info *revs,\n> \t\t\t\t struct object_array_entry *entry)\n> {\n> @@ -458,8 +468,10 @@ static struct commit *handle_commit(struct rev_info *revs,\n> \t */\n> \tif (object->type == OBJ_TREE) {\n> \t\tstruct tree *tree = (struct tree *)object;\n> -\t\tif (!revs->tree_objects)\n> +\t\tif (!revs->tree_objects) {\n> +\t\t\twarn_ignored_object(object, name);\n> \t\t\treturn NULL;\n> +\t\t}\n> \t\tif (flags & UNINTERESTING) {\n> \t\t\tmark_tree_contents_uninteresting(revs->repo, tree);\n> \t\t\treturn NULL;\n> @@ -472,8 +484,10 @@ static struct commit *handle_commit(struct rev_info *revs,\n> \t * Blob object? You know the drill by now..\n> \t */\n> \tif (object->type == OBJ_BLOB) {\n> -\t\tif (!revs->blob_objects)\n> +\t\tif (!revs->blob_objects) {\n> +\t\t\twarn_ignored_object(object, name);\n> \t\t\treturn NULL;\n> +\t\t}\n> \t\tif (flags & UNINTERESTING)\n> \t\t\treturn NULL;\n> \t\tadd_pending_object_with_path(revs, object, name, mode, path);\n> \n\n"},{"id":"386244","messageId":"CAGyf7-GTWsQEYH9mkM8TkY1PusMimtYcSaKhHubN_KsOtMRiBA@mail.gmail.com","threadId":"52264","inReplyTo":"20191114055906.GA10643@sigill.intra.peff.net","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-11-15T01:19:39Z","receivedAt":"2019-11-15T01:19:53Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Wed, Nov 13, 2019 at 9:59 PM Jeff King wrote:\n>\n> On Wed, Nov 13, 2019 at 08:35:47PM -0800, Bryan Turner wrote:\n>\n> > When using a command like `git rev-list dc41e --`, it's possible to\n> > get output like this: (from newer Git versions)\n> > error: short SHA1 dc41e is ambiguous\n> > hint: The candidates are:\n> > hint: dc41eeb01ba commit 2012-11-23 - Stuff from the commit message\n> > hint: dc41e0d508b tree\n> > hint: dc41e5bef41 tree\n> > hint: dc41e11ee18 blob\n> > fatal: bad revision 'dc41e'\n> >\n> > Is there any way to ask rev-list to be a little...pickier about what\n> > it considers a candidate? Almost without question the two trees and\n> > the blob aren't what I'm asking for, which means there's actually only\n> > one real candidate.\n>\n> Try \"dc41e^{commit}\", which will realize that trees and blobs cannot\n> peel to a commit (there would still be an ambiguity with a tag).\n\nSlick!\n\n>\n> I think one could argue that without \"--objects\" in play, rev-list\n> should automatically disambiguate in favor of a committish. But that's\n> not true for every command.\n>\n> You can also set core.disambiguate to \"committish\" (or even \"commit\").\n> At the time we added that option (and started reporting the list of\n> candidates), we pondered whether it might make sense to make that the\n> default. That would probably help in a lot of cases, but the argument\n> against it is that when it goes wrong, it may be quite confusing (so\n> we're better off with the current message, which punts back to the\n> user).\n\nHaving no disambiguation by default seems fine. Both of the approaches\nhere seem easy enough to activate explicitly in cases where the caller\n(in this case Bitbucket Server; more on that later) knows they're\nlooking for a commit.\n\n>\n> I think it also comes up fairly rarely these days, as short sha1s we\n> print have some headroom built in (as you can see above; the one you've\n> input is really quite short compared to anything Git would have printed\n> in that repo).\n\nJust to provide a little context, this isn't coming up as something I\nmyself hit. Rather, it's a fairly common issue reported by Bitbucket\nServer end users, and I would assume it happens with other hosting\nproviders as well: A user URL-hacks an ambiguous (or \"ambiguous\", in\ncases like this) short hash and is disappointed when the system\ndoesn't manage to find the commit they were looking for. I'm just\ninvestigating possible avenues for improving how Bitbucket Server\nhandles these cases. One option is to (essentially) parse the \"hint\",\nif it's present, to get the candidates, and include them on the error\nmessage we display. But in cases like the above it gets weird because\nthere's only one _commit_ candidate, and having our error message\ninclude trees and blobs seems likely to be confusing/unexpected. I\nsuspect most Bitbucket Server users would say \"The answer's obvious!\nWhy didn't you just use the commit?!\", and I can sort of get behind\nthat view. The combination of using the disambiguation mechanism, so\nsingle-commit ambiguities are resolved automatically, and parsing the\nhint seems like it would produce the most logical behavior.\n\nWhere users get the short hashes they try is an interesting question.\nAs you say, Git wouldn't display a 5 character short hash, at least by\ndefault, and Bitbucket Server doesn't either; it shows a flat 11\ncharacters. I'm not sure, on that point.\n\nThanks for your insights! Learned a new Git trick today.\n\nBest regards,\nBryan Turner\n\n\n\n>\n> > Also, while considering this, I noticed that `git rev-list\n> > dc41e11ee18` (the blob from the output above) doesn't fail. It\n> > silently exits, nothing written to stdout or stderr, with 0 status. A\n> > little surprising; I would have expected rev-list to complain that\n> > dc41e11ee18 isn't a valid commit-ish value.\n>\n> Yeah, this is a separate issue. If the revision machinery has pending\n> trees or blobs but isn't asked to show them via \"--objects\", then it\n> just ignores them.\n>\n> I've been running with the patch below for several years; it just adds a\n> warning when we ignore such an object. I've been tempted to send it for\n> inclusion, but it has some rough edges:\n>\n> - there are some fast-export calls in the test scripts that trigger\n> this. I don't remember the details, and what the fix would look\n> like.\n>\n> - it makes wildcards like \"rev-list --all\" complain, because they may\n> add a tag-of-blob, for example (in git.git, junio-gpg-pub triggers\n> this). Things like \"--all\" would probably need to get smarter, and\n> avoid adding non-commits in the first place (when --objects is not\n> in use, of course)\n>\n> ---\n> revision.c | 18 ++++++++++++++++--\n> 1 file changed, 16 insertions(+), 2 deletions(-)\n>\n> diff --git a/revision.c b/revision.c\n> index 0e39b2b8a5..7dc2d9a822 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -393,6 +393,16 @@ void add_pending_oid(struct rev_info *revs, const char *name,\n> add_pending_object(revs, object, name);\n> }\n>\n> +static void warn_ignored_object(struct object *object, const char *name)\n> +{\n> + if (object->flags & UNINTERESTING)\n> + return;\n> +\n> + warning(_(\"ignoring %s object in traversal: %s\"),\n> + type_name(object->type),\n> + (name && *name) ? name : oid_to_hex(&object->oid));\n> +}\n> +\n> static struct commit *handle_commit(struct rev_info *revs,\n> struct object_array_entry *entry)\n> {\n> @@ -458,8 +468,10 @@ static struct commit *handle_commit(struct rev_info *revs,\n> */\n> if (object->type == OBJ_TREE) {\n> struct tree *tree = (struct tree *)object;\n> - if (!revs->tree_objects)\n> + if (!revs->tree_objects) {\n> + warn_ignored_object(object, name);\n> return NULL;\n> + }\n> if (flags & UNINTERESTING) {\n> mark_tree_contents_uninteresting(revs->repo, tree);\n> return NULL;\n> @@ -472,8 +484,10 @@ static struct commit *handle_commit(struct rev_info *revs,\n> * Blob object? You know the drill by now..\n> */\n> if (object->type == OBJ_BLOB) {\n> - if (!revs->blob_objects)\n> + if (!revs->blob_objects) {\n> + warn_ignored_object(object, name);\n> return NULL;\n> + }\n> if (flags & UNINTERESTING)\n> return NULL;\n> add_pending_object_with_path(revs, object, name, mode, path);\n> --\n> 2.24.0.739.gb5632e4929\n>\n"},{"id":"386246","messageId":"20191115034941.GB20863@sigill.intra.peff.net","threadId":"52264","inReplyTo":"ab4dcc9c-4416-aef8-c8c4-38bb5ec97990@virtuell-zuhause.de","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-15T03:49:41Z","receivedAt":"2019-11-15T03:49:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 15, 2019 at 01:12:47AM +0100, Thomas Braun wrote:\n\n> > That would probably help in a lot of cases, but the argument\n> > against it is that when it goes wrong, it may be quite confusing (so\n> > we're better off with the current message, which punts back to the\n> > user).\n> \n> Just out of curiosity: Is there a use case for inspecting non-commit\n> objects with git log?\n\nNot that I can think of. You can't even say \"--objects\" there.\n\nAnd indeed, \"git log\" already prefers commits for disambiguation, since\nd5f6b1d756 (revision.c: the \"log\" family, except for \"show\", takes\ncommittish, 2012-07-02).\n\nBut...\n\n> If I do (in the git repo)\n> \n> $ git log 1231\n> \n> I get\n> \n> error: short SHA1 1231 is ambiguous\n> hint: The candidates are:\n> hint: 123139fc89 tree\n> hint: 12316a1673 tree\n> hint: 123144fe8a blob\n> fatal: ambiguous argument '1231': unknown revision or path not in the\n> working tree.\n> Use '--' to separate paths from revisions, like this:\n> 'git [...] -- [...]'\n> \n> with\n> $ git --version\n> git version 2.24.0.windows.2\n> \n> and all of these candidates are no commits.\n\n...remember that the disambiguation code is just about preferring one\nobject to the other. If the rule in effect doesn't have a preference,\nit's still ambiguous. On my system, \"1231\" actually _does_ have a\ncommit:\n\n $ git show 1231\n error: short SHA1 1231 is ambiguous\n hint: The candidates are:\n hint: 12319e3bf2 commit 2017-03-25 - Merge 'git-gui-add-2nd-line' into HEAD\n hint: 123139fc89 tree\n hint: 12315b58b8 tree\n hint: 12316a1673 tree\n hint: 12317ab2d9 tree\n hint: 123193f802 tree\n hint: 123144fe8a blob\n fatal: ambiguous argument '1231': unknown revision or path not in the working tree.\n Use '--' to separate paths from revisions, like this:\n 'git [...] -- [...]'\n\nThat's ambiguous because git-show can handle trees and blobs, too. But\nif I feed that sha1 to git-log:\n\n $ git log --oneline -1 1231\n 12319e3bf2 Merge 'git-gui-add-2nd-line' into HEAD\n\nit's perfectly fine, because git-log knows to disambiguate the commit.\nBut if I choose another prefix that has no commits at all, it's\nambiguous under either, because the \"committish\" rule has no way to\ndecide:\n\n $ git show abcd2\n error: short SHA1 abcd2 is ambiguous\n hint: The candidates are:\n hint: abcd22f55e tree\n hint: abcd238df0 tree\n hint: abcd2b1cc8 blob\n \n $ git log abcd2\n error: short SHA1 abcd2 is ambiguous\n hint: The candidates are:\n hint: abcd22f55e tree\n hint: abcd238df0 tree\n hint: abcd2b1cc8 blob\n\n-Peff\n"},{"id":"386247","messageId":"20191115035711.GC20863@sigill.intra.peff.net","threadId":"52264","inReplyTo":"CAGyf7-GTWsQEYH9mkM8TkY1PusMimtYcSaKhHubN_KsOtMRiBA@mail.gmail.com","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-15T03:57:11Z","receivedAt":"2019-11-15T03:57:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 14, 2019 at 05:19:39PM -0800, Bryan Turner wrote:\n\n> Just to provide a little context, this isn't coming up as something I\n> myself hit. Rather, it's a fairly common issue reported by Bitbucket\n> Server end users, and I would assume it happens with other hosting\n> providers as well: A user URL-hacks an ambiguous (or \"ambiguous\", in\n> cases like this) short hash and is disappointed when the system\n> doesn't manage to find the commit they were looking for. I'm just\n> investigating possible avenues for improving how Bitbucket Server\n> handles these cases. One option is to (essentially) parse the \"hint\",\n> if it's present, to get the candidates, and include them on the error\n> message we display. But in cases like the above it gets weird because\n> there's only one _commit_ candidate, and having our error message\n> include trees and blobs seems likely to be confusing/unexpected. I\n> suspect most Bitbucket Server users would say \"The answer's obvious!\n> Why didn't you just use the commit?!\", and I can sort of get behind\n> that view. The combination of using the disambiguation mechanism, so\n> single-commit ambiguities are resolved automatically, and parsing the\n> hint seems like it would produce the most logical behavior.\n\nIt depends on your URL scheme obviously, but on GitHub for example, it\nwould make sense for https://github.com/user/repo/commit/1234abcd to use\nthe \"^{commit}\" trick. I don't think it currently does, though.\n\n> Where users get the short hashes they try is an interesting question.\n> As you say, Git wouldn't display a 5 character short hash, at least by\n> default, and Bitbucket Server doesn't either; it shows a flat 11\n> characters. I'm not sure, on that point.\n\nGitHub often produces 7-char short hashes, because it's abbreviating\nthem in presentation code that doesn't want to spend the round trip to\ntalk to the repo (to find out if it's unique, or how many objects are in\nthe repo). It _usually_ shouldn't matter much, because we try to produce\nabbreviated hashes where users might read them, and long hashes when we\ngenerate URLs. But of course people sometimes generate URLs themselves\nfrom who knows where. :) I've been lightly lobbying to bump our default\nto something higher, like 12.\n\n-Peff\n"},{"id":"386253","messageId":"xmqqa78x918e.fsf@gitster-ct.c.googlers.com","threadId":"52264","inReplyTo":"ab4dcc9c-4416-aef8-c8c4-38bb5ec97990@virtuell-zuhause.de","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-15T05:07:13Z","receivedAt":"2019-11-15T05:07:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Braun writes:\n\n> Just out of curiosity: Is there a use case for inspecting non-commit\n> objects with git log?\n\nI do not think there is (rev-list is a different story, given that\nyou can pass --objects), and it probably is not too difficult to\nteach \"git log\" and friends that they only want commit-ish.\n\nThanks.\n"},{"id":"386262","messageId":"20191115081614.GA27149@sigill.intra.peff.net","threadId":"52264","inReplyTo":"xmqqa78x918e.fsf@gitster-ct.c.googlers.com","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-15T08:16:14Z","receivedAt":"2019-11-15T08:16:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 15, 2019 at 02:07:13PM +0900, Junio C Hamano wrote:\n\n> Thomas Braun writes:\n> \n> > Just out of curiosity: Is there a use case for inspecting non-commit\n> > objects with git log?\n> \n> I do not think there is (rev-list is a different story, given that\n> you can pass --objects), and it probably is not too difficult to\n> teach \"git log\" and friends that they only want commit-ish.\n\nI think you already did; see my other reply. ;)\n\n-Peff\n"},{"id":"386303","messageId":"xmqq4kz57587.fsf@gitster-ct.c.googlers.com","threadId":"52264","inReplyTo":"20191115081614.GA27149@sigill.intra.peff.net","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-15T11:23:52Z","receivedAt":"2019-11-15T11:23:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Nov 15, 2019 at 02:07:13PM +0900, Junio C Hamano wrote:\n>\n>> Thomas Braun writes:\n>> \n>> > Just out of curiosity: Is there a use case for inspecting non-commit\n>> > objects with git log?\n>> \n>> I do not think there is (rev-list is a different story, given that\n>> you can pass --objects), and it probably is not too difficult to\n>> teach \"git log\" and friends that they only want commit-ish.\n>\n> I think you already did; see my other reply. ;)\n\nHeh, indeed I did. I just did not recall.\n\n"},{"id":"386348","messageId":"917e2664-6059-c190-30fd-02f3cf7aa5dc@virtuell-zuhause.de","threadId":"52264","inReplyTo":"20191115034941.GB20863@sigill.intra.peff.net","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Thomas Braun","fromEmail":"thomas.braun@virtuell-zuhause.de","sentAt":"2019-11-15T23:38:27Z","receivedAt":"2019-11-15T23:38:47Z","isPatch":false,"sender":{"key":"thomas.braun@virtuell-zuhause.de","avatar":"https://avatars.githubusercontent.com/u/1185677?v=4"},"body":"Am 15.11.2019 um 04:49 schrieb Jeff King:\n> On Fri, Nov 15, 2019 at 01:12:47AM +0100, Thomas Braun wrote:\n> \n>>> That would probably help in a lot of cases, but the argument\n>>> against it is that when it goes wrong, it may be quite confusing (so\n>>> we're better off with the current message, which punts back to the\n>>> user).\n>>\n>> Just out of curiosity: Is there a use case for inspecting non-commit\n>> objects with git log?\n> \n> Not that I can think of. You can't even say \"--objects\" there.\n> \n> And indeed, \"git log\" already prefers commits for disambiguation, since\n> d5f6b1d756 (revision.c: the \"log\" family, except for \"show\", takes\n> committish, 2012-07-02).\n> \n> But...\n> \n>> If I do (in the git repo)\n>>\n>> $ git log 1231\n>>\n>> I get\n>>\n>> error: short SHA1 1231 is ambiguous\n>> hint: The candidates are:\n>> hint: 123139fc89 tree\n>> hint: 12316a1673 tree\n>> hint: 123144fe8a blob\n>> fatal: ambiguous argument '1231': unknown revision or path not in the\n>> working tree.\n>> Use '--' to separate paths from revisions, like this:\n>> 'git [...] -- [...]'\n>>\n>> with\n>> $ git --version\n>> git version 2.24.0.windows.2\n>>\n>> and all of these candidates are no commits.\n> \n> ...remember that the disambiguation code is just about preferring one\n> object to the other. If the rule in effect doesn't have a preference,\n> it's still ambiguous. On my system, \"1231\" actually _does_ have a\n> commit:\n> \n> $ git show 1231\n> error: short SHA1 1231 is ambiguous\n> hint: The candidates are:\n> hint: 12319e3bf2 commit 2017-03-25 - Merge 'git-gui-add-2nd-line' into HEAD\n> hint: 123139fc89 tree\n> hint: 12315b58b8 tree\n> hint: 12316a1673 tree\n> hint: 12317ab2d9 tree\n> hint: 123193f802 tree\n> hint: 123144fe8a blob\n> fatal: ambiguous argument '1231': unknown revision or path not in the working tree.\n> Use '--' to separate paths from revisions, like this:\n> 'git [...] -- [...]'\n> \n> That's ambiguous because git-show can handle trees and blobs, too. But\n> if I feed that sha1 to git-log:\n> \n> $ git log --oneline -1 1231\n> 12319e3bf2 Merge 'git-gui-add-2nd-line' into HEAD\n> \n> it's perfectly fine, because git-log knows to disambiguate the commit.\n> But if I choose another prefix that has no commits at all, it's\n> ambiguous under either, because the \"committish\" rule has no way to\n> decide:\n> \n> $ git show abcd2\n> error: short SHA1 abcd2 is ambiguous\n> hint: The candidates are:\n> hint: abcd22f55e tree\n> hint: abcd238df0 tree\n> hint: abcd2b1cc8 blob\n> \n> $ git log abcd2\n> error: short SHA1 abcd2 is ambiguous\n> hint: The candidates are:\n> hint: abcd22f55e tree\n> hint: abcd238df0 tree\n> hint: abcd2b1cc8 blob\n\nI would have expected that git log did just tell me that it could not\nfind something commitish, instead it told me that there are multiple\ncandidates, all of them being no commit.\n\n"},{"id":"386360","messageId":"xmqqmucw4h4n.fsf@gitster-ct.c.googlers.com","threadId":"52264","inReplyTo":"917e2664-6059-c190-30fd-02f3cf7aa5dc@virtuell-zuhause.de","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-16T03:47:20Z","receivedAt":"2019-11-16T03:47:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Braun writes:\n\n>> But if I choose another prefix that has no commits at all, it's\n>> ambiguous under either, because the \"committish\" rule has no way to\n>> decide:\n>> \n>> $ git show abcd2\n>> error: short SHA1 abcd2 is ambiguous\n>> hint: The candidates are:\n>> hint: abcd22f55e tree\n>> hint: abcd238df0 tree\n>> hint: abcd2b1cc8 blob\n>> \n>> $ git log abcd2\n>> error: short SHA1 abcd2 is ambiguous\n>> hint: The candidates are:\n>> hint: abcd22f55e tree\n>> hint: abcd238df0 tree\n>> hint: abcd2b1cc8 blob\n>\n> I would have expected that git log did just tell me that it could not\n> find something commitish, instead it told me that there are multiple\n> candidates, all of them being no commit.\n\nWith this, I 100% agree with. The latter should instead say\n\n $ git log abcd2 [--]\n error: bad revision 'abcd2'\n\njust like the case where no object has abcd2 as prefix.\n\nWhen we ask for commit-ish or any specific type in general, there\nare a few possible cases.\n\n - There is only one such object that has the prefix and is\n compatible with the type. We handle this correctly---yield that\n object and do not complain about ambiguity.\n\n - There are two or more such objects, or there is no such object.\n We show all objects that share the prefix, regardless of the\n type, which is way suboptimal.\n\nAn improvement can be localized to sha1-name.c::get_short_oid(), I\nwould think. We know what type we want (e.g. GET_OID_COMMITTISH)\nin the function, so we should be able to teach collect_ambiguous() \nto discard an object with the given prefix but of a wrong type.\n\n"},{"id":"386449","messageId":"20191118120315.GB12766@sigill.intra.peff.net","threadId":"52264","inReplyTo":"xmqqmucw4h4n.fsf@gitster-ct.c.googlers.com","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-18T12:03:15Z","receivedAt":"2019-11-18T12:03:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 16, 2019 at 12:47:20PM +0900, Junio C Hamano wrote:\n\n> > I would have expected that git log did just tell me that it could not\n> > find something commitish, instead it told me that there are multiple\n> > candidates, all of them being no commit.\n> \n> With this, I 100% agree with. The latter should instead say\n> \n> $ git log abcd2 [--]\n> error: bad revision 'abcd2'\n> \n> just like the case where no object has abcd2 as prefix.\n> \n> When we ask for commit-ish or any specific type in general, there\n> are a few possible cases.\n> \n> - There is only one such object that has the prefix and is\n> compatible with the type. We handle this correctly---yield that\n> object and do not complain about ambiguity.\n> \n> - There are two or more such objects, or there is no such object.\n> We show all objects that share the prefix, regardless of the\n> type, which is way suboptimal.\n> \n> An improvement can be localized to sha1-name.c::get_short_oid(), I\n> would think. We know what type we want (e.g. GET_OID_COMMITTISH)\n> in the function, so we should be able to teach collect_ambiguous() \n> to discard an object with the given prefix but of a wrong type.\n\nI think that changes the meaning of GET_OID_COMMITTISH, though. Right\nnow it means \"if disambiguating, prefer a committish\", but not \"I can\nonly accept a commit\". So we would still happily return an unambiguous\nobject that does not match that type. And that is why \"git -c\ncore.disambiguate=committish show $short_blob\" works, for example.\n\nIf you adjust your first case above to \"only one such object...and the\ntype does not matter\" then I think it is OK. I.e., the logic in\nget_short_oid() becomes:\n\n - if there is only one, return it\n\n - if there is more than one, and only one matches the disambiguator,\n return it\n\n - otherwise, _do not_ print the ambiguous list, and return an error\n (and no object at all), letting the caller complain\n\nwhere the third part is the new behavior. I think that helps in some\nways (you do not get a list of non-commits for a context that only takes\ncommits). But it might also hurt, because it gives the user less\ninformation. E.g., imagine the user feeds a short sha1 that they know to\nbe a blob to a command expecting a committish and is told \"no, that\nshort sha1 does not exist\", even though the actual problem is that there\nare two such blobs.\n\nI think it's a bit simpler for a command which doesn't expect\nnon-commits at all, like \"git log\". But it would need to communicate\nthat to get_short_oid() with more than just GET_OID_COMMITTISH, so that\nthe latter can tell it apart from contexts which merely prefer a\ncommittish.\n\nI'm also not entirely sure that even that case doesn't suffer from\ntelling the user less information. If I say \"git log 1234\" knowing that\n\"1234\" is a blob, that's a mistake. But Git may guide me in correcting\nthat mistake by saying \"yes, we know about 1234, but it's ambiguous\"\nrather than \"1234 is not something we know about\".\n\nPerhaps a simple fix would just be for get_short_oid()'s error message\nto mention the disambiguation rule. E.g., something like:\n\n $ git show abcd2\n error: short SHA1 abcd2 is ambiguous\n hint: We would have preferred a commit or tag pointing to a commit,\n hint: but none were found. The candidates are:\n hint: abcd22f55e tree\n hint: abcd238df0 tree\n hint: abcd2b1cc8 blob\n\nor\n\n $ git show abcd2\n error: short SHA1 abcd2 is ambiguous\n hint: We preferred a commit or tag pointing to a commit to other\n hint: object types, but two candidates were found:\n hint: abcd22f55e commit\n hint: abcd238df0 commit\n hint: abcd2b1cc8 blob\n\n(optionally the second one could even not mention the blob, though I\nthink with the lead-in sentence it's OK).\n\nThe verbiage there isn't great (I was trying to avoid the jargon\n\"committish\"), but hopefully you get the point.\n\n-Peff\n"},{"id":"386493","messageId":"xmqq7e3w1wvg.fsf@gitster-ct.c.googlers.com","threadId":"52264","inReplyTo":"20191118120315.GB12766@sigill.intra.peff.net","subject":"Re: rev-list and \"ambiguous\" IDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T01:24:35Z","receivedAt":"2019-11-19T01:24:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think that changes the meaning of GET_OID_COMMITTISH, though. Right\n> now it means \"if disambiguating, prefer a committish\", but not \"I can\n> only accept a commit\". So we would still happily return an unambiguous\n> object that does not match that type.\n\nAh, OK, so I was stupid (not a news anymore ;-)\n\n> And that is why \"git -c\n> core.disambiguate=committish show $short_blob\" works, for example.\n\nYes, and it should work that way.\n\n> Perhaps a simple fix would just be for get_short_oid()'s error message\n> to mention the disambiguation rule. E.g., something like:\n>\n> $ git show abcd2\n> error: short SHA1 abcd2 is ambiguous\n> hint: We would have preferred a commit or tag pointing to a commit,\n> hint: but none were found. The candidates are:\n> hint: abcd22f55e tree\n> hint: abcd238df0 tree\n> hint: abcd2b1cc8 blob\n>\n> or\n>\n> $ git show abcd2\n> error: short SHA1 abcd2 is ambiguous\n> hint: We preferred a commit or tag pointing to a commit to other\n> hint: object types, but two candidates were found:\n> hint: abcd22f55e commit\n> hint: abcd238df0 commit\n> hint: abcd2b1cc8 blob\n>\n> (optionally the second one could even not mention the blob, though I\n> think with the lead-in sentence it's OK).\n>\n> The verbiage there isn't great (I was trying to avoid the jargon\n> \"committish\"), but hopefully you get the point.\n\nYup, if we were to do anything, this is a much more sensible thing\nto do than make GET_OID_ reject objects that are not of ,\nI think.\n\nThanks for a dose of sanity.\n"}]}