Git development
 help / color / mirror / Atom feed
* [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

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

^ permalink raw reply related

* [RFC/PATCH 3/5] reset: add test cases for "--keep" option
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

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

^ permalink raw reply related

* [RFC/PATCH 1/5] reset: make "reset --merge" discard work tree changes on unmerged entries
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

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

^ permalink raw reply related

* [RFC/PATCH 0/5] add "--keep" option to "git reset"
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd

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(-)

^ permalink raw reply

* Re: [PATCH v6 4/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2010-01-02  5:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <7vhbr6mhn9.fsf@alter.siamese.dyndns.org>

Hi and happy new year!

On vendredi 01 janvier 2010, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > At least disallowing means that the user _is notified_ and has to
> > manually deal with the situation.  Pretending it succeeded by resetting
> > only the index while still leaving the conflicted state in the work
> > tree intact is a bit worse in that sense.
> >
> >> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> >> index c9044c9..b40999f 100644
> >> --- a/Documentation/git-reset.txt
> >> +++ b/Documentation/git-reset.txt
> >> @@ -122,7 +122,7 @@ entries:
> >>         X       U     A    B     --soft  (disallowed)
> >>                                  --mixed  X       B     B
> >>                                  --hard   B       B     B
> >> -                                --merge (disallowed)
> >> +                                --merge  X       B     B
> >
> > IOW, I think the result should be "B B B" instead of "X B B" in this
> > case.
>
> A squashable fix-up on top of your patch to match the wish in the part
> you quoted from 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01)
> would look like this, I think.
>
> 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.
>
> Please take this with a grain of salt as I am under slight influence of
> CH3-CH2-OH while writing it, and I usually almost never drink.

I like this patch. It seems to improve the behavior of the --keep option for 
unmerged entries too.

The previous behavior was:

    working index HEAD target         working index HEAD
    ----------------------------------------------------
     X       U     A    B     --keep   X       B     B
     X       U     A    A     --keep   X       A     A

and now it is:

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

And I think it is better, as it is more consistent with the behavior when 
there are no unmerged entries.

The only problem is that when it fails the error message 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.

I will send a RFC patch series so people interested can test this.

Thanks,
Christian.

^ permalink raw reply

* [PATCH 8/6] t4030, t4031: work around bogus MSYS bash path conversion
From: Johannes Sixt @ 2010-01-01 22:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <4B3E73AE.6050003@kdbg.org>

Recall that MSYS bash converts POSIX style absolute paths to Windows style
absolute paths. Unfortunately, it converts a program argument that begins
with a double-quote and otherwise looks like an absolute POSIX path, but
in doing so, it strips everything past the second double-quote[*]. This
case is triggered in the two test scripts. The work-around is to place the
Windows style path between the quotes to avoid the path conversion.

[*] It is already bogus that a conversion is even considered when a program
argument begins with a double-quote because it cannot be an absolute POSIX
path.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  t/t4030-diff-textconv.sh       |    2 +-
  t/t4031-diff-rewrite-binary.sh |    2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 3cb7e63..9b94647 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -50,7 +50,7 @@ test_expect_success 'file is considered binary by plumbing' '

  test_expect_success 'setup textconv filters' '
  	echo file diff=foo >.gitattributes &&
-	git config diff.foo.textconv "\"$PWD\""/hexdump &&
+	git config diff.foo.textconv "\"$(pwd)\""/hexdump &&
  	git config diff.fail.textconv false
  '

diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
index 27fb31b..7e7b307 100755
--- a/t/t4031-diff-rewrite-binary.sh
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -54,7 +54,7 @@ chmod +x dump

  test_expect_success 'setup textconv' '
  	echo file diff=foo >.gitattributes &&
-	git config diff.foo.textconv "\"$PWD\""/dump
+	git config diff.foo.textconv "\"$(pwd)\""/dump
  '

  test_expect_success 'rewrite diff respects textconv' '
-- 
1.6.6.1073.gd853b.dirty

^ permalink raw reply related

* [PATCH 7/6] t0021: use $SHELL_PATH for the filter script
From: Johannes Sixt @ 2010-01-01 22:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20091230110335.GF22959@coredump.intra.peff.net>

On Windows, we need the shbang line to correctly invoke shell scripts via
a POSIX shell, except when the script is invoked via 'sh -c' because
sh (a bash) does "the right thing". Since nowadays the clean and smudge
filters are not always invoked via 'sh -c' anymore, we have to mark the
the one in t0021-conversion with #!$SHELL_PATH.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  t/t0021-conversion.sh    |    3 ++-
  t/t4030-diff-textconv.sh |    6 ++++--
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 8fc39d7..6cb8d60 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -4,7 +4,8 @@ test_description='blob conversion via gitattributes'

  . ./test-lib.sh

-cat <<\EOF >rot13.sh
+cat <<EOF >rot13.sh
+#!$SHELL_PATH
  tr \
    'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
    'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index c16d538..3cb7e63 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -19,10 +19,12 @@ cat >expect.text <<'EOF'
  +1
  EOF

-cat >hexdump <<'EOF'
-#!/bin/sh
+{
+	echo "#!$SHELL_PATH"
+	cat <<'EOF'
  perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1"
  EOF
+} >hexdump
  chmod +x hexdump

  test_expect_success 'setup binary file with history' '
-- 
1.6.6.1073.gd853b.dirty

^ permalink raw reply related

* Re: [PATCH 1/6] run-command: add "use shell" option
From: Johannes Sixt @ 2010-01-01 22:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20091230105316.GA22959@coredump.intra.peff.net>

Jeff King schrieb:
> Many callsites run "sh -c $CMD" to run $CMD. We can make it
> a little simpler for them by factoring out the munging of
> argv.
> 
> For simple cases with no arguments, this doesn't help much, but:
> 
>   1. For cases with arguments, we save the caller from
>      having to build the appropriate shell snippet.
> 
>   2. We can later optimize to avoid the shell when
>      there are no metacharacters in the program.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I made the matching tweak to the Windows half of run-command, but I
> don't actually have a box to test it on.
> 
> I modeled this after execv_git_cmd. Like that function, I try to release
> the allocated argv on error. However, we do actually leak the strbuf
> memory in one case. I'm not sure how much we care. On unix, this will
> always happen in a forked process which will either exec or die. On
> Windows, we seem to already be leaking the prepared argv for the git_cmd
> case (and now we leak the shell_cmd case, too).

That is OK. We can fix this when we find a work-load where this is
a problem.

But would you please squash this in to avoid a warning about an unused
static function on Windows.

---
  run-command.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/run-command.c b/run-command.c
index 22e2777..47ced57 100644
--- a/run-command.c
+++ b/run-command.c
@@ -48,6 +48,7 @@ static const char **prepare_shell_cmd(const char **argv)
  	return nargv;
  }

