Re: git cherry-pick --strategy=resolve segfaults if picking a root commit
- From
Jeff King <peff@peff.net>
- Date
- May 12, 2011, 10:45 UTC
- Message-ID
- <20110512104558.GA22108@sigill.intra.peff.net>
- In-Reply-To
- <iqgbvs$pm5$1@dough.gmane.org>
On Thu, May 12, 2011 at 12:15:24PM +0200, Sebastian Schuberth wrote:
Show 6 quoted lines
> I just noticed that (under both Linux and Windows) Git 1.7.5 > segfaults for me if cherry-picking a root commit from one repo into > another one with "--strategy=resolve". It does not segfault if > "--strategy=resolve" is omitted. Here's what I did: > [...] > Is this already a known issue?
No, I think you found a new one (I doubt many people use alternate strategies for cherry-picking, and probably even less frequently on root commits).
Here's a fix, but see below.
--- builtin/revert.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c index f697e66..2eebb58 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -497,7 +497,8 @@ static int do_pick_commit(void) write_message(&msgbuf, defmsg); - commit_list_insert(base, &common); + if (base) + commit_list_insert(base, &common); commit_list_insert(next, &remotes); res = try_merge_command(strategy, xopts_nr, xopts, common, sha1_to_hex(head), remotes); The base commit comes from the "parent" pointer in a cherry, which is going to be NULL in this case. In a revert, however, the parent pointer ends up in "next". However, you can't get a segfault with "git revert --strategy=resolve $some_root_commit", because earlier we explicitly disallow reverting a root commit. So this patch makes it stop segfaulting, but I'm not sure that it is doing anything useful. Merge-resolve seems to just barf, whereas merge-recursive properly handles this case (it cherry-picks the difference between the empty tree and the root commit's tree). So probably we should: 1. Pass the empty tree along to merge-resolve. This will take a little bit of refactoring, but more importantly, it means we will be passing a tree-ish and not a commit-ish to a merge strategy. Is that OK? 2. Consider lifting the restriction on reverting root commits. If we can cherry-pick it, we can revert it, so I suspect this would already work with merge-recursive, but I didn't try. I don't care too much either way, though; I doubt it's something people would do a lot. It just seems like an unnecessary restriction. -Peff