From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Denton Liu <liu.denton@gmail.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] checkout -p: handle tree arguments correctly again
Date: Sat, 19 Dec 2020 09:40:43 -0800 [thread overview]
Message-ID: <xmqq5z4xpuck.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <pull.819.git.1608389760050.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Sat, 19 Dec 2020 14:55:59 +0000")
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> I literally just ran into this segmentation fault after rebasing Git for
> Windows onto -rc1, and did not really think that the regression was
> introduced in the v2.30.0 cycle, but was proven wrong by my
> investigation: it was introduced by v2.30.0-rc0151^23.
As "checkout -p branch~4" is easier than "checkout -p branch~4:", I
think you have to work harder to give a true tree object rather than
the containing commit object.
It still is curious how you stumbled onto this, but I am glad you
did ;-)
> @@ -482,7 +482,7 @@ static int checkout_paths(const struct checkout_opts *opts,
> * properly. However, there is special logic for the HEAD case
> * so we mustn't replace that.
> */
> - if (rev && strcmp(rev, "HEAD"))
> + if (rev && new_branch_info->commit && strcmp(rev, "HEAD"))
The comment above talks about why "&& strcmp()" is there. We now
have two.
I'd squash this in while queuing. Thanks.
builtin/checkout.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git c/builtin/checkout.c w/builtin/checkout.c
index 6f24b381c5..728c67e8ca 100644
--- c/builtin/checkout.c
+++ w/builtin/checkout.c
@@ -479,7 +479,9 @@ static int checkout_paths(const struct checkout_opts *opts,
* with the hex of the commit (whether it's in `...` form or
* not) for the run_add_interactive() machinery to work
* properly. However, there is special logic for the HEAD case
- * so we mustn't replace that.
+ * so we mustn't replace that. Also, when we were given a
+ * tree-object, new_branch_info->commit would be NULL, but we
+ * do not have to do any replacement, either.
*/
if (rev && new_branch_info->commit && strcmp(rev, "HEAD"))
rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
prev parent reply other threads:[~2020-12-19 17:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-19 14:55 [PATCH] checkout -p: handle tree arguments correctly again Johannes Schindelin via GitGitGadget
2020-12-19 15:02 ` Denton Liu
2020-12-19 17:34 ` Junio C Hamano
2020-12-19 17:40 ` Junio C Hamano [this message]
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=xmqq5z4xpuck.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
--cc=liu.denton@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.