| Message ID | 00270315b83b585f7d62ad1204ca1df93a668791.1733354035.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fixup! describe: stop traversing when we run out of names | expand |
On Wed, Dec 04, 2024 at 03:15:42PM -0800, Josh Steadmon wrote: > diff --git a/builtin/describe.c b/builtin/describe.c > index 8ec3be87df..065c1bde6e 100644 > --- a/builtin/describe.c > +++ b/builtin/describe.c > @@ -336,7 +336,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst) > return; > } > > - if (!max_candidates) > + if (!max_candidates && !always) > die(_("no tag exactly matches '%s'"), oid_to_hex(&cmit->object.oid)); > if (debug) > fprintf(stderr, _("No exact match on refs or tags, searching to describe\n")); Yep, this is the same spot I found. I think it's the right place to make the fix. > Subject: Re: [PATCH] fixup! describe: stop traversing when we run out of names This commit is already in 'next', so it's too late to squash in a change (though I'd have done this separately anyway, as it's already an issue for a manual --candidates=0 setting, as unlikely as that is). Can you re-send with a full commit message? > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh > index 5633b11d01..9aebf09d3d 100755 > --- a/t/t6120-describe.sh > +++ b/t/t6120-describe.sh > @@ -715,4 +715,18 @@ test_expect_success 'describe --broken --dirty with a file with changed stat' ' > ) > ' > > +test_expect_success '--always with no refs falls back to commit hash' ' > + git init always-no-refs && > + ( > + cd always-no-refs && > + test_commit --no-tag A && > + test_commit --no-tag B && > + test_commit --no-tag C && > + git describe --abbrev=12 --always HEAD^ >actual && > + echo 13 >expected_size && > + test_file_size actual >actual_size && > + test_cmp expected_size actual_size > + ) > +' I'm not sure if I'm missing anything subtle, but this seems more complicated than necessary to show the bug. I think just the exit code of: git describe --match=does-not-exist --always HEAD is sufficient, even in a repo with tags. If you really want to check stdout, then probably comparing against: git rev-list -1 --abbrev-commit --abbrev=13 HEAD >expect && test_cmp expect actual is a little more obvious than the size check. -Peff
On Wed, Dec 04, 2024 at 06:27:50PM -0500, Jeff King wrote: > > - if (!max_candidates) > > + if (!max_candidates && !always) > > die(_("no tag exactly matches '%s'"), oid_to_hex(&cmit->object.oid)); > > if (debug) > > fprintf(stderr, _("No exact match on refs or tags, searching to describe\n")); > > Yep, this is the same spot I found. I think it's the right place to make > the fix. > > > Subject: Re: [PATCH] fixup! describe: stop traversing when we run out of names > > This commit is already in 'next', so it's too late to squash in a change > (though I'd have done this separately anyway, as it's already an issue > for a manual --candidates=0 setting, as unlikely as that is). > > Can you re-send with a full commit message? In case it helps with writing a commit message: I wondered why this line was there at all. It comes from 2c33f75754 (Teach git-describe --exact-match to avoid expensive tag searches, 2008-02-24). The --exact-match option is implemented by setting max-candidates to 0. So: git describe --exact-match --always has always been broken, but probably nobody ever cared. My series reduces the max_candidates setting automatically when there is nothing to find, which means you are more likely to hit the bug. -Peff
diff --git a/builtin/describe.c b/builtin/describe.c index 8ec3be87df..065c1bde6e 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -336,7 +336,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst) return; } - if (!max_candidates) + if (!max_candidates && !always) die(_("no tag exactly matches '%s'"), oid_to_hex(&cmit->object.oid)); if (debug) fprintf(stderr, _("No exact match on refs or tags, searching to describe\n")); diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh index 5633b11d01..9aebf09d3d 100755 --- a/t/t6120-describe.sh +++ b/t/t6120-describe.sh @@ -715,4 +715,18 @@ test_expect_success 'describe --broken --dirty with a file with changed stat' ' ) ' +test_expect_success '--always with no refs falls back to commit hash' ' + git init always-no-refs && + ( + cd always-no-refs && + test_commit --no-tag A && + test_commit --no-tag B && + test_commit --no-tag C && + git describe --abbrev=12 --always HEAD^ >actual && + echo 13 >expected_size && + test_file_size actual >actual_size && + test_cmp expected_size actual_size + ) +' + test_done
Don't exit when we run out of names if we also set --always Signed-off-by: Josh Steadmon <steadmon@google.com> --- builtin/describe.c | 2 +- t/t6120-describe.sh | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) base-commit: a4f8a869558d59677e8d9798666a23391f0b4ca8