| Message ID | patch-v4-1.3-2e7090c09f9-20211122T175219Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | object-name: make ambiguous object output translatable + show tag date | expand |
On Mon, Nov 22, 2021 at 06:53:23PM +0100, Ævar Arnfjörð Bjarmason wrote: > Remove unreachable "unknown type" handling in the code that displays > the ambiguous object list. See [1] for the current output, and [1] for > the commit that added the "unknown type" handling. > > The reason this code wasn't reachable is because we're not passing in > OBJECT_INFO_ALLOW_UNKNOWN_TYPE, so we'll die in sort_ambiguous() > before we get to show_ambiguous_object(): > > $ git rev-parse 8315 > error: short object ID 8315 is ambiguous > hint: The candidates are: > fatal: invalid object type I'm not so sure about this reasoning. In the code we are getting the type fresh from oid_object_info(): > @@ -361,6 +361,8 @@ static int show_ambiguous_object(const struct object_id *oid, void *data) > return 0; > > type = oid_object_info(ds->repo, oid, NULL); > + assert(type == OBJ_TREE || type == OBJ_COMMIT || > + type == OBJ_BLOB || type == OBJ_TAG); so at the very least we have to worry about the answer changing between the two spots. You talk above about ALLOW_UNKNOWN_TYPE, but can't we just get a straight "-1" if there's an error opening the object? I'm also confused about the mention of die in sort_ambiguous(). It looks like it would just produce a funny sort order in that case. Here's a case that triggers the difference: git init repo cd repo one=$(echo 851 | git hash-object -w --stdin) two=$(echo 872 | git hash-object -w --stdin) oid=$(echo $two | cut -c1-4) fn=.git/objects/$(echo $two | perl -pe 's{..}{$&/}') chmod +w $fn echo broken >$fn git show $oid Without your patch, it produces: error: short object ID ee3d is ambiguous hint: The candidates are: error: inflate: data stream error (incorrect header check) error: unable to unpack ee3d8abaa95a7395b373892b2593de2f426814e2 header error: inflate: data stream error (incorrect header check) error: unable to unpack ee3d8abaa95a7395b373892b2593de2f426814e2 header hint: ee3d8ab unknown type hint: ee3de99 blob With your patch: error: short object ID ee3d is ambiguous hint: The candidates are: error: inflate: data stream error (incorrect header check) error: unable to unpack ee3d8abaa95a7395b373892b2593de2f426814e2 header error: inflate: data stream error (incorrect header check) error: unable to unpack ee3d8abaa95a7395b373892b2593de2f426814e2 header git: object-name.c:364: show_ambiguous_object: Assertion `type == OBJ_TREE || type == OBJ_COMMIT || type == OBJ_BLOB || type == OBJ_TAG' failed. Aborted -Peff
diff --git a/object-name.c b/object-name.c index fdff4601b2c..59e934262e7 100644 --- a/object-name.c +++ b/object-name.c @@ -361,6 +361,8 @@ static int show_ambiguous_object(const struct object_id *oid, void *data) return 0; type = oid_object_info(ds->repo, oid, NULL); + assert(type == OBJ_TREE || type == OBJ_COMMIT || + type == OBJ_BLOB || type == OBJ_TAG); if (type == OBJ_COMMIT) { struct commit *commit = lookup_commit(ds->repo, oid); if (commit) { @@ -376,8 +378,7 @@ static int show_ambiguous_object(const struct object_id *oid, void *data) advise(" %s %s%s", repo_find_unique_abbrev(ds->repo, oid, DEFAULT_ABBREV), - type_name(type) ? type_name(type) : "unknown type", - desc.buf); + type_name(type), desc.buf); strbuf_release(&desc); return 0;
Remove unreachable "unknown type" handling in the code that displays the ambiguous object list. See [1] for the current output, and [1] for the commit that added the "unknown type" handling. The reason this code wasn't reachable is because we're not passing in OBJECT_INFO_ALLOW_UNKNOWN_TYPE, so we'll die in sort_ambiguous() before we get to show_ambiguous_object(): $ git rev-parse 8315 error: short object ID 8315 is ambiguous hint: The candidates are: fatal: invalid object type We should do better here, but let's leave that for some future improvement. In a subsequent commit I'll improve the output we do show, and not having to handle the "unknown type" case simplifies that change. Even though we know that this isn't reachable let's back that up with an assert() both for self-documentation and sanity checking. 1. 5cc044e0257 (get_short_oid: sort ambiguous objects by type, then SHA-1, 2018-05-10) 2. 1ffa26c461 (get_short_sha1: list ambiguous objects on error, 2016-09-26) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- object-name.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)