[PATCH v1] load_ref_decorations(): fix decoration with tags

["Martin Ågren" <martin.agren@gmail.com>]

Commit 88473c8bae ("load_ref_decorations(): avoid parsing non-tag
objects", 2021-06-22) introduced a shortcut to `add_ref_decoration()`:
Rather than calling `parse_object()` directly, call `oid_object_info()`
and then either return early or go on to call `lookup_object_by_type()`
using the type just discovered. As detailed in the commit message, this
provides a significant time saving.

Unfortunately, it also changes the behavior. As an example, on git.git,

  git log --oneline --decorate origin/master | grep '(.*tag:.*)'

returns zero hits after 88473c8bae. Before it, we have around 800 hits.
What happens is, all annotated tags are lost from the decoration.

Let's do a partial revert of 88473c8bae: Keep doing that early
`oid_object_info()`, but after that, go on with the full
`parse_object()`. This restores the pre-88473c8bae behavior. We clearly
have lacking test coverage here, so make sure to add a test. Without
this fix to log-tree.c, the git-log invocation in this new test does
pick up the lightweight tags involved, but misses out on the annotated
tag.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Cc-ing Peff, the author of 88473c8bae, and Taylor, who made a comment
 regarding the peeling of tags [1], which might be related.

 I'm genuinely unconvinced that my proposed fix is the best possible one.
 Or maybe trying a more surgical fix around annotated tags misses a
 whole bunch of *other* special cases and we really should go for the
 full `parse_object()` to cover all possibilities.

 In my brief testing (time git log -1 --decorate on git.git), the time
 savings from 88473c8bae seem to be gone. So maybe this should be a full
 revert, rather than a partial one. (Plus the test.) Let's hope that
 won't be necessary.

 Also, I'm not sure whether the test really needs to worry about the
 order of the decorations suddenly changing -- maybe it's supposed to be
 stable.

 [1] https://lore.kernel.org/git/YNKgkGkPiMgNubNE@nand.local/

 log-tree.c     |  6 ++----
 t/t4202-log.sh | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 4f69ed176d..0b638d2e3c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -134,7 +134,6 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 			      int flags, void *cb_data)
 {
 	struct object *obj;
-	enum object_type objtype;
 	enum decoration_type deco_type = DECORATION_NONE;
 	struct decoration_filter *filter = (struct decoration_filter *)cb_data;
 
@@ -156,10 +155,9 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
 		return 0;
 	}
 
-	objtype = oid_object_info(the_repository, oid, NULL);
-	if (objtype < 0)
+	if (oid_object_info(the_repository, oid, NULL) < 0)
 		return 0;
-	obj = lookup_object_by_type(the_repository, oid, objtype);
+	obj = parse_object(the_repository, oid);
 
 	if (starts_with(refname, "refs/heads/"))
 		deco_type = DECORATION_REF_LOCAL;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 350cfa3593..3aa5451913 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1905,6 +1905,20 @@ test_expect_success '--exclude-promisor-objects does not BUG-crash' '
 	test_must_fail git log --exclude-promisor-objects source-a
 '
 
+test_expect_success 'log --decorate includes, e.g., all kinds of tags' '
+	git log --oneline --decorate >log &&
+	test_line_count = 2 log &&
+	grep "^1ac6c77 (tag: one) one$" log &&
+	grep HEAD log | sed -e "s/^.*(\(.*\)).*$/\1/" -e "s/, /\n/g" |
+		sort >actual &&
+	cat >expect <<-\EOF &&
+		HEAD -> source-b
+		tag: source-tag
+		tag: three
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'log --end-of-options' '
        git update-ref refs/heads/--source HEAD &&
        git log --end-of-options --source >actual &&
-- 
2.32.0.402.g57bb445576