[RFC/PATCH 0/5] add "--keep" option to "git reset"

[RFC/PATCH 0/5] add "--keep" option to "git reset"

[Christian Couder]

This patch series is sent to make it easy to test the changes in the
first patch by Junio of this series on a new "--keep" reset option.  

Christian Couder (4):
  reset: add option "--keep" to "git reset"
  reset: add test cases for "--keep" option
  Documentation: reset: describe new "--keep" option
  reset: disallow "reset --keep" outside a work tree

Junio C Hamano (1):
  reset: make "reset --merge" discard work tree changes on unmerged
    entries

 Documentation/git-reset.txt |   45 ++++++++++++++--
 builtin-reset.c             |   31 +++++++++--
 read-cache.c                |    2 +-
 t/t7103-reset-bare.sh       |   25 ++++++---
 t/t7110-reset-merge.sh      |  119 ++++++++++++++++++++++++++++++++++++++++---
 unpack-trees.c              |   19 ++++---
 6 files changed, 206 insertions(+), 35 deletions(-)

[RFC/PATCH 1/5] reset: make "reset --merge" discard work tree changes on unmerged entries

[Christian Couder]

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

Commit 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) disallowed
"git reset --merge" when there was unmerged entries. It acknowledged
that this is not the best possible behavior, and that it would be better
if unmerged entries were reset as if --hard (instead of --merge) has
been used.

Recently another commit (reset: use "unpack_trees()" directly instead of
"git read-tree", 2009-12-30) changed the behavior of --merge to accept
resetting unmerged entries if they are reset to a different state than
HEAD, but it did not reset the changes in the work tree. So the behavior
was kind of improved, but it was not yet as if --hard has been used.

This patch finally makes "git reset --merge" behaves like
"git reset --hard" on unmerged entries.