+#ifndef WIN32
  static int execv_shell_cmd(const char **argv)
  {
  	const char **nargv = prepare_shell_cmd(argv);
@@ -56,6 +57,7 @@ static int execv_shell_cmd(const char **argv)
  	free(nargv);
  	return -1;
  }
+#endif

  int start_command(struct child_process *cmd)
  {
-- 
1.6.6.1073.gd853b.dirty

^ permalink raw reply related

* Re: [PATCH v6 0/4] "git reset --merge" related improvements
From: Junio C Hamano @ 2010-01-01 20:42 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christian Couder, git, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <alpine.LFD.2.00.0912311623210.3630@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, 30 Dec 2009, Christian Couder wrote:
>>
>> Another reroll with the following changes:
>> - patches to add --keep option have been removed for now
>> - documentation patch has been moved before the core code changes
>> - commit messages have been improved
>> - tests have been much improved thanks to Junio's suggestions
>> 
>> Christian Couder (3):
>>   reset: improve mixed reset error message when in a bare repo
>>   Documentation: reset: add some tables to describe the different
>>     options
>>   reset: add a few tests for "git reset --merge"
>> 
>> Stephan Beyer (1):
>>   reset: use "unpack_trees()" directly instead of "git read-tree"
>
> FWIW, Ack on this version of the whole series - I don't think there is 
> anything controversial here, and I think avoiding the execve of read-tree 
> for something we have a library function for is a good thing (and the 
> change in behavior looks like a bug-fix to me).

The "bug-fix" needs my follow-up patch [*1*] to be a real "bug-fix", I
think.

By the way, I've always felt it somewhat confusing that "reset --merge"
implementation uses one-tree form of "read-tree -m -u", especially given
that your 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) that
describes the "bug" contrasts its behaviour with the behaviour of going to
a different branch with "checkout", that is inherently an operation that
depends on the two-tree "read-tree -m -u" semantics.

In the context of "reset", however, we don't want the check "checkout"
does between the tree that the index came from (the first tree argument,
typically HEAD) and the index before going to the target tree (the second
tree argument), so it is the right thing to do to use the one-tree form
semantics "go from the state in the index to the target tree".


[Refererence]

*1* http://article.gmane.org/gmane.comp.version-control.git/136004

^ permalink raw reply

