checkout: do not lose staged removal

The logic to checkout a different commit implements the safety to never
lose user's local changes.  For example, switching from a commit to
another commit, when you have changed a path that is different between
them, need to merge your changes to the version from the switched-to
commit, which you may not necessarily be able to resolve easily.  By
default, "git checkout" refused to switch branches, to give you a chance
to stash your local changes (or use "-m" to merge, accepting the risks of
getting conflicts).

This safety, however, had one deliberate hole since early June 2005.  When
your local change was to remove a path (and optionally to stage that
removal), the command checked out the path from the switched-to commit
nevertheless.

This was to allow an initial checkout to happen smoothly (e.g. an initial
checkout is done by starting with an empty index and switching from the
commit at the HEAD to the same commit).  We can tighten the rule slightly
to allow this special case to pass, without losing sight of removal
explicitly done by the user, by noticing if the index is truly empty when
the operation begins.

For historical background, see:

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

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

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index 6f4b9b017f7b504a2b9e909639a61b1ef7750af0..309deac23b5bf8eea28441d34e78b7cdb3a02d28 100644 (file)
--- a/Documentation/git-read-tree.txt
+++ b/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
       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 "initial checkout"
+                                    H == M   keep index otherwise
+                                    exists   fail
+                                    H != M

 
         clean I==H  I==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.
 
 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
 ~~~~~~~~~~~
 
 3-Way Merge
 ~~~~~~~~~~~

diff --git a/builtin-checkout.c b/builtin-checkout.c
index f6f8f086de980b5bc19ca59985a2b0d8201831d9..774f29992a967201d1d4f39e6d2a9993d11a8eb7 100644 (file)
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -242,6 +242,8 @@ static int merge_working_tree(struct checkout_opts *opts,
                }
 
                /* 2-way merge to the new branch */
                }
 
                /* 2-way merge to the new branch */

+               topts.initial_checkout = (!active_nr &&
+                                         (old->commit == new->commit));

                topts.update = 1;
                topts.merge = 1;
                topts.gently = opts->merge;
                topts.update = 1;
                topts.merge = 1;
                topts.gently = opts->merge;

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index dddc3044b8e42d3083f1f6db0d9c205f01e14141..362216b272f36afc625c97501c34bac6be813eb0 100644 (file)
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -206,6 +206,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
                        break;
                case 2:
                        opts.fn = twoway_merge;
                        break;
                case 2:
                        opts.fn = twoway_merge;

+                       opts.initial_checkout = !active_nr;

                        break;
                case 3:
                default:
                        break;
                case 3:
                default:

diff --git a/unpack-trees.c b/unpack-trees.c
index ef21c62195d61980d4727e3f6d9c285422fcfe91..e59d144d28164f2451784513105f6269f0e9167c 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -941,8 +941,17 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)
                        return -1;
                }
        }
                        return -1;
                }
        }

-       else if (newtree)
+       else if (newtree) {
+               if (oldtree && !o->initial_checkout) {
+                       /*
+                        * deletion of the path was staged;
+                        */
+                       if (same(oldtree, newtree))
+                               return 1;
+                       return reject_merge(oldtree, o);
+               }

                return merged_entry(newtree, current, o);
                return merged_entry(newtree, current, o);

+       }

        return deleted_entry(oldtree, current, o);
 }
 
        return deleted_entry(oldtree, current, o);
 }
 

diff --git a/unpack-trees.h b/unpack-trees.h
index 94e567265af9a69a30dd5c578439b6444e50004d..0d26f3d73e773230972db86f34a5147ada881e8b 100644 (file)
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -26,6 +26,7 @@ struct unpack_trees_options {
                     verbose_update:1,
                     aggressive:1,
                     skip_unmerged:1,
                     verbose_update:1,
                     aggressive:1,
                     skip_unmerged:1,

+                    initial_checkout:1,

                     gently:1;
        const char *prefix;
        int pos;
                     gently:1;
        const char *prefix;
        int pos;