checkout -m dumping core

Previous thread: [PATCH RFC] gitk: display submodule diffs with appropriate encoding by Kirill Smelkov on Tuesday, January 5, 2010 - 5:44 am. (2 messages)

Next thread: Re: checkout -m dumping core by =?UTF-8?Q?Daniel?= on Wednesday, January 6, 2010 - 1:20 am. (1 message)
From: Tomas Carnecky
Date: Tuesday, January 5, 2010 - 11:15 am

git version 1.6.6.78.gbd757c

HEAD points to a non-existent branch refs/heads/master. Normal checkout 
origin fails with:
error: Entry '.cvsignore' would be overwritten by merge. Cannot merge.
(the working tree does indeed contain this file). So I tried checkout -m 
and git crashed. Workaround for me was reset --hard origin; checkout 
origin. I have the repository backed up in case someone wants to try 
themselves.

$ gdb `which git`
GNU gdb 6.8
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i386-pc-solaris2.11"...
(gdb) run checkout -m origin
Starting program: /export/home/tomc/local/git/bin/git checkout -m origin
warning: Lowest section in /lib/libpthread.so.1 is .dynamic at 00000074

Program received signal SIGSEGV, Segmentation fault.
0x080788fa in cmd_checkout (argc=0, argv=0x8047538, prefix=0x0) at 
builtin-checkout.c:450
450                             merge_trees(&o, new->commit->tree, work,
(gdb) list
445                             ret = reset_tree(new->commit->tree, 
opts, 1);
446                             if (ret)
447                                     return ret;
448                             o.branch1 = new->name;
449                             o.branch2 = "local";
450                             merge_trees(&o, new->commit->tree, work,
451                                     old->commit->tree, &result);
452                             ret = reset_tree(new->commit->tree, 
opts, 0);
453                             if (ret)
454                                     return ret;
(gdb) p o
$1 = {branch1 = 0x8047650 "origin", branch2 = 0x0, subtree_merge = 0, 
buffer_output = 1, verbosity = 0, diff_rename_limit = -1, 
merge_rename_limit = -1, call_depth = 0, obuf = {alloc = ...
From: =?UTF-8?Q?Daniel?=
Date: Wednesday, January 6, 2010 - 1:11 am

Does this patch help?

---
From b2203bded22db1a496ee3c9f6f5f4a384a8ccefa Mon Sep 17 00:00:00 2001
From: Daniel Baranski <mjucde@o2.pl>
Date: Wed, 6 Jan 2010 08:58:21 +0100
Subject: [PATCH] checkout -m: Fix SEGFAULT if HEAD is not valid.

Signed-off-by: Daniel Barański <mjucde@o2.pl>
---
 builtin-checkout.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..0ab59b2 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -422,7 +422,8 @@ static int merge_working_tree(struct checkout_opts *opts,
                        struct merge_options o;
                        if (!opts->merge)
                                return 1;
-                       parse_commit(old->commit);
+                       if (!parse_commit(old->commit))
+                               die("Couldn't parse commit '%s'", old->path);

                        /* Do more real merge */

-- 
1.6.5.6

--

From: Junio C Hamano
Date: Wednesday, January 6, 2010 - 10:07 pm

There actually is no point running parse_commit() at that point, as the
code obtained old->commit from lookup_commit_reference_gently() in
switch_branches() which must have already parsed it.  We do use it to try
switching the branches before falling back to the --merge mode (or we
notice old->commit does not exist and switch from an empty tree).

Instead of silently dying without diagnosing what paths are problematic
like your patch does, we can unsquelch the error from the unpack_tree() we
run before falling back to the --merge mode, and allow it to notice and
report the paths that will be overwritten (since you do not have HEAD,
everything in the work tree will be overwritten).  Perhaps like this.

 builtin-checkout.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..2708669 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -397,7 +397,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.initial_checkout = is_cache_unborn();
 		topts.update = 1;
 		topts.merge = 1;
-		topts.gently = opts->merge;
+		topts.gently = opts->merge && old->commit;
 		topts.verbose_update = !opts->quiet;
 		topts.fn = twoway_merge;
 		topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -422,7 +422,13 @@ static int merge_working_tree(struct checkout_opts *opts,
 			struct merge_options o;
 			if (!opts->merge)
 				return 1;
-			parse_commit(old->commit);
+
+			/*
+			 * Without old->commit, the below is the same as
+			 * the two-tree unpack we already tried and failed.
+			 */
+			if (!old->commit)
+				return 1;
 
 			/* Do more real merge */
 
--

Previous thread: [PATCH RFC] gitk: display submodule diffs with appropriate encoding by Kirill Smelkov on Tuesday, January 5, 2010 - 5:44 am. (2 messages)

Next thread: Re: checkout -m dumping core by =?UTF-8?Q?Daniel?= on Wednesday, January 6, 2010 - 1:20 am. (1 message)