* Re: Filename quoting / parsing problem
From: Junio C Hamano @ 2010-01-01 20:01 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: git
In-Reply-To: <7vws01li4c.fsf@alter.siamese.dyndns.org>

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

> I used "cat -e" to make it easier to see that "c file " not only has SP in
> it but it has trailing space.  Let's try the result.
> 
>         $ git diff --cached | cat -e
>         diff --git "a/a\001file" "b/a\001file"$
>         new file mode 100644$
>         index 0000000..e69de29$
>         diff --git a/b file b/b file$
>         new file mode 100644$
>         index 0000000..e69de29$
>         diff --git a/c file  b/c file $
>         new file mode 100644$
>         index 0000000..e69de29$
>         $ git diff --cached >P.diff
> 
> And as you described, "b file" and "c file " are not quoted and they do
> not have ---/+++ lines.
> 
> But observe this:
> ...
> We are now back in the state without any of these files, and P.diff records
> a patch to recreate these three files, one with quoting and the other two
> without.
> 
>         $ git apply --index P.diff
>         $ git ls-files -s | cat -e
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       "a\001file"$
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       b file$
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       c file $
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       hello$
> 
> This demonstrates that The claim below is false, doesn't it?
> 
> > Not parseable:
> >     diff --git a/baz  b/baz 
> >     new file mode 100644
> >     index 0000000..e69de29
> 
> Both "b file" and "c file " are parsed by "git apply" perfectly fine.

Having said all that, I don't think we would mind a change to treat a
pathname with trailing SP a bit specially (iow, quoting "c file " in the
above failed attempt to reproduce the issue).  A pathname with SP in it is
an eyesore but is a fact of life outside of sane world, but a quoted
pathname is an even worse eyesore.  A pathname with trailing SP would be
much less common and is more likely to be corrupted by MUA and cut & paste;
with quoting we can protect them a bit better.

^ permalink raw reply

* Re: Filename quoting / parsing problem
From: Junio C Hamano @ 2010-01-01 19:50 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: git
In-Reply-To: <201001011844.23571.agruen@suse.de>

Andreas Gruenbacher <agruen@suse.de> writes:

