From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Todd Zullinger" <tmz@pobox.com>,
"Petr Šplíchal" <psplicha@redhat.com>
Subject: [PATCH] checkout: avoid BUG() when hitting a broken repository
Date: Fri, 21 Jan 2022 16:58:30 -0800 [thread overview]
Message-ID: <xmqqbl04d1s9.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqqlez8d2e6.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 21 Jan 2022 16:45:21 -0800")
So, taking the two earlier comments from me together...
I _think_ I was the one who spotted the funny skip_prefix() whose
result was not used, and suggested this unrelated check, during the
review. Sorry about that.
----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] checkout: avoid BUG() when hitting a broken repository
When 9081a421 (checkout: fix "branch info" memory leaks, 2021-11-16)
cleaned up existing memory leaks, we added an unrelated sanity check
to ensure that a local branch is truly local and not a symref to
elsewhere that dies with BUG() otherwise. This was misguided in two
ways. First of all, such a tightening did not belong to a leak-fix
patch. And the condition it detected was *not* a bug in our program
but a problem in user data, where warning() or die() would have been
more appropriate.
As the condition is not fatal (the result of computing the local
branch name in the code that is involved in the faulty check is only
used as a textual label for the commit), let's revert the code to
the original state, i.e. strip "refs/heads/" to compute the local
branch name if possible, and otherwise leave it NULL. The consumer
of the information in merge_working_tree() is prepared to see NULL
in there and act accordingly.
cf. https://bugzilla.redhat.com/show_bug.cgi?id=2042920
Reported-by: Petr Šplíchal <psplicha@redhat.com>
Reported-by: Todd Zullinger <tmz@pobox.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/checkout.c | 3 ---
t/t2018-checkout-branch.sh | 13 +++++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 43d0275187..1fb34d537d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1094,9 +1094,6 @@ static int switch_branches(const struct checkout_opts *opts,
const char *p;
if (skip_prefix(old_branch_info.path, prefix, &p))
old_branch_info.name = xstrdup(p);
- else
- BUG("should be able to skip past '%s' in '%s'!",
- prefix, old_branch_info.path);
}
if (opts->new_orphan_branch && opts->orphan_from_empty_tree) {
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 93be1c0eae..5dda5ad4cb 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -85,6 +85,19 @@ test_expect_success 'setup' '
git branch -m branch1
'
+test_expect_success 'checkout a branch without refs/heads/* prefix' '
+ git clone --no-tags . repo-odd-prefix &&
+ (
+ cd repo-odd-prefix &&
+
+ origin=$(git symbolic-ref refs/remotes/origin/HEAD) &&
+ git symbolic-ref refs/heads/a-branch "$origin" &&
+
+ git checkout -f a-branch &&
+ git checkout -f a-branch
+ )
+'
+
test_expect_success 'checkout -b to a new branch, set to HEAD' '
test_when_finished "
git checkout branch1 &&
--
2.35.0-rc2-150-gc312dde8e9
next prev parent reply other threads:[~2022-01-22 0:58 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-20 16:51 [BUG] builtin/checkout.c:1098: should be able to skip past 'refs/heads/' Todd Zullinger
2022-01-20 17:04 ` Todd Zullinger
2022-01-20 21:26 ` [PATCH] checkout: fix BUG() case in 9081a421a6 Ævar Arnfjörð Bjarmason
2022-01-20 22:29 ` Junio C Hamano
2022-01-20 22:33 ` Junio C Hamano
2022-01-21 11:14 ` Ævar Arnfjörð Bjarmason
2022-01-21 14:29 ` Petr Šplíchal
2022-01-21 21:58 ` Todd Zullinger
2022-01-21 21:19 ` Junio C Hamano
2022-01-20 22:33 ` Todd Zullinger
2022-01-22 0:33 ` Junio C Hamano
2022-01-22 0:45 ` Junio C Hamano
2022-01-22 0:58 ` Junio C Hamano [this message]
2022-01-22 8:10 ` [PATCH] checkout: avoid BUG() when hitting a broken repository Johannes Sixt
2022-01-22 11:55 ` Ævar Arnfjörð Bjarmason
2022-01-23 16:38 ` Johannes Schindelin
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbl04d1s9.fsf_-_@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=psplicha@redhat.com \
--cc=tmz@pobox.com \
/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.
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 NNTP newsgroup(s).