file deletion in index lost after checkout -b

file deletion in index lost after checkout -b

[Jing Xue]

In Git 1.6.0 the following sequence:

$ git init
$ echo 'abcdefgh' >1.txt
$ echo '12345678' >2.txt
$ git add 1.txt 2.txt
$ git commit -m 'init'
$ git rm 2.txt
$ echo 'qwertyuiop' >>1.txt
$ git add 1.txt
$ echo 'asdfghjkl;' >3.txt
$ git add 3.txt
$ git status
$ git checkout -b foo
$ git status

produces this output:

Initialized empty Git repository in /home/jingxue/workspace/sandboxes/test.git/.git/
Created initial commit 918f3c6: init
 2 files changed, 2 insertions(+), 0 deletions(-)
 create mode 100644 1.txt
 create mode 100644 2.txt
rm '2.txt'
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   1.txt
#       deleted:    2.txt
#       new file:   3.txt
#
M       1.txt
A       3.txt
Switched to a new branch "foo"
# On branch foo
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   1.txt
#       new file:   3.txt
#

The deletion of 2.txt appears lost during 'checkout -b foo', while the
modification and addition were both brought over. Is it a bug?

Cheers.
-- 
Jing Xue

Re: file deletion in index lost after checkout -b

[Junio C Hamano]

Jing Xue <jingxue@digizenstudio.com> writes:

[jc: please redirect an answer _meant for you_ off to the list with M-F-T header]

> In Git 1.6.0 the following sequence:
> ...
> The deletion of 2.txt appears lost during 'checkout -b foo', while the
> modification and addition were both brought over. Is it a bug?

This behaviour is unchanged since early June 2005.

    http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646

This is exactly the case marked as *0*, which both Linus and I said "it
feels somewhat wrong but otherwise we cannot start from an empty index".

We may want to do better this time around, though.

Re: file deletion in index lost after checkout -b

[Jing Xue]

On Thu, Sep 04, 2008 at 11:12:26PM -0700, Junio C Hamano wrote:
> [jc: please redirect an answer _meant for you_ off to the list with M-F-T header]

I changed mutt to not use M-F-T with the git list at all. Hope this one
turns out better.

> > The deletion of 2.txt appears lost during 'checkout -b foo', while the
> > modification and addition were both brought over. Is it a bug?
> 
> This behaviour is unchanged since early June 2005.
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646
> 
> This is exactly the case marked as *0*, which both Linus and I said "it
> feels somewhat wrong but otherwise we cannot start from an empty index".
> 
> We may want to do better this time around, though.

I have since found out that:

1. file deletions in the working directory but not in index would not be forgotten. That
makes "file deletions in index" case rather a corner one.

2. "checkout -b -m" would do the right thing.

Cheers.
-- 
Jing

Re: file deletion in index lost after checkout -b

[Junio C Hamano]

Jing Xue <jingxue@digizenstudio.com> writes:

> On Thu, Sep 04, 2008 at 11:12:26PM -0700, Junio C Hamano wrote:
>> [jc: please redirect an answer _meant for you_ off to the list with M-F-T header]
>
> I changed mutt to not use M-F-T with the git list at all. Hope this one
> turns out better.

Thanks; I meant to say "please do not redirect", but you got what I wanted
to say correctly.

>> > The deletion of 2.txt appears lost during 'checkout -b foo', while the
>> > modification and addition were both brought over. Is it a bug?
>> 
>> This behaviour is unchanged since early June 2005.
>> 
>>     http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646
>> 
>> This is exactly the case marked as *0*, which both Linus and I said "it
>> feels somewhat wrong but otherwise we cannot start from an empty index".
>> 
>> We may want to do better this time around, though.
>
> I have since found out that:
>
> 1. file deletions in the working directory but not in index would not be forgotten. That
> makes "file deletions in index" case rather a corner one.
>
> 2. "checkout -b -m" would do the right thing.

Both correct.  1. does not involve case *0*; 2. does not do two-tree
switch but internally uses three-tree switch and uses different codepath.

