On Thu, May 12, 2011 at 12:15:24PM +0200, Sebastian Schuberth wrote:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html