reset: disallow using --keep when there are unmerged entries

The use case for --keep option is to remove previous commits unrelated
to the current changes in the working tree. So in this use case we are
not supposed to have unmerged entries. This is why it seems safer to
just disallow using --keep when there are unmerged entries.

And this patch changes the error message when --keep was disallowed and
there were some unmerged entries from:

error: Entry 'file1' would be overwritten by merge. Cannot merge.
fatal: Could not reset index file to revision 'HEAD^'.

to:

fatal: Cannot do a keep reset in the middle of a merge.

which is nicer.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 1e9ae0ae9fe4d2fc9e36c4904c6e6f39c2f37639..58d9b4c062d6d6d09f10fd2fad1edabddd961d92 100644 (file)
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -155,7 +155,8 @@ tree. If there could be conflicts between the changes in the commit we
 want to remove and the changes in the working tree we want to keep,
 the reset is disallowed. That's why it is disallowed if there are both
 changes between the working tree and HEAD, and between HEAD and the
 want to remove and the changes in the working tree we want to keep,
 the reset is disallowed. That's why it is disallowed if there are both
 changes between the working tree and HEAD, and between HEAD and the

-target.
+target. To be safe, it is also disallowed when there are unmerged
+entries.

 
 The following tables show what happens when there are unmerged
 entries:
 
 The following tables show what happens when there are unmerged
 entries:


@@ -174,7 +175,7 @@ entries:
                                --mixed  X       A     A
                                --hard   A       A     A
                                --merge  A       A     A
                                --mixed  X       A     A
                                --hard   A       A     A
                                --merge  A       A     A

-                               --keep   X       A     A
+                               --keep  (disallowed)

 
 X means any state and U means an unmerged index.
 
 
 X means any state and U means an unmerged index.
 

diff --git a/builtin-reset.c b/builtin-reset.c
index 52584afbdfa6696d7e8dbd2fda3ab2d97467b1f9..2c3a69adc674bb3f539d29b2d29116e89f887066 100644 (file)
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -224,6 +224,14 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
                warning("Reflog action message too long: %.*s...", 50, buf);
 }
 
                warning("Reflog action message too long: %.*s...", 50, buf);
 }
 

+static void die_if_unmerged_cache(int reset_type)
+{
+       if (is_merge() || read_cache() < 0 || unmerged_cache())
+               die("Cannot do a %s reset in the middle of a merge.",
+                   reset_type_names[reset_type]);
+
+}
+

 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
        int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
        int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;


@@ -329,11 +337,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
        /* Soft reset does not touch the index file nor the working tree
         * at all, but requires them in a good order.  Other resets reset
         * the index file to the tree object we are switching to. */
        /* Soft reset does not touch the index file nor the working tree
         * at all, but requires them in a good order.  Other resets reset
         * the index file to the tree object we are switching to. */

-       if (reset_type == SOFT) {
-               if (is_merge() || read_cache() < 0 || unmerged_cache())
-                       die("Cannot do a soft reset in the middle of a merge.");
-       } else {
-               int err = reset_index_file(sha1, reset_type, quiet);
+       if (reset_type == SOFT)
+               die_if_unmerged_cache(reset_type);
+       else {
+               int err;
+               if (reset_type == KEEP)
+                       die_if_unmerged_cache(reset_type);
+               err = reset_index_file(sha1, reset_type, quiet);

                if (reset_type == KEEP)
                        err = err || reset_index_file(sha1, MIXED, quiet);
                if (err)
                if (reset_type == KEEP)
                        err = err || reset_index_file(sha1, MIXED, quiet);
                if (err)

diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 1a9c1c74944130900c0b8d4bbb9b1787241511fd..70cdd8e618c648f7ee6550997d68c40d912c8db9 100755 (executable)
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -237,7 +237,7 @@ test_expect_success '"reset --keep HEAD^" fails with pending merge' '
     git reset --hard third &&
     test_must_fail git merge branch1 &&
     test_must_fail git reset --keep HEAD^ 2>err.log &&
     git reset --hard third &&
     test_must_fail git merge branch1 &&
     test_must_fail git reset --keep HEAD^ 2>err.log &&

-    grep file1 err.log | grep "overwritten by merge"
+    grep "middle of a merge" err.log

 '
 
 # The next test will test the following:
 '
 
 # The next test will test the following:


@@ -258,18 +258,15 @@ test_expect_success '"reset --merge HEAD" is ok with pending merge' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------

-# file1:     X       U     B    B     --keep  X       B     B
-test_expect_success '"reset --keep HEAD" is ok with pending merge' '
+# file1:     X       U     B    B     --keep   (disallowed)
+test_expect_success '"reset --keep HEAD" fails with pending merge' '

     git reset --hard third &&
     test_must_fail git merge branch1 &&
     git reset --hard third &&
     test_must_fail git merge branch1 &&

-    cat file1 >orig_file1 &&
-    git reset --keep HEAD &&
-    test "$(git rev-parse HEAD)" = "$(git rev-parse third)" &&
-    test -z "$(git diff --cached)" &&
-    test_cmp file1 orig_file1
+    test_must_fail git reset --keep HEAD 2>err.log &&
+    grep "middle of a merge" err.log

 '
 
 '
 

-test_expect_success '--merge with added/deleted' '
+test_expect_success '--merge is ok with added/deleted merge' '

     git reset --hard third &&
     rm -f file2 &&
     test_must_fail git merge branch3 &&
     git reset --hard third &&
     rm -f file2 &&
     test_must_fail git merge branch3 &&


@@ -283,7 +280,7 @@ test_expect_success '--merge with added/deleted' '
     git diff --exit-code --cached
 '
 
     git diff --exit-code --cached
 '
 

-test_expect_success '--keep with added/deleted' '
+test_expect_success '--keep fails with added/deleted merge' '

     git reset --hard third &&
     rm -f file2 &&
     test_must_fail git merge branch3 &&
     git reset --hard third &&
     rm -f file2 &&
     test_must_fail git merge branch3 &&


@@ -291,10 +288,8 @@ test_expect_success '--keep with added/deleted' '
     test -f file3 &&
     git diff --exit-code file3 &&
     git diff --exit-code branch3 file3 &&
     test -f file3 &&
     git diff --exit-code file3 &&
     git diff --exit-code branch3 file3 &&

-    git reset --keep HEAD &&
-    test -f file3 &&
-    ! test -f file2 &&
-    git diff --exit-code --cached
+    test_must_fail git reset --keep HEAD 2>err.log &&
+    grep "middle of a merge" err.log

 '
 
 test_done
 '
 
 test_done

diff --git a/t/t7111-reset-table.sh b/t/t7111-reset-table.sh
index 2ebda97828a83e849528b303ca4cda6f1ef84a40..ce421ad5ac4b3a17d0e347a7d86a386e2b14547b 100755 (executable)
--- a/t/t7111-reset-table.sh
+++ b/t/t7111-reset-table.sh
@@ -115,7 +115,7 @@ X U B B soft   XXXXX
 X U B B mixed  X B B
 X U B B hard   B B B
 X U B B merge  B B B
 X U B B mixed  X B B
 X U B B hard   B B B
 X U B B merge  B B B

-X U B B keep   X B B
+X U B B keep   XXXXX

 EOF
 
 test_done
 EOF
 
 test_done