Re* file deletion in index lost after checkout -b

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jing Xue <jingxue@digizenstudio.com> writes:
> ...
>> In Git 1.6.0 the following sequence:
>> ...
>> The deletion of 2.txt appears lost during 'checkout -b foo', while the
>> modification and addition were both brought over. Is it a bug?
>
> This behaviour is unchanged since early June 2005.
>
>     http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646
>
> This is exactly the case marked as *0*, which both Linus and I said "it
> feels somewhat wrong but otherwise we cannot start from an empty index".
>
> We may want to do better this time around, though.

Try this patch.

 Documentation/git-read-tree.txt |   11 ++++++++++-
 builtin-checkout.c              |    1 +
 builtin-read-tree.c             |    1 +
 unpack-trees.c                  |   11 ++++++++++-
 unpack-trees.h                  |    1 +
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git c/Documentation/git-read-tree.txt i/Documentation/git-read-tree.txt
index 6f4b9b0..24155ab 100644
--- c/Documentation/git-read-tree.txt
+++ i/Documentation/git-read-tree.txt
@@ -160,7 +160,10 @@ Here are the "carry forward" rules:
       0 nothing             nothing  nothing  (does not happen)
       1 nothing             nothing  exists   use M
       2 nothing             exists   nothing  remove path from index
-      3 nothing             exists   exists   use M
+      3 nothing             exists   exists,  use M if index is empty
+				     H == M   keep index otherwise
+				     exists   fail
+				     H != M
 
         clean I==H  I==M
        ------------------
@@ -207,6 +210,12 @@ you picked it up via e-mail in a patch form), `git diff-index
 merge, but it would not show in `git diff-index --cached $M`
 output after two-tree merge.
 
+Case #3 is slightly tricky and needs explanation.  The result from this
+rule logically should be to remove the path if the user staged the removal
+of the path and then swiching to a new branch.  That however will prevent
+the initial checkout from happening, so the rule is modified to use M (new
+tree) only when the contents of the index is empty.  Otherwise the removal
+of the path is kept as long as $H and $M are the same.
 
 3-Way Merge
 ~~~~~~~~~~~
diff --git c/builtin-checkout.c i/builtin-checkout.c
index efdb1e0..c73a815 100644
--- c/builtin-checkout.c
+++ i/builtin-checkout.c
@@ -242,6 +242,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		}
 
 		/* 2-way merge to the new branch */
+		topts.index_was_empty = !active_nr;
 		topts.update = 1;
 		topts.merge = 1;
 		topts.gently = opts->merge;
diff --git c/builtin-read-tree.c i/builtin-read-tree.c
index dddc304..41ece57 100644
--- c/builtin-read-tree.c
+++ i/builtin-read-tree.c
@@ -160,6 +160,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 				die("you need to resolve your current index first");
 			stage = 1;
 			opts.merge = 1;
+			opts.index_was_empty = !active_nr;
 			continue;
 		}
 
diff --git c/unpack-trees.c i/unpack-trees.c
index ef21c62..43ff477 100644
--- c/unpack-trees.c
+++ i/unpack-trees.c
@@ -941,8 +941,17 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)
 			return -1;
 		}
 	}
-	else if (newtree)
+	else if (newtree) {
+		if (oldtree && !o->index_was_empty) {
+			/*
+			 * deletion of the path was staged;
+			 */
+			if (same(oldtree, newtree))
+				return 1;
+			return reject_merge(oldtree, o);
+		}
 		return merged_entry(newtree, current, o);
+	}
 	return deleted_entry(oldtree, current, o);
 }
 
diff --git c/unpack-trees.h i/unpack-trees.h
index 94e5672..61d82ce 100644
--- c/unpack-trees.h
+++ i/unpack-trees.h
@@ -26,6 +26,7 @@ struct unpack_trees_options {
 		     verbose_update:1,
 		     aggressive:1,
 		     skip_unmerged:1,
+		     index_was_empty:1,
 		     gently:1;
 	const char *prefix;
 	int pos;

Re: Re* file deletion in index lost after checkout -b

[Jing Xue]

On Sun, Sep 07, 2008 at 07:49:25PM -0700, Junio C Hamano wrote:
> Try this patch.

Works well. It doesn't give the fatal warning from "checkout -m" either.
"make test" shows it doesn't break any existing tests.

Thanks.
-- 
Jing