From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Matheus Tavares <matheus.bernardino@usp.br>,
git@vger.kernel.org, rhi@pengutronix.de
Subject: Re: [PATCH] describe: output tag's ref instead of embedded name
Date: Tue, 18 Feb 2020 14:54:02 -0500 [thread overview]
Message-ID: <20200218195402.GA21586@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqd0abk7zc.fsf@gitster-ct.c.googlers.com>
On Tue, Feb 18, 2020 at 11:31:35AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I think the conversion of the die() to warning() makes sense here. Do we
> > want to cover that with a test?
>
> I presume that you are talking about this case.
>
> > if (n->tag && !n->name_checked) {
> > if (!n->tag->tag)
> > - die(_("annotated tag %s has no embedded name"), n->path);
> > + warning(_("annotated tag %s has no embedded name"), n->path);
Yep, that's the one.
> The attached is my attempt to craft such a test. It turns out that
> it is tricky to trigger this die/warning. I haven't dug deeply
> enough, but I suspect this might be a dead code now.
Thanks for digging so deeply on this. I think you're right that we'll
never get a "struct tag" with a NULL tag field because
parse_tag_buffer() unconditionally puts something there or returns an
error.
We probably used to be able to trigger this by "double parsing" the tag
(probably with two refs pointing at the same tag object), in which case
the first would mark it as parsed but return an error, and the second
would quietly return "oh, it's already parsed". But I fixed that
recently in 228c78fbd4 (commit, tag: don't set parsed bit for parse
failures, 2019-10-25).
So yeah, I think I'd be OK to turn this into a BUG(), or just eliminate
it entirely (we'd segfault, which is BUG-equivalent ;) ).
-Peff
next prev parent reply other threads:[~2020-02-18 19:54 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-05 14:13 git-describe --tags warning: 'X' is really 'Y' here Roland Hieber
2020-02-05 17:15 ` Matheus Tavares Bernardino
2020-02-14 6:53 ` Jeff King
2020-02-14 16:57 ` Junio C Hamano
2020-02-15 21:34 ` [PATCH] describe: output tag's ref instead of embedded name Matheus Tavares
2020-02-16 6:51 ` Jeff King
2020-02-18 19:31 ` Junio C Hamano
2020-02-18 19:54 ` Jeff King [this message]
2020-02-18 23:05 ` Junio C Hamano
2020-02-18 23:28 ` Junio C Hamano
2020-02-19 1:57 ` Jeff King
2020-02-19 3:22 ` Junio C Hamano
2020-02-19 3:56 ` Jeff King
2020-02-19 11:14 ` Junio C Hamano
2020-02-20 11:25 ` Jeff King
2020-02-20 17:34 ` Junio C Hamano
2020-02-20 22:19 ` Matheus Tavares Bernardino
2020-02-20 22:59 ` Junio C Hamano
2020-02-21 1:33 ` Matheus Tavares
2020-02-21 2:05 ` Junio C Hamano
2020-02-21 6:00 ` Jeff King
2020-02-21 5:58 ` Jeff King
2020-02-19 10:08 ` Roland Hieber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200218195402.GA21586@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=matheus.bernardino@usp.br \
--cc=rhi@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).