> Git quotes file names as documented in the git-diff manual page:
>
>   TAB, LF, double quote and backslash characters in pathnames are represented
>   as \t, \n, \" and \\, respectively.  If there is need for such substitution
>   then the whole pathname is put in double quotes.
>
> Spaces in file names currently do not trigger quoting.  (And \r triggers 
> quoting even though the man page doesn't say so).

Any control character is quoted; the documentation quoted above lists only
the common ones that have non-octal representation.

        $ git init one
        Initialized empty Git repository in /var/tmp/gomi/one/.git/
        $ cd one
        $ >hello && git add hello && git commit -m 'hello'
        [master (root-commit) 5338884] hello
         0 files changed, 0 insertions(+), 0 deletions(-)
         create mode 100644 hello
        $ >'a^Afile' ; >'b file' ; 'c file '
        $ git add 'a^Afile' 'b file' 'c file '

In the above sequence that attempts to reproduce the issue, "^A" actually
is a byte with value \001 (typically you would type ^V^A to get it from
the terminal on the shell).  "^G" seems to get "\a" even though it is not
in the list (that is why I said "only the common ones" above).

        $ git ls-files -s | cat -e
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	"a\001file"$
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	b file$
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	c file $
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	hello$

I used "cat -e" to make it easier to see that "c file " not only has SP in
it but it has trailing space.  Let's try the result.

        $ git diff --cached | cat -e
        diff --git "a/a\001file" "b/a\001file"$
        new file mode 100644$
        index 0000000..e69de29$
        diff --git a/b file b/b file$
        new file mode 100644$
        index 0000000..e69de29$
        diff --git a/c file  b/c file $
        new file mode 100644$
        index 0000000..e69de29$
        $ git diff --cached >P.diff

And as you described, "b file" and "c file " are not quoted and they do
not have ---/+++ lines.

But observe this:

	$ git reset --hard
        $ ls
        P.diff	hello
	$ git ls-files -s
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	hello

We are now back in the state without any of these files, and P.diff records
a patch to recreate these three files, one with quoting and the other two
without.

	$ git apply --index P.diff
        $ git ls-files -s | cat -e
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	"a\001file"$
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	b file$
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	c file $
        100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	hello$

This demonstrates that The claim below is false, doesn't it?

> Not parseable:
>     diff --git a/baz  b/baz 
>     new file mode 100644
>     index 0000000..e69de29

Both "b file" and "c file " are parsed by "git apply" perfectly fine.

^ permalink raw reply

* Filename quoting / parsing problem
From: Andreas Gruenbacher @ 2010-01-01 17:44 UTC (permalink / raw)
  To: git

Git quotes file names as documented in the git-diff manual page:

  TAB, LF, double quote and backslash characters in pathnames are represented
  as \t, \n, \" and \\, respectively.  If there is need for such substitution
  then the whole pathname is put in double quotes.

Spaces in file names currently do not trigger quoting.  (And \r triggers 
quoting even though the man page doesn't say so).  When there are no "---" and 
"+++" lines, this can lead to a parsing problem: only the "diff --git" line 
contains the file names, sometimes with insufficient quoting.  The following 
examples show the problem:

Parseable:
    diff --git "a/foo \r" "b/foo \r"
    new file mode 100644
    index 0000000..257cc56
    --- /dev/null
    +++ "b/foo \r"
    @@ -0,0 +1 @@
   +foo

Parseable:
    diff --git a/bar  b/bar 
    new file mode 100644
    index 0000000..5716ca5
    --- /dev/null
    +++ b/bar 
    @@ -0,0 +1 @@
    +bar

Not parseable:
    diff --git a/baz  b/baz 
    new file mode 100644
    index 0000000..e69de29


Could this please be changed so that filenames with spaces are also quoted, at 
least in the "diff --git" line, and possibly also in the "---" and "+++" 
lines?  Alternatively, how about a new extended header with the file name in 
this particular case?

Thanks,
Andreas

^ permalink raw reply

* Re: [PATCH 3/6] run-command: optimize out useless shell calls
From: Johannes Sixt @ 2010-01-01 10:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20100101045017.GA20769@coredump.intra.peff.net>

Jeff King schrieb:
> How should we proceed, then? The "DWIM with spaces" magic seems like
> something that can come later, so I am tempted to recommend taking my
> series now, fixing up msysgit as mentioned earlier (or just dropping the
> pager.c portion of my 2/6), and then implementing DWIM once Ilari's
> topic matures.

I think this procedure is fine. msysgit can resolve the conflict in 
pager.c as suggested earlier.

> We might want to hold my 5/6 and 6/6 back from master until the DWIM
> (which would make both totally safe, I think).

Would make sense, too.

-- Hannes

^ permalink raw reply

* Re: 65c042d4 broke `remote update' without named group
From: Björn Gustavsson @ 2010-01-01  9:14 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: YONETANI Tomokazu, git
In-Reply-To: <be6fef0d1001010100u7ebf25beydd2ff11ef71a9d66@mail.gmail.com>

2010/1/1 Tay Ray Chuan <rctay89@gmail.com>:

> Björn (added to Cc list), you seem to have been the main author of
> 'bg/fetch-multi', do you have any idea on this?

Yes. I have fixed the problem and my correction is included in
the latest 'pu' (bg/maint-remote-update-default).

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB

^ permalink raw reply

* Re: 65c042d4 broke `remote update' without named group
From: Tay Ray Chuan @ 2010-01-01  9:00 UTC (permalink / raw)
  To: YONETANI Tomokazu; +Cc: Björn Gustavsson, git
In-Reply-To: <20091229234959.GA94644@les.ath.cx>

Hi,

On Wed, Dec 30, 2009 at 7:49 AM, YONETANI Tomokazu <qhwt+git@les.ath.cx> wrote:
> Hello.
> It seems that with 65c042d4 (and v1.6.6), git-remote update without
> specifying a group on the command line ends up with the following error
> message if you define remotes.default in the config file:
>  $ git remote update
>  fatal: 'default' does not appear to be a git repository
>  fatal: The remote end hung up unexpectedly
>
> The document still says that when you don't specify the named group,
> remotes.default in the configuration will get used, so this should work
> (and it used to work with v1.6.4).
>
> After the commit 65c042d4, the commands are translated as follows:
>  $ git remote update
>  ==> git-fetch default         (with remotes.default defined)          NG
>  ==> git-fetch --all           (without remotes.default defined)       OK
>
>  $ git remote update default
>  ==> git-fetch --multiple default                                      OK
>
> So just adding "--multiple" in front of "default" in the first case
> above should fix it.

Björn (added to Cc list), you seem to have been the main author of
'bg/fetch-multi', do you have any idea on this?

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH v6 4/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Junio C Hamano @ 2010-01-01  7:03 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <7vtyv6o18q.fsf@alter.siamese.dyndns.org>

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

> At least disallowing means that the user _is notified_ and has to manually
> deal with the situation.  Pretending it succeeded by resetting only the
> index while still leaving the conflicted state in the work tree intact is
> a bit worse in that sense.
>
>> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
>> index c9044c9..b40999f 100644
>> --- a/Documentation/git-reset.txt
>> +++ b/Documentation/git-reset.txt
>> @@ -122,7 +122,7 @@ entries:
>>         X       U     A    B     --soft  (disallowed)
>>                                  --mixed  X       B     B
>>                                  --hard   B       B     B
>> -                                --merge (disallowed)
>> +                                --merge  X       B     B
>
> IOW, I think the result should be "B B B" instead of "X B B" in this
> case.

A squashable fix-up on top of your patch to match the wish in the part you
quoted from 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) would
look like this, I think.

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.

Please take this with a grain of salt as I am under slight influence of
CH3-CH2-OH while writing it, and I usually almost never drink.

---

 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 cf2433d..dc73dca 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -122,14 +122,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 1bbaf1c..616dd03 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1606,7 +1606,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 dd5999c..4a4d18c 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -666,7 +666,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.
@@ -682,11 +686,12 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 				return -1;
 			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);
