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@xxxxxxxxx>
---
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