To do that it does three things:

 - Updates the documentation to match the wish of original "reset
   --merge" better, namely, "An unmerged entry is a sign that the path
   didn't have any local modification and can be safely resetted to
   whatever the new HEAD records";

 - Updates read_index_unmerged(), which reads the index file into the
   cache while dropping any higher-stage entries down to stage #0, not
   to copy the object name recorded in the cache entry.  The code used to
   take the object name from the highest stage entry ("theirs" if you
   happened to have stage #3, or "ours" if they removed while you kept),
   which essentially meant that you are getting random results and
   didn't make sense.

   The _only_ reason we want to keep a previously unmerged entry in the
   index at stage #0 is so that we don't forget the fact that we have
   corresponding file in the work tree in order to be able to remove it
   when the tree we are resetting to does not have the path.  In order
   to differentiate such an entry from ordinary cache entry, the cache
   entry added by read_index_unmerged() records null sha1.

 - Updates merged_entry() and deleted_entry() so that they pay attention
   to cache entries with null sha1 (note that we _might_ want to use a
   new in-core ce->ce_flags instead of using the null-sha1 hack).  They
   are previously unmerged entries, and the files in the work tree that
   correspond to them are resetted away by oneway_merge() to the version
   from the tree we are resetting to.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-reset.txt |    4 ++--
 read-cache.c                |    2 +-
 t/t7110-reset-merge.sh      |   14 +++++++-------
 unpack-trees.c              |   19 ++++++++++++-------
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 2480be5..b78e2e1 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -123,14 +123,14 @@ entries:
        X       U     A    B     --soft  (disallowed)
 				--mixed  X       B     B
 				--hard   B       B     B
-				--merge  X       B     B
+				--merge  B       B     B
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
        X       U     A    A     --soft  (disallowed)
 				--mixed  X       A     A
 				--hard   A       A     A
-				--merge (disallowed)
+				--merge  A       A     A
 
 X means any state and U means an unmerged index.
 
diff --git a/read-cache.c b/read-cache.c
index 94b92d3..e07a6df 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1635,7 +1635,7 @@ int read_index_unmerged(struct index_state *istate)
 		len = strlen(ce->name);
 		size = cache_entry_size(len);
 		new_ce = xcalloc(1, size);
-		hashcpy(new_ce->sha1, ce->sha1);
+		/* don't copy sha1; this should not match anything */
 		memcpy(new_ce->name, ce->name, len);
 		new_ce->ce_flags = create_ce_flags(len, 0);
 		new_ce->ce_mode = ce->ce_mode;
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index ff2875c..249df7c 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -135,27 +135,27 @@ test_expect_success 'setup 2 different branches' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
-# file1:     X       U     B    C     --merge  X       C     C
+# file1:     X       U     B    C     --merge  C       C     C
 test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
     test_must_fail git merge branch1 &&
-    cat file1 >orig_file1 &&
     git reset --merge HEAD^ &&
     test "$(git rev-parse HEAD)" = "$(git rev-parse second)" &&
     test -z "$(git diff --cached)" &&
-    test_cmp file1 orig_file1
+    test -z "$(git diff)"
 '
 
 # The next test will test the following:
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
-# file1:     X       U     B    B     --merge  (disallowed)
-test_expect_success '"reset --merge HEAD" fails with pending merge' '
+# file1:     X       U     B    B     --merge  B       B     B
+test_expect_success '"reset --merge HEAD" is ok with pending merge' '
     git reset --hard third &&
     test_must_fail git merge branch1 &&
-    test_must_fail git reset --merge HEAD &&
+    git reset --merge HEAD &&
     test "$(git rev-parse HEAD)" = "$(git rev-parse third)" &&
-    test -n "$(git diff --cached)"
+    test -z "$(git diff --cached)" &&
+    test -z "$(git diff)"
 '
 
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 7570475..87f95a1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -809,7 +809,11 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 {
 	int update = CE_UPDATE;
 
-	if (old) {
+	if (!old) {
+		if (verify_absent(merge, "overwritten", o))
+			return -1;
+		invalidate_ce_path(merge, o);
+	} else if (!is_null_sha1(old->sha1)) {
 		/*
 		 * See if we can re-use the old CE directly?
 		 * That way we get the uptodate stat info.
@@ -827,11 +831,12 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 				update |= CE_SKIP_WORKTREE;
 			invalidate_ce_path(old, o);
 		}
-	}
-	else {
-		if (verify_absent(merge, "overwritten", o))
-			return -1;
-		invalidate_ce_path(merge, o);
+	} else {
+		/*
+		 * Previously unmerged entry left as an existence
+		 * marker by read_index_unmerged();
+		 */
+		invalidate_ce_path(old, o);
 	}
 
 	add_entry(o, merge, update, CE_STAGEMASK);
@@ -847,7 +852,7 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,
 			return -1;
 		return 0;
 	}
-	if (verify_uptodate(old, o))
+	if (!is_null_sha1(old->sha1) && verify_uptodate(old, o))
 		return -1;
 	add_entry(o, ce, CE_REMOVE, 0);
 	invalidate_ce_path(ce, o);
-- 
1.6.6.rc2.5.g49666

[RFC/PATCH 2/5] reset: add option "--keep" to "git reset"

[Christian Couder]

The purpose of this new option is to discard some of the
last commits but to keep current changes in the work tree.

The use case is when you work on something and commit
that work. And then you work on something else that touches
other files, but you don't commit it yet. Then you realize
that what you commited when you worked on the first thing
is not good or belongs to another branch.

So you want to get rid of the previous commits (at least in
the current branch) but you want to make sure that you keep
the changes you have in the work tree. And you are pretty
sure that your changes are independent from what you
previously commited, so you don't want the reset to succeed
if the previous commits changed a file that you also
changed in your work tree.

The table below shows what happens when running
"git reset --option target" to reset the HEAD to another
commit (as a special case "target" could be the same as
HEAD) in the cases where "--merge" and "--keep" behave
differently.

working index HEAD target         working index HEAD
----------------------------------------------------
  A      B     C     D   --keep    (disallowed)
                         --merge   (disallowed)
  A      B     C     C   --keep     A      C     C
                         --merge   (disallowed)
  B      B     C     D   --keep    (disallowed)
                         --merge    D      D     D
  B      B     C     C   --keep     B      C     C
                         --merge    C      C     C

In this table, A, B and C are some different states of
a file. For example the last line of the table means
that if a file is in state B in the working tree and
the index, and in a different state C in HEAD and in
the target, then "git reset --merge target" will put
the file in state C in the working tree, in the index
and in HEAD.

The following table shows what happens on unmerged entries:

working index HEAD target         working index HEAD
----------------------------------------------------
 X       U     A    B     --keep  (disallowed)
                          --merge  B       B     B
 X       U     A    A     --keep   X       A     A
                          --merge  A       A     A

In this table X can be any state and U means an unmerged
entry.

Though the error message when "reset --keep" is disallowed
on unmerged entries is something like:

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

which is not very nice.

A following patch will add some test cases for
"--keep", where the differences between "--merge" and
"--keep" can also be seen.

The "--keep" option is implemented by doing a 2 way merge
between HEAD and the reset target, and if this succeeds
by doing a mixed reset to the target.

The code comes from the sequencer GSoC project, where
such an option was developed by Stephan Beyer:

git://repo.or.cz/git/sbeyer.git

(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

But in the sequencer project the "reset" flag was set
in the "struct unpack_trees_options" passed to
"unpack_trees()". With this flag the changes in the
working tree were discarded if the file was different
between HEAD and the reset target.

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c |   29 ++++++++++++++++++++++++-----
 1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 0f5022e..da61f20 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -22,13 +22,15 @@
 #include "cache-tree.h"
 
 static const char * const git_reset_usage[] = {
-	"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
+	"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]",
 	"git reset [--mixed] <commit> [--] <paths>...",
 	NULL
 };
 
-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
-static const char *reset_type_names[] = { "mixed", "soft", "hard", "merge", NULL };
+enum reset_type { MIXED, SOFT, HARD, MERGE, KEEP, NONE };
+static const char *reset_type_names[] = {
+	"mixed", "soft", "hard", "merge", "keep", NULL
+};
 
 static char *args_to_str(const char **argv)
 {
@@ -71,6 +73,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 	if (!quiet)
 		opts.verbose_update = 1;
 	switch (reset_type) {
+	case KEEP:
 	case MERGE:
 		opts.update = 1;
 		break;
@@ -85,6 +88,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 
 	read_cache_unmerged();
 
+	if (reset_type == KEEP) {
+		unsigned char head_sha1[20];
+		if (get_sha1("HEAD", head_sha1))
+			return error("You do not have a valid HEAD.");
+		if (!fill_tree_descriptor(desc, head_sha1))
+			return error("Failed to find tree of HEAD.");
+		nr++;
+		opts.fn = twoway_merge;
+	}
+
 	if (!fill_tree_descriptor(desc + nr - 1, sha1))
 		return error("Failed to find tree of %s.", sha1_to_hex(sha1));
 	if (unpack_trees(nr, desc, &opts))
@@ -229,6 +242,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", HARD),
 		OPT_SET_INT(0, "merge", &reset_type,
 				"reset HEAD, index and working tree", MERGE),
+		OPT_SET_INT(0, "keep", &reset_type,
+				"reset HEAD but keep local changes", KEEP),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
@@ -317,9 +332,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	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 == KEEP)
+			err = err || reset_index_file(sha1, MIXED, quiet);
+		if (err)
+			die("Could not reset index file to revision '%s'.", rev);
 	}
-	else if (reset_index_file(sha1, reset_type, quiet))
-		die("Could not reset index file to revision '%s'.", rev);
 
 	/* Any resets update HEAD to the head being switched to,
 	 * saving the previous head in ORIG_HEAD before. */
-- 
1.6.6.rc2.5.g49666

[RFC/PATCH 3/5] reset: add test cases for "--keep" option

[Christian Couder]

This shows that with the "--keep" option, changes that are both in
the work tree and the index are kept in the work tree after the
reset (but discarded in the index). As with the "--merge" option,
changes that are in both the work tree and the index are discarded
after the reset.

In the case of unmerged entries, we can see that "git reset --merge"
resets everything to the target, while with "--keep" it works only
when the target state is the same as HEAD and the work tree is not
reset.

And this shows that otherwise "--merge" and "--keep" have the same
behavior.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t7110-reset-merge.sh |  105 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 104 insertions(+), 1 deletions(-)

diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 249df7c..a9b2fba 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2009 Christian Couder
 #
 
-test_description='Tests for "git reset --merge"'
+test_description='Tests for "git reset" with "--merge" and "--keep" options'
 
 . ./test-lib.sh
 
@@ -47,6 +47,30 @@ test_expect_success 'reset --merge is ok when switching back' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     C       C     C    D     --keep   D       D     D
+# file2:     C       D     D    D     --keep   C       D     D
+test_expect_success 'reset --keep is ok with changes in file it does not touch' '
+    git reset --hard second &&
+    cat file1 >file2 &&
+    git reset --keep HEAD^ &&
+    ! grep 4 file1 &&
+    grep 4 file2 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
+    test -z "$(git diff --cached)"
+'
+
+test_expect_success 'reset --keep is ok when switching back' '
+    git reset --keep second &&
+    grep 4 file1 &&
+    grep 4 file2 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse second)" &&
+    test -z "$(git diff --cached)"
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     B       B     C    D     --merge  D       D     D
 # file2:     C       D     D    D     --merge  C       D     D
 test_expect_success 'reset --merge discards changes added to index (1)' '
@@ -78,6 +102,18 @@ test_expect_success 'reset --merge is ok again when switching back (1)' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     B       B     C    D     --keep   (disallowed)
+test_expect_success 'reset --keep fails with changes in index in files it touches' '
+    git reset --hard second &&
+    echo "line 5" >> file1 &&
+    git add file1 &&
+    test_must_fail git reset --keep HEAD^
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     C       C     C    D     --merge  D       D     D
 # file2:     C       C     D    D     --merge  D       D     D
 test_expect_success 'reset --merge discards changes added to index (2)' '
@@ -104,6 +140,30 @@ test_expect_success 'reset --merge is ok again when switching back (2)' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     C       C     C    D     --keep   D       D     D
+# file2:     C       C     D    D     --keep   C       D     D
+test_expect_success 'reset --keep keeps changes it does not touch' '
+    git reset --hard second &&
+    echo "line 4" >> file2 &&
+    git add file2 &&
+    git reset --keep HEAD^ &&
+    grep 4 file2 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
+    test -z "$(git diff --cached)"
+'
+
+test_expect_success 'reset --keep keeps changes when switching back' '
+    git reset --keep second &&
+    grep 4 file2 &&
+    grep 4 file1 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse second)" &&
+    test -z "$(git diff --cached)"
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     A       B     B    C     --merge  (disallowed)
 test_expect_success 'reset --merge fails with changes in file it touches' '
     git reset --hard second &&
@@ -116,6 +176,22 @@ test_expect_success 'reset --merge fails with changes in file it touches' '
     grep file1 err.log | grep "not uptodate"
 '
 
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
+# file1:     A       B     B    C     --keep   (disallowed)
+test_expect_success 'reset --keep fails with changes in file it touches' '
+    git reset --hard second &&
+    echo "line 5" >> file1 &&
+    test_tick &&
+    git commit -m "add line 5" file1 &&
+    sed -e "s/line 1/changed line 1/" <file1 >file3 &&
+    mv file3 file1 &&
+    test_must_fail git reset --keep HEAD^ 2>err.log &&
+    grep file1 err.log | grep "not uptodate"
+'
+
 test_expect_success 'setup 2 different branches' '
     git reset --hard second &&
     git branch branch1 &&
@@ -148,6 +224,18 @@ test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     X       U     B    C     --keep   (disallowed)
+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 &&
+    grep file1 err.log | grep "overwritten by merge"
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     X       U     B    B     --merge  B       B     B
 test_expect_success '"reset --merge HEAD" is ok with pending merge' '
     git reset --hard third &&
@@ -158,4 +246,19 @@ test_expect_success '"reset --merge HEAD" is ok with pending merge' '
     test -z "$(git diff)"
 '
 
+# The next test will test the following:
+#
+#           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' '
+    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_done
-- 
1.6.6.rc2.5.g49666

[RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option

[Christian Couder]

and give an example to show how it can be used.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-reset.txt |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index b78e2e1..3d7c206 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,7 +8,7 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
+'git reset' [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]
 'git reset' [-q] [<commit>] [--] <paths>...
 'git reset' --patch [<commit>] [--] [<paths>...]
 
@@ -52,6 +52,11 @@ OPTIONS
 	and updates the files that are different between the named commit
 	and the current commit in the working tree.
 
+--keep::
+	Resets the index to match the tree recorded by the named commit,
+	but keep changes in the working tree. Aborts if the reset would
+	change files that are already changes in the working tree.
+
 -p::
 --patch::
 	Interactively select hunks in the difference between the index
@@ -86,6 +91,7 @@ reset options depending on the state of the files.
 				--mixed  A       D     D
 				--hard   D       D     D
 				--merge (disallowed)
+				--keep  (disallowed)
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -93,6 +99,7 @@ reset options depending on the state of the files.
 				--mixed  A       C     C
 				--hard   C       C     C
 				--merge (disallowed)
+				--keep   A       C     C
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -100,6 +107,7 @@ reset options depending on the state of the files.
 				--mixed  B       D     D
 				--hard   D       D     D
 				--merge  D       D     D
+				--keep  (disallowed)
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -107,13 +115,14 @@ reset options depending on the state of the files.
 				--mixed  B       C     C
 				--hard   C       C     C
 				--merge  C       C     C
+				--keep   B       C     C
 
 In these tables, A, B, C and D are some different states of a
 file. For example, the last line of the last table means that if a
 file is in state B in the working tree and the index, and in a
 different state C in HEAD and in the target, then "git reset
---merge target" will put the file in state C in the working tree,
-in the index and in HEAD.
+--keep target" will put the file in state B in the working tree,
+and in state C in the index and in HEAD.
 
 The following tables show what happens when there are unmerged
 entries:
@@ -124,6 +133,7 @@ entries:
 				--mixed  X       B     B
 				--hard   B       B     B
 				--merge  B       B     B
+				--keep  (disallowed)
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -131,6 +141,7 @@ entries:
 				--mixed  X       A     A
 				--hard   A       A     A
 				--merge  A       A     A
+				--keep   X       A     A
 
 X means any state and U means an unmerged index.
 
@@ -302,6 +313,30 @@ $ git add frotz.c                           <3>
 <2> This commits all other changes in the index.
 <3> Adds the file to the index again.
 
+Keep changes in working tree while discarding some previous commits::
++
+Suppose you are working on something and you commit it, and then you
+continue working a bit more, but now you think that what you have in
+your working tree should be in another branch that has nothing to do
+with what you commited previously. You can start a new branch and
+reset it while keeping the changes in your work tree.
++
+------------
+$ git tag start
+$ git branch branch1
+$ edit
+$ git commit ...                            <1>
+$ edit
+$ git branch branch2                        <2>
+$ git reset --keep start                    <3>
+------------
++
+<1> This commits your first edits in branch1.
+<2> This creates branch2, but unfortunately it contains the previous
+commit that you don't want in this branch.
+<3> This removes the unwanted previous commit, but this keeps the
+changes in your working tree.
+
 Author
 ------
 Written by Junio C Hamano <gitster@pobox.com> and Linus Torvalds <torvalds@osdl.org>
-- 
1.6.6.rc2.5.g49666

[RFC/PATCH 5/5] reset: disallow "reset --keep" outside a work tree

[Christian Couder]

It is safer and consistent with "--merge" and "--hard" resets to disallow
"git reset --keep" outside a work tree.

So let's just do that and add some tests while at it.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c       |    2 +-
 t/t7103-reset-bare.sh |   25 +++++++++++++++++--------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index da61f20..52584af 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -319,7 +319,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
-	if (reset_type == HARD || reset_type == MERGE)
+	if (reset_type != SOFT && reset_type != MIXED)
 		setup_work_tree();
 
 	if (reset_type == MIXED && is_bare_repository())
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index afb55b3..1eef93c 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -11,21 +11,26 @@ test_expect_success 'setup non-bare' '
 	git commit -a -m two
 '
 
-test_expect_success 'hard reset requires a worktree' '
+test_expect_success '"hard" reset requires a worktree' '
 	(cd .git &&
 	 test_must_fail git reset --hard)
 '
 
-test_expect_success 'merge reset requires a worktree' '
+test_expect_success '"merge" reset requires a worktree' '
 	(cd .git &&
 	 test_must_fail git reset --merge)
 '
 
-test_expect_success 'mixed reset is ok' '
+test_expect_success '"keep" reset requires a worktree' '
+	(cd .git &&
+	 test_must_fail git reset --keep)
+'
+
+test_expect_success '"mixed" reset is ok' '
 	(cd .git && git reset)
 '
 
-test_expect_success 'soft reset is ok' '
+test_expect_success '"soft" reset is ok' '
 	(cd .git && git reset --soft)
 '
 
@@ -40,19 +45,23 @@ test_expect_success 'setup bare' '
 	cd bare.git
 '
 
-test_expect_success 'hard reset is not allowed in bare' '
+test_expect_success '"hard" reset is not allowed in bare' '
 	test_must_fail git reset --hard HEAD^
 '
 
-test_expect_success 'merge reset is not allowed in bare' '
+test_expect_success '"merge" reset is not allowed in bare' '
 	test_must_fail git reset --merge HEAD^
 '
 
-test_expect_success 'mixed reset is not allowed in bare' '
+test_expect_success '"keep" reset is not allowed in bare' '
+	test_must_fail git reset --keep HEAD^
+'
+
+test_expect_success '"mixed" reset is not allowed in bare' '
 	test_must_fail git reset --mixed HEAD^
 '
 
-test_expect_success 'soft reset is allowed in bare' '
+test_expect_success '"soft" reset is allowed in bare' '
 	git reset --soft HEAD^ &&
 	test "`git show --pretty=format:%s | head -n 1`" = "one"
 '
-- 
1.6.6.rc2.5.g49666

Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option

[Andreas Schwab]

Christian Couder <chriscool@tuxfamily.org> writes:

> +--keep::
> +	Resets the index to match the tree recorded by the named commit,
> +	but keep changes in the working tree. Aborts if the reset would
> +	change files that are already changes in the working tree.

s/changes/changed/ in the last sentence.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option

[Daniel Convissor]

On Sat, Jan 02, 2010 at 06:39:32AM +0100, Christian Couder wrote:
> +++ b/Documentation/git-reset.txt
...
> +--keep::
> +	Resets the index to match the tree recorded by the named commit,
> +	but keep changes in the working tree. Aborts if the reset would
> +	change files that are already changes in the working tree.

Grammar suggestion for the last line there.  Change "already changes" to 
"already changed".  Or better yet, use a different word, such as "already 
modified", since "change" based words have been used so many times 
already in the sentence.

Thanks,

--Dan

-- 
 T H E   A N A L Y S I S   A N D   S O L U T I O N S   C O M P A N Y
            data intensive web and database programming
                http://www.AnalysisAndSolutions.com/
 4015 7th Ave #4, Brooklyn NY 11232  v: 718-854-0335 f: 718-854-0409

Re: [RFC/PATCH 1/5] reset: make "reset --merge" discard work tree changes on unmerged entries

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> From: Junio C Hamano <gitster@pobox.com>
>
> Commit 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) disallowed
> "git reset --merge" when there was unmerged entries. It acknowledged
> that this is not the best possible behavior, and that it would be better
> if unmerged entries were reset as if --hard (instead of --merge) has
> been used.
>
> Recently another commit (reset: use "unpack_trees()" directly instead of
> "git read-tree", 2009-12-30) changed the behavior of --merge to accept
> resetting unmerged entries if they are reset to a different state than
> HEAD, but it did not reset the changes in the work tree. So the behavior
> was kind of improved, but it was not yet as if --hard has been used.

It would be more honest if we said something like:

	It changed a safer "I can't do as asked, please do it by hand"
	into a more dangerous "I pretend that I did so but I didn't do the
	full job; you need to fix up the result but I am not telling you
	that you have to", which is a lot worse.

here instead (that is one reason why I said my fix-up was "squashable").

But that is a minor issue.

I have been thinking about two issues on this --merge change.  

>  - Updates merged_entry() and deleted_entry() so that they pay attention
>    to cache entries with null sha1 (note that we _might_ want to use a
>    new in-core ce->ce_flags instead of using the null-sha1 hack).  They
>    are previously unmerged entries, and the files in the work tree that
>    correspond to them are resetted away by oneway_merge() to the version
>    from the tree we are resetting to.

One is the use of ce_flags instead of relying on the 20-byte comparison I
said above, for both performance (minor) and future maintainability (much
bigger) concern.  I have a feeling that we will regret later that we used
the null_sha1 trick here, when we want to express another "special" kind
of cache entry in unrelated situations.  The use of null_sha1 hack was
expedient but I fell victim of the same mentality of declaring that this
is the _last_ such kind of special index entry and closing the door to
others who want to extend the system later with different kind of special
cache entry, which I often complain about myself to patches from other
people.

Another is that it _might_ make sense to use two-tree form of read-tree
machinery (but using a different version of unpack-trees.c::twoway_merge()
function), instead of the one-tree form of "we don't bother checking if
the index is consistent with HEAD and assume it is, and jump to the
target."

"git reset --merge $there" is about the situation where you started a
"mergy" operation (e.g. "git merge", "git am -3", "git rebase", ...) while
you had unrelated local changes in the work tree, and you want to go back
to the state before that operation $there (which is HEAD if the mergy
operation is "merge", but is different from HEAD if it was "am -3" and you
have successfully applied a patch or more already).  Linus may know and
won't use "reset --merge" in a situation where it is not suitable, but not
everybody is Linus.  Even though "reset --merge" may "correctly" work with
respect to the table you added to "git reset" documentation, it would do
something that may not "make sense" from the end-user's point of view when
used in a situation that it wasn't designed for.  Using the two-tree form
allows the machinery to inspect the difference between HEAD and the index,
and detect cases where "reset --merge" was attempted when it shouldn't.
For example, if stage #2 of an unmerged path does not match HEAD, we know
there is something wrong.

This latter issue is much bigger and needs a lot more thought, and I don't
think it should block the series from going forward at all.  But I think
it is worth keeping in the back of our heads.

Re: [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> The purpose of this new option is to discard some of the last commits
> but to keep current changes in the work tree.
>
> The use case is when you work on something and commit that work. And
> then you work on something else that touches other files, but you don't
> commit it yet. Then you realize that what you commited when you worked
> on the first thing is not good or belongs to another branch.
>
> So you want to get rid of the previous commits (at least in the current
> branch) but you want to make sure that you keep the changes you have in
> the work tree. And you are pretty sure that your changes are independent
> from what you previously commited, so you don't want the reset to
> succeed if the previous commits changed a file that you also changed in
> your work tree.
>
> The table below shows what happens when running "git reset --option
> target" to reset the HEAD to another commit (as a special case "target"
> could be the same as HEAD) in the cases where "--merge" and "--keep"
> behave differently.

I think this new option is unrelated to "--merge"; iow, the only relation
to it is that it is an option to the same command "git reset", so it is
related but it is related the same way and to the degree as "--mixed" is.

Thinking about it even more, if the number of commits you are resetting
away is zero in your use case (i.e. target is HEAD), shouldn't this new
mode of operation degenerate to "--mixed"?  So in that sense, it might
make sense to contrast it with "--mixed".

But let's try not to contrast it with anything else, and see how well it
stands on its own.  The below is my attempt.

> working index HEAD target         working index HEAD
> ----------------------------------------------------
>   A      B     C     D   --keep    (disallowed)
>   A      B     C     C   --keep     A      C     C
>   B      B     C     D   --keep    (disallowed)
>   B      B     C     C   --keep     B      C     C

Let's give an explanation of the above in terms of what this means to the
end users.

    When you have changes to a path that the accumulated commits between
    target..HEAD touch, you don't want to discard them.  It doesn't matter
    if the changes in the work tree has been partially added (A != B) or
    fully added (A == B) to the index.  In both cases, the operation is
    disallowed, just like "checkout $another_branch" stops in such a case.

    But if you have local modifications based on one version and dropping
    the accumulated commits between target..HEAD does not involve that
    path, we can safely "transplant" that change to the target.

Presented this way, a future direction (iow, I am not suggesting you to do
this before the current series solidifies) might be to allow users to do
something similar to "checkout -m $the_other_branch".  IOW, instead of
disallowing "I have changed from C to B and switching to D" case, perform
a three-way merge to bring the work tree file to (D+(B-C)), transplanting
the local change you made on top of the target.

> The following table shows what happens on unmerged entries:
>
> working index HEAD target         working index HEAD
> ----------------------------------------------------
>  X       U     A    B     --keep  (disallowed)
>  X       U     A    A     --keep   X       A     A

In a sense, this is consistent with the above; the local change attempted
happens to be an unmerged result.

But it is inconsistent with the intended use case you presented, which
leaves no room for unmerged entries to enter in the index to begin with.
It might be safer to error out on any unmerged entry in the index.  I
dunno.

Re: [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"

[Christian Couder]

On samedi 02 janvier 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > The purpose of this new option is to discard some of the last commits
> > but to keep current changes in the work tree.
> >
> > The use case is when you work on something and commit that work. And
> > then you work on something else that touches other files, but you don't
> > commit it yet. Then you realize that what you commited when you worked
> > on the first thing is not good or belongs to another branch.
> >
> > So you want to get rid of the previous commits (at least in the current
> > branch) but you want to make sure that you keep the changes you have in
> > the work tree. And you are pretty sure that your changes are
> > independent from what you previously commited, so you don't want the
> > reset to succeed if the previous commits changed a file that you also
> > changed in your work tree.
> >
> > The table below shows what happens when running "git reset --option
> > target" to reset the HEAD to another commit (as a special case "target"
> > could be the same as HEAD) in the cases where "--merge" and "--keep"
> > behave differently.
>
> I think this new option is unrelated to "--merge"; iow, the only relation
> to it is that it is an option to the same command "git reset", so it is
> related but it is related the same way and to the degree as "--mixed" is.
>
> Thinking about it even more, if the number of commits you are resetting
> away is zero in your use case (i.e. target is HEAD), shouldn't this new
> mode of operation degenerate to "--mixed"?  So in that sense, it might
> make sense to contrast it with "--mixed".
>
> But let's try not to contrast it with anything else, and see how well it
> stands on its own. 

Ok, I removed parts of the commit messages that contrasted it 
with "--merge".

[...]

> > The following table shows what happens on unmerged entries:
> >
> > working index HEAD target         working index HEAD
> > ----------------------------------------------------
> >  X       U     A    B     --keep  (disallowed)
> >  X       U     A    A     --keep   X       A     A
>
> In a sense, this is consistent with the above; the local change attempted
> happens to be an unmerged result.
>
> But it is inconsistent with the intended use case you presented, which
> leaves no room for unmerged entries to enter in the index to begin with.
> It might be safer to error out on any unmerged entry in the index.  I
> dunno.

Yeah I agree it might be safer, so I added a patch to disallow using --keep 
when there are unmerged entries.

Thanks,
Christian.

Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option

[Christian Couder]

On samedi 02 janvier 2010, Daniel Convissor wrote:
> On Sat, Jan 02, 2010 at 06:39:32AM +0100, Christian Couder wrote:
> > +++ b/Documentation/git-reset.txt
>
> ...
>
> > +--keep::
> > +	Resets the index to match the tree recorded by the named commit,
> > +	but keep changes in the working tree. Aborts if the reset would
> > +	change files that are already changes in the working tree.
>
> Grammar suggestion for the last line there.  Change "already changes" to
> "already changed".  Or better yet, use a different word, such as "already
> modified", since "change" based words have been used so many times
> already in the sentence.

Ok I used "already modified".

Thanks,
Christian.