@@ -702,7 +707,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);

^ permalink raw reply related

* Magit and TRAMP issues
From: Dave Abrahams @ 2010-01-01  6:49 UTC (permalink / raw)
  To: git

1. If shell-file-name points to a shell on the local machine but not
   on the remote server, a remote magit-status fails.  I had to
   create a symlink from /bin/bash to /usr/local/bin/bash on my
   FreeBSD server before it would work

2. Any attempt to stage changes fails for me.  I get a "Git Failed."
   message and see this in the *magit-process* buffer:

  $ git --no-pager add -u .
  fatal: Not a git repository (or any of the parent directories): .git

Any clues for me?

TIA,

--
Dave Abrahams           Meet me at BoostCon: http://www.boostcon.com
BoostPro Computing
http://www.boostpro.com

^ permalink raw reply

* Re: [PATCH v6 3/4] reset: add a few tests for "git reset --merge"
From: Junio C Hamano @ 2010-01-01  5:15 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20091230055448.4475.42383.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Commit 9e8eceab ("Add 'merge' mode to 'git reset'", 2008-12-01),
> added the --merge option to git reset, but there were no test cases
> for it.
>
> This was not a big problem because "git reset" was just forking and
> execing "git read-tree", but this will change in a following patch.
>
> So let's add a few test cases to make sure that there will be no
> regression.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Looks good.

> +# 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       D     D    D     --merge  C       D     D
> +test_expect_success 'reset --merge is ok with changes in file it does not touch' '
> +    git reset --merge HEAD^ &&
> +    ! grep 4 file1 &&
> +    grep 4 file2 &&
> +    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
> +    test -z "$(git diff --cached)"
> +'
> ...
> +# 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)' '
> +    git reset --hard second &&
> +    echo "line 4" >> file2 &&
> +    git add file2 &&
> +    git reset --merge HEAD^ &&
> +    ! grep 4 file2 &&
> +    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
> +    test -z "$(git diff)" &&
> +    test -z "$(git diff --cached)"
> +'

These two seem to duplicate the same case for file1; is it necessary?

I am not pointing it out as something that needs to be removed; I am just
puzzled and wondering if there is some interaction between the ways two
paths are handled and the test is trying to check that (which I do not
think is the case).

^ permalink raw reply

* Re: [PATCH v6 2/4] Documentation: reset: add some tables to describe the different options
From: Junio C Hamano @ 2010-01-01  5:15 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20091230055448.4475.83629.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> This patch adds a DISCUSSION section that contains some tables to
> show how the different "git reset" options work depending on the
> states of the files in the working tree, the index, HEAD and the
> target commit.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Much nicer.

>  Documentation/git-reset.txt |   66 +++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> index 2d27e40..c9044c9 100644
> --- a/Documentation/git-reset.txt
> +++ b/Documentation/git-reset.txt
> @@ -67,6 +67,72 @@ linkgit:git-add[1]).
>  <commit>::
>  	Commit to make the current HEAD. If not given defaults to HEAD.
>  
> +DISCUSSION
> +----------
> +
> +The tables below show what happens when running:
> +
> +----------
> +git reset --option target
> +----------
> +
> +to reset the HEAD to another commit (`target`) with the different
> +reset options depending on the state of the files.

Together with these "mechanical definitions", I think the readers would
benefit from reading why some are disallowed.

> +      working index HEAD target         working index HEAD
> +      ----------------------------------------------------
> +       A       B     C    D     --soft   A       B     D
> +                                --mixed  A       D     D
> +                                --hard   D       D     D
> +                                --merge (disallowed)

