[PATCH v1] load_ref_decorations(): fix decoration with tags
flat view
STALE1915d
From: Martin Ågren <hidden>
Date: 2021-07-12 22:42:40
Subsystem:
the rest · Maintainer:
Linus Torvalds
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 <redacted>
---
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/ (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