"reset --merge" is meant to be used when resetting out of a conflicted
merge.  Because any mergy operation guarantees that the work tree file
that is involved in the merge does not have local change wrt the index
before it starts, and that it writes the result out to the work tree, the
fact that we see difference between the index and the HEAD and also
between the index and the work tree means that we are not seeing a state
that a mergy operation left after failing with a conflict.  That is why we
disallow --merge option in this case, and the next one.

> +      working index HEAD target         working index HEAD
> +      ----------------------------------------------------
> +       A       B     C    C     --soft   A       B     C
> +                                --mixed  A       C     C
> +                                --hard   C       C     C
> +                                --merge (disallowed)

The same as above, but you are resetting to the same commit.

> +      working index HEAD target         working index HEAD
> +      ----------------------------------------------------
> +       B       B     C    D     --soft   B       B     D
> +                                --mixed  B       D     D
> +                                --hard   D       D     D
> +                                --merge  D       D     D

> +      working index HEAD target         working index HEAD
> +      ----------------------------------------------------
> +       B       B     C    C     --soft   B       B     C
> +                                --mixed  B       C     C
> +                                --hard   C       C     C
> +                                --merge  C       C     C

As this table is not only about "--merge" but to explain "reset", I think
other interesting cases should also be covered.

w=A	i=B	H=B	t=B

This is "we had local change in the work tree that was unrelated to the
merge", and "reset --merge" should be a no-op for this path.

w=A	i=B	H=B	t=C

This "reset --merge" is like "using checkout to switch to a commit that
has C but we have changes in the work tree", and should fail just like
"checkout branch" in such a situation fails without "-m" option.

^ permalink raw reply

* Re: [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend  into pieces
From: Junio C Hamano @ 2010-01-01  5:15 UTC (permalink / raw)
  To: Tarmigan; +Cc: Junio C Hamano, Shawn O . Pearce, git
In-Reply-To: <905315640912301009x491f957al839f66de7aba56ed@mail.gmail.com>

Tarmigan <tarmigan+git@gmail.com> writes:

> One reason it's labeled RFC is that I'm not very confident in my
> ability to write portable shell script.  It works for me with bash,
> but I'm not completely confident that is would work on ksh or dash.
> So it would be nice if you could specifically take a look at the new
> POST() and GET() and see if you notice anything obviously wrong there.

Looked Ok to me from a cursory reading, even though I wonder what the
first argument to run_backend function is good for...

^ permalink raw reply

* Re: [PATCH] builtin-config: add --path option doing ~ and ~user expansion.
From: Junio C Hamano @ 2010-01-01  5:15 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <1262191913-8340-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> 395de250 (Expand ~ and ~user in core.excludesfile, commit.template)
> introduced a C function git_config_pathname, doing ~/ and ~user/
> expansion. This patch makes the feature available to scripts with 'git
> config --get --path'.

Makes sense.  Thanks.

^ permalink raw reply

* Re: [PATCH v6 4/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Junio C Hamano @ 2010-01-01  5:14 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
	Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
	Johannes Sixt, Stephen Boyd
In-Reply-To: <20091230055448.4475.64716.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> But in 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) there is
> the following:
>
> "
>      - if the index has unmerged entries, "--merge" will currently
>        simply refuse to reset ("you need to resolve your current index
>        first"). You'll need to use "--hard" or similar in this case.
>
>        This is sad, because normally a unmerged index means that the
>        working tree file should have matched the source tree, so the
>        correct action is likely to make --merge reset such a path to
>        the target (like --hard), regardless of dirty state in-tree or
>        in-index. But that's not how read-tree has ever worked, so..
> "
>
> So the new behavior looks better according to the original
> implementation of "git reset --merge".

This is not really an improvement..  You are swapping an breakage with a
different breakage of a riskier kind.

At least disallowing means that the user _is notified_ and has to manually
deal with the situation.  Pretending it succeeded by resetting only the
index while still leaving the conflicted state in the work tree intact is
a bit worse in that sense.

> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> index c9044c9..b40999f 100644
> --- a/Documentation/git-reset.txt
> +++ b/Documentation/git-reset.txt
> @@ -122,7 +122,7 @@ entries:
>         X       U     A    B     --soft  (disallowed)
>                                  --mixed  X       B     B
>                                  --hard   B       B     B
> -                                --merge (disallowed)
> +                                --merge  X       B     B

IOW, I think the result should be "B B B" instead of "X B B" in this
case.

^ permalink raw reply

* Re: [PATCH 3/6] run-command: optimize out useless shell calls
From: Jeff King @ 2010-01-01  4:50 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <200912312316.47925.j6t@kdbg.org>

On Thu, Dec 31, 2009 at 11:16:47PM +0100, Johannes Sixt wrote:

> > > It does assume that we are able to detect execvp failure due to
> > > ENOENT which is currently proposed elsewhere by Ilari Liusvaara (and
> > > which is already possible on Windows).
> >
> > We could also simply do the path lookup ourselves, decide whether to use
> > the shell, and then exec.
> 
> I tried to convince Ilari that this is the way to go, but...

How should we proceed, then? The "DWIM with spaces" magic seems like
something that can come later, so I am tempted to recommend taking my
series now, fixing up msysgit as mentioned earlier (or just dropping the
pager.c portion of my 2/6), and then implementing DWIM once Ilari's
topic matures.

We might want to hold my 5/6 and 6/6 back from master until the DWIM
(which would make both totally safe, I think).

-Peff

^ permalink raw reply

* Re: [PATCH] Reword -M, when in `git log`s documention, to suggest --follow
From: Junio C Hamano @ 2010-01-01  4:35 UTC (permalink / raw)
  To: Alex Vandiver; +Cc: git
In-Reply-To: <1261428059-31286-1-git-send-email-alex@chmrr.net>

Alex Vandiver <alex@chmrr.net> writes:

> The documentation for `git log` is sadly misleading when it comes
> to tracking renames.  By far the most common option that users
> new to git want is the ability to view the history of a file
> across renames.  Unfortunately, `git log --help` shows:
>
>     NAME
>            git-log - Show commit logs
>     [...]
>     OPTIONS
>     [...]
>            -M
>                Detect renames.
>
> ...and most users stop reading there.  Unfortunately, what
> they're generally looking for comes significantly later:
>
>     [...]
>            --follow
>                Continue listing the history of a file beyond renames.
>
> Signed-off-by: Alex Vandiver <alex@chmrr.net>
> ---
>  Documentation/diff-options.txt |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 8707d0e..bcbad88 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -175,7 +175,13 @@ endif::git-format-patch[]
>  	Break complete rewrite changes into pairs of delete and create.
>  
>  -M::
> +ifdef::git-log[]
> +	Show renames in diff output.  See `--follow` to track history
> +	across renames.

With "s/history/history of a single file/", I think the new wording makes
more sense.

> +endif::git-log[]
> +ifndef::git-log[]
>  	Detect renames.
> +endif::git-log[]

Also we probably should do s/Detect renames/Show renames in diff output/
to match the earlier change.

>  
>  -C::
>  	Detect copies as well as renames.  See also `--find-copies-harder`.
> -- 
> 1.6.6.rc0.363.g69d13.dirty

^ permalink raw reply

* [ANNOUNCE] git-cola 1.4.1.2
From: David Aguilar @ 2010-01-01  1:48 UTC (permalink / raw)
  To: git

git-cola is a powerful GUI for Git written in Python/PyQt4

git-cola v1.4.1.2 has been tagged and released:

* git clone git://github.com/davvid/git-cola.git
* http://github.com/davvid/git-cola
* http://cola.tuxfamily.org/

I haven't sent an announcement in a long time but we've
been quite busy working on enhancements nonetheless.
The release notes below summarize all changes since 1.3.8
in case you haven't kept up with the latest cola.

NYE is fast approaching here on the west coast so I've got a
party to catch.  Thus, this is the last git-cola for 2009 ;-)
Happy New Years everybody!


git-cola v1.4.1.2
=================

Usability, bells and whistles
-----------------------------
* It is now possible to checkout from the index as well
  as from `HEAD`.  This corresponds to the
  `Removed Unstaged Changes` action in the `Repository Status` tool.
* The `remote` dialogs (fetch, push, pull) are now slightly
  larger by default.
* Bookmarks can be selected when `git-cola` is run outside of a Git repository.
* Added more user documentation.  We now include many links to
  external git resources.

Fixes
-----
* Fixed a missing ``import`` when showing `right-click` actions
  for unmerged files in the `Repository Status` tool.
* ``git update-index --refresh`` is no longer run everytime
  ``git cola version`` is run.
* Don't try to watch non-existant directories when using `inotify`.

Packaging
---------
* The ``Makefile`` will now conditionally include a ``config.mak``
  file located at the root of the project.  This allows for user
  customizations such as changes to the `prefix` variable
  to be stored in a file so that custom settings do not need to
  be specified every time on the command-line.
* The build scripts no longer require a ``.git`` directory to
  generate the ``builtin_version.py`` module.  The release tarballs
  now include a ``version`` file at the root of the project which
  is used in lieu of having the Git repository available.
  This allows for ``make clean && make`` to function outside of
  a Git repository.
* Added maintainer's ``make dist`` target to the ``Makefile``.
* The built-in `simplejson` and `jsonpickle` libraries can be
  excluded from ``make install`` by specifying the ``standalone=true``
  `make` variable.  For example, ``make standalone=true install``.
  This corresponds to the ``--standalone`` option to ``setup.py``.


git-cola v1.4.1.1
=================

Usability, bells and whistles
-----------------------------
* We now use patience diff by default when it is available via
  `git diff --patience`.

* Allow closing the `cola classic` tool with `Ctrl+W`.

* Update desktop menu entry to read `Cola Git GUI`.

Fixes
-----
* Fixed an unbound variable error in the `push` dialog.

Packaging
---------
* Don't include `simplejson` in MANIFEST.in.
* Update desktop entry to read `Cola Git GUI`.


git-cola v1.4.1
===============

This feature release adds two new features directly from
`git-cola`'s github issues backlog.  On the developer
front, further work was done towards modularizing the code base.

Usability, bells and whistles
-----------------------------
* Dragging and dropping patches invokes `git-am`

  http://github.com/davvid/git-cola/issues/closed#issue/3

* A dialog to allow opening or cloning a repository
  is presented when `git-cola` is launched outside of a git repository.

  http://github.com/davvid/git-cola/issues/closed/#issue/22

* Warn when `push` is used to create a new branch

  http://github.com/davvid/git-cola/issues/closed#issue/35

* Optimized startup time by removing several calls to `git`.


Portability
-----------
* `git-cola` is once again compatible with PyQt 4.3.x.

Developer
---------
* `cola.gitcmds` was added to factor out git command-line utilities
* `cola.gitcfg` was added for interacting with `git-config`
* `cola.models.browser` was added to factor out repobrowser data
* Added more tests


git-cola v1.4.0.5
=================

Fixes
-----
* Fix launching external applications on Windows
* Ensure that the `amend` checkbox is unchecked when switching modes
* Update the status tree when amending commits


git-cola v1.4.0.4
=================

Packaging
---------
* Fix Lintian warnings


git-cola v1.4.0.3
=================

Fixes
-----
* Fix X11 warnings on application startup


git-cola v1.4.0.2
=================

Fixes
-----
* Added missing 'Exit Diff Mode' button for 'Diff Expression' mode

  http://github.com/davvid/git-cola/issues/closed/#issue/31

* Fix a bug when initializing fonts on Windows

  http://github.com/davvid/git-cola/issues/closed/#issue/32


git-cola v1.4.0.1
=================

Fixes
-----
* Keep entries in sorted order in the `cola classic` tool
* Fix staging untracked files

  http://github.com/davvid/git-cola/issues/closed/#issue/27

* Fix the `show` command in the Stash dialog

  http://github.com/davvid/git-cola/issues/closed/#issue/29

* Fix a typo when loading merge commit messages

  http://github.com/davvid/git-cola/issues/closed/#issue/30


git-cola v1.4.0
===============

This release focuses on a redesign of the git-cola user interface,
a tags interface, and better integration of the `cola classic` tool.
A flexible interface based on configurable docks is used to manage the
various cola widgets.

Usability, bells and whistles
-----------------------------
* New GUI is flexible and user-configurable
* Individual widgets can be detached and rearranged arbitrarily
* Add an interface for creating tags
* Provide a fallback `SSH_ASKPASS` implementation to prompt for
  SSH passwords on fetch/push/pull
* The commit message editor displays the current row/column and
  warns when lines get too long
* The `cola classic` tool displays upstream changes
* `git cola --classic` launches `cola classic` in standalone mode
* Provide more information in log messages

Fixes
-----
* Inherit the window manager's font settings
* Miscellaneous PyQt4 bug fixes and workarounds

Developer
---------
* Removed all usage of Qt Designer `.ui` files
* Simpler model/view architecture
* Selection is now shared across tools
* Centralized notifications are used to keep views in sync
* The `cola.git` command class was made thread-safe
* Less coupling between model and view actions
* The status view was rewritten to use the MVC architecture
* Added more documentation and tests


git-cola v1.3.9
===============

Usability, bells and whistles
-----------------------------
* Added a `cola classic` tool for browsing the entire repository
* Handle diff expressions with spaces
* Handle renamed files

Portability
-----------
* Handle carat `^` characters in diff expressions on Windows
* Worked around a PyQt 4.5/4.6 QThreadPool bug

Documentation
-------------
* Added a keyboard shortcuts reference page
* Added developer API documentation

Fixes
-----
* Fix the diff expression used when reviewing branches
* Fix a bug when pushing branches
* Fix X11 warnings at startup
* Fix more interrupted system calls on Mac OS X



-- 
		David

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox