* [PATCH v2 3/5] Documentation: reset: describe new "--keep" option
From: Christian Couder @ 2010-01-19 4:25 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, Andreas Schwab, Daniel Convissor
In-Reply-To: <20100119042404.4510.48855.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 | 47 ++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 46 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 168db08..90239f5 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 modified in the working tree.
+
-p::
--patch::
Interactively select hunks in the difference between the index
@@ -93,6 +98,7 @@ in the index and in state D in HEAD.
--mixed A D D
--hard D D D
--merge (disallowed)
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -100,6 +106,7 @@ in the index and in state D in HEAD.
--mixed A C C
--hard C C C
--merge (disallowed)
+ --keep A C C
working index HEAD target working index HEAD
----------------------------------------------------
@@ -107,6 +114,7 @@ in the index and in state D in HEAD.
--mixed B D D
--hard D D D
--merge D D D
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -114,6 +122,7 @@ in the index and in state D in HEAD.
--mixed B C C
--hard C C C
--merge C C C
+ --keep B C C
working index HEAD target working index HEAD
----------------------------------------------------
@@ -121,6 +130,7 @@ in the index and in state D in HEAD.
--mixed B D D
--hard D D D
--merge (disallowed)
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -128,6 +138,7 @@ in the index and in state D in HEAD.
--mixed B C C
--hard C C C
--merge B C C
+ --keep B C C
"reset --merge" is meant to be used when resetting out of a conflicted
merge. Any mergy operation guarantees that the work tree file that is
@@ -138,6 +149,14 @@ between the index and the work tree, then it means that we are not
resetting out from a state that a mergy operation left after failing
with a conflict. That is why we disallow --merge option in this case.
+"reset --keep" is meant to be used when remove some of the last
+commits in the current branch while keep changes in the working tree.
+If there could be conflicts between the changes in the commit we want
+to remove and the changes in the working tree we want to keep, the
+reset is disallowed. That's why it is disallowed if there are both
+changes between the working tree and HEAD, and between HEAD and the
+target.
+
The following tables show what happens when there are unmerged
entries:
@@ -147,6 +166,7 @@ entries:
--mixed X B B
--hard B B B
--merge B B B
+ --keep (disallowed)
working index HEAD target working index HEAD
----------------------------------------------------
@@ -154,6 +174,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.
@@ -325,6 +346,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.271.gc8799
^ permalink raw reply related
* [PATCH v2 2/5] reset: add test cases for "--keep" option
From: Christian Couder @ 2010-01-19 4:25 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, Andreas Schwab, Daniel Convissor
In-Reply-To: <20100119042404.4510.48855.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).
In the case of unmerged entries, we can see that "git reset --keep"
works only when the target state is the same as HEAD. And then the
work tree is not reset.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t7110-reset-merge.sh | 119 +++++++++++++++++++++++++++++++++++++++++++++++-
t/t7111-reset-table.sh | 8 +++
2 files changed, 126 insertions(+), 1 deletions(-)
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 8704d00..1a9c1c7 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 3 different branches' '
git reset --hard second &&
git branch branch1 &&
@@ -156,6 +232,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 &&
@@ -166,6 +254,21 @@ 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_expect_success '--merge with added/deleted' '
git reset --hard third &&
rm -f file2 &&
@@ -180,4 +283,18 @@ test_expect_success '--merge with added/deleted' '
git diff --exit-code --cached
'
+test_expect_success '--keep with added/deleted' '
+ git reset --hard third &&
+ rm -f file2 &&
+ test_must_fail git merge branch3 &&
+ ! test -f file2 &&
+ test -f file3 &&
+ git diff --exit-code file3 &&
+ git diff --exit-code branch3 file3 &&
+ git reset --keep HEAD &&
+ test -f file3 &&
+ ! test -f file2 &&
+ git diff --exit-code --cached
+'
+
test_done
diff --git a/t/t7111-reset-table.sh b/t/t7111-reset-table.sh
index de896c9..2ebda97 100755
--- a/t/t7111-reset-table.sh
+++ b/t/t7111-reset-table.sh
@@ -44,26 +44,32 @@ A B C D soft A B D
A B C D mixed A D D
A B C D hard D D D
A B C D merge XXXXX
+A B C D keep XXXXX
A B C C soft A B C
A B C C mixed A C C
A B C C hard C C C
A B C C merge XXXXX
+A B C C keep A C C
B B C D soft B B D
B B C D mixed B D D
B B C D hard D D D
B B C D merge D D D
+B B C D keep XXXXX
B B C C soft B B C
B B C C mixed B C C
B B C C hard C C C
B B C C merge C C C
+B B C C keep B C C
B C C D soft B C D
B C C D mixed B D D
B C C D hard D D D
B C C D merge XXXXX
+B C C D keep XXXXX
B C C C soft B C C
B C C C mixed B C C
B C C C hard C C C
B C C C merge B C C
+B C C C keep B C C
EOF
test_expect_success 'setting up branches to test with unmerged entries' '
@@ -104,10 +110,12 @@ X U B C soft XXXXX
X U B C mixed X C C
X U B C hard C C C
X U B C merge C C C
+X U B C keep XXXXX
X U B B soft XXXXX
X U B B mixed X B B
X U B B hard B B B
X U B B merge B B B
+X U B B keep X B B
EOF
test_done
--
1.6.6.271.gc8799
^ permalink raw reply related
* [PATCH v2 5/5] reset: disallow using --keep when there are unmerged entries
From: Christian Couder @ 2010-01-19 4:26 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, Andreas Schwab, Daniel Convissor
In-Reply-To: <20100119042404.4510.48855.chriscool@tuxfamily.org>
The use case for --keep option is to remove previous commits unrelated
to the current changes in the working tree. So in this use case we are
not supposed to have unmerged entries. This is why it seems safer to
just disallow using --keep when there are unmerged entries.
And this patch changes the error message when --keep was disallowed and
there were some unmerged entries from:
error: Entry 'file1' would be overwritten by merge. Cannot merge.
fatal: Could not reset index file to revision 'HEAD^'.
to:
fatal: Cannot do a keep reset in the middle of a merge.
which is nicer.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-reset.txt | 5 +++--
builtin-reset.c | 17 +++++++++++++----
t/t7110-reset-merge.sh | 23 +++++++++--------------
t/t7111-reset-table.sh | 2 +-
4 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 90239f5..d2f6880 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -155,7 +155,8 @@ If there could be conflicts between the changes in the commit we want
to remove and the changes in the working tree we want to keep, the
reset is disallowed. That's why it is disallowed if there are both
changes between the working tree and HEAD, and between HEAD and the
-target.
+target. To be safe, it is also disallowed when there are unmerged
+entries.
The following tables show what happens when there are unmerged
entries:
@@ -174,7 +175,7 @@ entries:
--mixed X A A
--hard A A A
--merge A A A
- --keep X A A
+ --keep (disallowed)
X means any state and U means an unmerged index.
diff --git a/builtin-reset.c b/builtin-reset.c
index 52584af..441ac1b 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -224,6 +224,14 @@ static void prepend_reflog_action(const char *action, char *buf, size_t size)
warning("Reflog action message too long: %.*s...", 50, buf);
}
+static void die_if_unmerged_cache(int reset_type)
+{
+ if (is_merge() || read_cache() < 0 || unmerged_cache())
+ die("Cannot do a %s reset in the middle of a merge.",
+ reset_type_names[reset_type]);
+
+}
+
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -329,10 +337,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
- 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 {
+ if (reset_type == SOFT)
+ die_if_unmerged_cache(reset_type);
+ else {
+ if (reset_type == KEEP)
+ die_if_unmerged_cache(reset_type);
int err = reset_index_file(sha1, reset_type, quiet);
if (reset_type == KEEP)
err = err || reset_index_file(sha1, MIXED, quiet);
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 1a9c1c7..70cdd8e 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -237,7 +237,7 @@ test_expect_success '"reset --keep HEAD^" fails with pending merge' '
git reset --hard third &&
test_must_fail git merge branch1 &&
test_must_fail git reset --keep HEAD^ 2>err.log &&
- grep file1 err.log | grep "overwritten by merge"
+ grep "middle of a merge" err.log
'
# The next test will test the following:
@@ -258,18 +258,15 @@ test_expect_success '"reset --merge HEAD" is ok with pending merge' '
#
# working index HEAD target working index HEAD
# ----------------------------------------------------
-# file1: X U B B --keep X B B
-test_expect_success '"reset --keep HEAD" is ok with pending merge' '
+# file1: X U B B --keep (disallowed)
+test_expect_success '"reset --keep HEAD" fails with pending merge' '
git reset --hard third &&
test_must_fail git merge branch1 &&
- cat file1 >orig_file1 &&
- git reset --keep HEAD &&
- test "$(git rev-parse HEAD)" = "$(git rev-parse third)" &&
- test -z "$(git diff --cached)" &&
- test_cmp file1 orig_file1
+ test_must_fail git reset --keep HEAD 2>err.log &&
+ grep "middle of a merge" err.log
'
-test_expect_success '--merge with added/deleted' '
+test_expect_success '--merge is ok with added/deleted merge' '
git reset --hard third &&
rm -f file2 &&
test_must_fail git merge branch3 &&
@@ -283,7 +280,7 @@ test_expect_success '--merge with added/deleted' '
git diff --exit-code --cached
'
-test_expect_success '--keep with added/deleted' '
+test_expect_success '--keep fails with added/deleted merge' '
git reset --hard third &&
rm -f file2 &&
test_must_fail git merge branch3 &&
@@ -291,10 +288,8 @@ test_expect_success '--keep with added/deleted' '
test -f file3 &&
git diff --exit-code file3 &&
git diff --exit-code branch3 file3 &&
- git reset --keep HEAD &&
- test -f file3 &&
- ! test -f file2 &&
- git diff --exit-code --cached
+ test_must_fail git reset --keep HEAD 2>err.log &&
+ grep "middle of a merge" err.log
'
test_done
diff --git a/t/t7111-reset-table.sh b/t/t7111-reset-table.sh
index 2ebda97..ce421ad 100755
--- a/t/t7111-reset-table.sh
+++ b/t/t7111-reset-table.sh
@@ -115,7 +115,7 @@ X U B B soft XXXXX
X U B B mixed X B B
X U B B hard B B B
X U B B merge B B B
-X U B B keep X B B
+X U B B keep XXXXX
EOF
test_done
--
1.6.6.271.gc8799
^ permalink raw reply related
* [PATCH v2 0/5] add "--keep" option to "git reset"
From: Christian Couder @ 2010-01-19 4:25 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, Andreas Schwab, Daniel Convissor
Changes since the previous series are the following:
- removed parts of the commit message that contrasted "--keep" with "--merge"
- improved wording in the documentation
- added a new patch to disallow "--keep" when there are unmerged entries
- added some tests for --keep in t7111-reset-table.sh
Christian Couder (5):
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
reset: disallow using --keep when there are unmerged entries
Documentation/git-reset.txt | 48 +++++++++++++++++-
builtin-reset.c | 46 ++++++++++++++---
t/t7103-reset-bare.sh | 25 ++++++---
t/t7110-reset-merge.sh | 116 ++++++++++++++++++++++++++++++++++++++++++-
t/t7111-reset-table.sh | 8 +++
5 files changed, 223 insertions(+), 20 deletions(-)
^ permalink raw reply
* [PATCH v2 1/5] reset: add option "--keep" to "git reset"
From: Christian Couder @ 2010-01-19 4:25 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, Andreas Schwab, Daniel Convissor
In-Reply-To: <20100119042404.4510.48855.chriscool@tuxfamily.org>
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 --keep target" to reset the HEAD to another
commit (as a special case "target" could be the same as
HEAD).
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
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 --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 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 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".
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.271.gc8799
^ permalink raw reply related
* [PATCH v2 4/5] reset: disallow "reset --keep" outside a work tree
From: Christian Couder @ 2010-01-19 4:26 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, Andreas Schwab, Daniel Convissor
In-Reply-To: <20100119042404.4510.48855.chriscool@tuxfamily.org>
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.271.gc8799
^ permalink raw reply related
* Re: [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"
From: Christian Couder @ 2010-01-19 4:28 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: <7viqbk8evw.fsf@alter.siamese.dyndns.org>
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.
^ permalink raw reply
* Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Christian Couder @ 2010-01-19 4:28 UTC (permalink / raw)
To: Daniel Convissor
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
Johannes Sixt, Stephen Boyd, Andreas Schwab
In-Reply-To: <20100102171422.GA19522@panix.com>
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.
^ permalink raw reply
* Re: idea: git "came from" tags
From: D Herring @ 2010-01-19 5:02 UTC (permalink / raw)
To: git
In-Reply-To: <4B542EB2.5030407@drmicha.warpmail.net>
Michael J Gruber wrote:
> D Herring venit, vidit, dixit 18.01.2010 05:22:
>> Actors:
>> - public "upstream" repository
>> - public "local" repository
>> - end users tracking both
>>
>> Situation:
>> - local starts by tracking upstream
>> - local makes changes, commits, and sends upstream
>> - users now tracking local ahead of upstream
>
> Here I have to ask why. If users choose to track a volatile branch then
> they have to live with rebasing or hard resets. If they want something
> stable then they should track upstream.
I'm maintaining the "local" repository in a distribution of upstream
libraries. I'm trying to avoid both volatile branches and unnecessary
clutter. Here, both upstream and local are stable; they are just
maintained by different teams. Upstream often accepts patches; but
they may tweak things, use a different version control system, etc.
and so the commit objects differ.
>> - upstream makes modified commits
>> - local satisfied, wants to reset master to upstream/master
>>
>> Problem:
>> - A merge will perpetually leave two parallel branches. Even though
>> there are no longer any diffs, local/master cannot use the same
>> objects as upstream/master.
>
> If there are no diffs then, in fact, it can share most objects since
> most trees will be the same, only a few commit objects will differ.
But once I have a local diff, the local tree must always use different
git objects, even though the file contents are the same...
>> - A hard reset lets local/master return to sharing objects with
>> upstream/master; but this may break pulls or cause other problems for
>> users.
>>
>> Proposed solution:
>> - Local adds a "came from" tag to upstream/master, leaves a tag on the
>> head of local/master, and does a hard reset from local/master to
>> upstream/master. When a user tracking local/master does a pull, their
>> client detects a non-fast-forward, finds the came-from tag, and treats
>> it as a fast-forward.
>>
>> Basically, this is a protocol to glue a "strategy ours" merge onto an
>> existing tree. This way local can cleanly track upstream, with no
>> added complexity in the nominal (no local changes) case.
>
> But doesn't that mean that users completely trust you about what they
> should consider a fast forward, i.e. when they should do a hard reset?
> So, this is completely equivalent to following one of your branches with
> +f, i.e. having a public a branch which they pull from no matter what,
> and having a private branch which pushes to the public one in case of
> fast-forwards as well as in the case when you would use your special tag.
This almost works, but it destroys some history preserved by a proper
merge or this proposed extension. For example, suppose there are
three commits between the user's last fetch and this merge/forced
update; a proper merge will download them, but a forced update will
not. This becomes important when a release tarball is based on one of
these missing commits.
If local uses merge objects to track this properly, it creates a
parallel branch that is simply nuisance clutter. Normally, the
nuisance is limited to a visual distraction in gitk; but it can be
significant if a user is trying to track both local and upstream.
When there are and have been no local changes, local is following
upstream; so the user can freely follow either until a local change is
made. When there are no but have been local changes that were merged,
the user must pick a branch even thought the contents are the same.
I could be obsessing over a minor detail; but the proposed change
doesn't seem drastic.
To reiterate,
Given
A - C - E - ... - Z
\ \
- B + D
where A-Z are commit objects and the contents of merge D are identical
to C, it would be nice to have a protocol that tags D for posterity
and allows D->E to be a fast forward, without requiring cooperation
from the source of E to Z.
Later,
Daniel
^ permalink raw reply
* [PATCH] post-receive-email: allow customizing of subject/intro/footer
From: Mike Frysinger @ 2010-01-19 5:12 UTC (permalink / raw)
To: git
The format of the subject/intro/footer are noise imo, but rather than
debate the issue, let the user customize the output using the existing
git config hooks section. The default output is retained for each part.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
contrib/hooks/post-receive-email | 76 +++++++++++++++++++++++++++++---------
1 files changed, 58 insertions(+), 18 deletions(-)
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 58a35c8..79ab6b1 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -38,6 +38,15 @@
# hooks.emailprefix
# All emails have their subjects prefixed with this prefix, or "[SCM]"
# if emailprefix is unset, to aid filtering
+# hooks.emailsubject
+# Allow customizing of the subject. Default is a description of what
+# ref changed and how/why.
+# hooks.emailbodyintro
+# Allow customizing of the body intro. Default is friendly paragraph that
+# explains why the user is receiving this e-mail and what has changed.
+# hooks.emailfooter
+# Allow customizing of the footer. Default is name of the script and the
+# repo description.
# hooks.showrev
# The shell command used to format each revision in the email, with
# "%s" replaced with the commit id. Defaults to "git rev-list -1
@@ -55,6 +64,10 @@
# "X-Git-Newrev", and "X-Git-Reftype" to enable fine tuned filtering and
# give information for debugging.
#
+# All variables that start with 'email' have substitution performed on them.
+# Patterns like @foo@ are replaced with the contents of the variable foo.
+# See subst_vars() for the specific keywords available for substitution.
+#
# ---------------------------- Functions
@@ -190,36 +203,47 @@ generate_email()
generate_email_footer
}
+subst_vars()
+{
+ sep=$(printf '\001')
+ # let this be used in a pipeline or by itself
+ ( [ "$#" -ne 0 ] && echo "$@" || cat ) | sed \
+ -e "s${sep}@change_type@${sep}${change_type}${sep}g" \
+ -e "s${sep}@describe@${sep}${describe}${sep}g" \
+ -e "s${sep}@newrev@${sep}${newrev}${sep}g" \
+ -e "s${sep}@oldrev@${sep}${oldrev}${sep}g" \
+ -e "s${sep}@projectdesc@${sep}${projectdesc}${sep}g" \
+ -e "s${sep}@refname@${sep}${refname}${sep}g" \
+ -e "s${sep}@refname_type@${sep}${refname_type}${sep}g" \
+ -e "s${sep}@oldrev@${sep}${oldrev}${sep}g" \
+ -e "s${sep}@short_refname@${sep}${short_refname}${sep}g"
+}
+
generate_email_header()
{
# --- Email (all stdout will be the email)
# Generate header
+ (
cat <<-EOF
To: $recipients
- Subject: ${emailprefix}$projectdesc $refname_type, $short_refname, ${change_type}d. $describe
- X-Git-Refname: $refname
- X-Git-Reftype: $refname_type
- X-Git-Oldrev: $oldrev
- X-Git-Newrev: $newrev
+ Subject: ${emailprefix}${emailsubject}
+ X-Git-Refname: @refname@
+ X-Git-Reftype: @refname_type@
+ X-Git-Oldrev: @oldrev@
+ X-Git-Newrev: @newrev@
+ EOF
- This is an automated email from the git hooks/post-receive script. It was
- generated because a ref change was pushed to the repository containing
- the project "$projectdesc".
+ if [ -n "${emailbodyintro}" ] ; then
+ printf '\n%s\n' "${emailbodyintro}"
+ fi
- The $refname_type, $short_refname has been ${change_type}d
- EOF
+ printf '\n%s\n' "The @refname_type@, @short_refname@ has been @change_type@d"
+ ) | subst_vars
}
generate_email_footer()
{
- SPACE=" "
- cat <<-EOF
-
-
- hooks/post-receive
- --${SPACE}
- $projectdesc
- EOF
+ subst_vars "${emailfooter}"
}
# --------------- Branches
@@ -671,6 +695,22 @@ recipients=$(git config hooks.mailinglist)
announcerecipients=$(git config hooks.announcelist)
envelopesender=$(git config hooks.envelopesender)
emailprefix=$(git config hooks.emailprefix || echo '[SCM] ')
+emailsubject=$(git config hooks.emailsubject || \
+ echo '@projectdesc@ @refname_type@, @short_refname@, @change_type@d. @describe@')
+emailbodyintro=$(git config hooks.emailbodyintro || cat <<-EOF
+ This is an automated email from the git hooks/post-receive script. It was
+ generated because a ref change was pushed to the repository containing
+ the project "@projectdesc@".
+ EOF
+)
+emailfooter=$(git config hooks.emailfooter || SPACE=" " cat <<-EOF
+
+
+ hooks/post-receive
+ --${SPACE}
+ @projectdesc@
+ EOF
+)
custom_showrev=$(git config hooks.showrev)
# --- Main loop
--
1.6.6
^ permalink raw reply related
* Re: [PATCH v2 1/5] reset: add option "--keep" to "git reset"
From: Junio C Hamano @ 2010-01-19 5:50 UTC (permalink / raw)
To: Christian Couder
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd, Andreas Schwab, Daniel Convissor
In-Reply-To: <20100119042602.4510.24100.chriscool@tuxfamily.org>
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.
You did this:
git checkout master
work; git commit ; work; git commit ; work; git commit
edit ; git add ; ... (but not commit)
and noticed the three commits should not be on master but on a new branch.
I think we currently teach users to do something like this:
git stash
git branch topic
git reset --hard HEAD~3
git stash pop
Instead you want to do this:
git branch topic
git reset --keep HEAD~3
Surely you halved the number of command involved, but is this really an
improvement? At least I can visualize (and more importantly, explain to
new users) how the "stash, flip and unstash" works, why it is safe, and
how to recover when "pop" stops in conflicts, but I have no confidence in
explaining what "reset --keep" does and how to recover when it refuses to
work to new users.
Another way to accomplish the same thing might be:
git branch -m topic
git checkout -b master HEAD~3
and with the same number of commands, conceptually it may be easier to
understand than "reset --keep". What you committed so far belongs to
another branch "topic", so you name the current history that way, and then
you switch branches with "checkout" that keeps your local modifications.
It also opens the possibility of retrying with "-m" after checkout refuses
to acti, to take the same mix-up risk CVS/SVN users have, if you are very
confident that your local change conflicts only minimally with the change
made on the topic and you can resolve them.
Of course, when you are not interested in keeping the topmost commits at
all, you either
git stash ; git reset --hard HEAD~3 ; git stash pop
or
git reset --keep HEAD~3
but even in this case, I think "stash, flip and unstash" is easier to
explain, especially when teaching what to do if things go wrong.
I dunno. Is this really an useful addition that helps real-world
workflows and is worth a five patch series, or is this just "because we
can" patch?
^ permalink raw reply
* why is tagger header optional?
From: Shawn O. Pearce @ 2010-01-19 6:09 UTC (permalink / raw)
To: git
So why is it legal to omit the tagger header from a tag?
E.g. the Linux kernel tag v2.6.12 has no tagger header:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=tag;h=26791a8bcf0e6d33f43aef7682bdb555236d56de
JGit is currently failing on this tag, because its fsck
implementation demands that a tag have a tagger header
that can be parsed as a person identity.
Looking at tag.c's parse_tag_buffer(), the variable sig_line seems
to be expected to point at the "tagger " header (given its name),
but its not actually validated as such.
Is there a version of Git floating around that doesn't create a
tagger header when creating a signed tag? WTF?
--
Shawn.
^ permalink raw reply
* Re: "warning: Updating the currently checked out branch may cause confusion" on bare repositories
From: Shawn O. Pearce @ 2010-01-19 6:29 UTC (permalink / raw)
To: Adam Megacz; +Cc: git
In-Reply-To: <xuu2ska3doix.fsf@nowhere.com>
Adam Megacz <adam@megacz.com> wrote:
>
> It seems that the message "warning: Updating the currently checked out
> branch may cause confusion" is unnecessary when pushing to a bare
> "myproject.git" repository (instead of a "myproject/.git" repository).
>
> Is this the case?
>
> If so, perhaps the warning should be omitted when the target is a bare
> repository.
That should already be the case. Did you create the bare repository
by taking a copy of a non-bare's .git directory? Check your bare
repository's config file and see if core.bare = false, if so set
it to true to signal it really is bare.
--
Shawn.
^ permalink raw reply
* Re: why is tagger header optional?
From: Junio C Hamano @ 2010-01-19 6:31 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20100119060946.GA23212@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> So why is it legal to omit the tagger header from a tag?
>
> E.g. the Linux kernel tag v2.6.12 has no tagger header:
We didn't.add tagger line until c818566 ([PATCH] Update tags to record who
made them, 2005-07-14), which is v0.99.1~9
Linux 2.6.12 is a lot older than that. v2.6.13-rc4 in late July is the
first one with tagger.
^ permalink raw reply
* Re: why is tagger header optional?
From: Shawn O. Pearce @ 2010-01-19 6:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4vebo6z.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > So why is it legal to omit the tagger header from a tag?
> >
> > E.g. the Linux kernel tag v2.6.12 has no tagger header:
>
> We didn't.add tagger line until c818566 ([PATCH] Update tags to record who
> made them, 2005-07-14), which is v0.99.1~9
>
> Linux 2.6.12 is a lot older than that. v2.6.13-rc4 in late July is the
> first one with tagger.
Ugh. So its like the 100640 or whatever mode tags in the kernel
trees that are also considered bogus by today's standards, but have
to be allowed because of the kernel history.
Thanks.
--
Shawn.
^ permalink raw reply
* Re: why is tagger header optional?
From: Junio C Hamano @ 2010-01-19 6:35 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20100119063255.GC23212@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>>
>> > So why is it legal to omit the tagger header from a tag?
>> >
>> > E.g. the Linux kernel tag v2.6.12 has no tagger header:
>>
>> We didn't.add tagger line until c818566 ([PATCH] Update tags to record who
>> made them, 2005-07-14), which is v0.99.1~9
>>
>> Linux 2.6.12 is a lot older than that. v2.6.13-rc4 in late July is the
>> first one with tagger.
>
> Ugh. So its like the 100640 or whatever mode tags in the kernel
> trees that are also considered bogus by today's standards, but have
> to be allowed because of the kernel history.
Yeah; don't we have "fsck --strict" or something to take the distinction
into account, though? I don't recall if lack of tagger triggers the check
offhand and I am too lazy to check.
^ permalink raw reply
* Re: why is tagger header optional?
From: Shawn O. Pearce @ 2010-01-19 6:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfx62bo0i.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > Junio C Hamano <gitster@pobox.com> wrote:
> >> "Shawn O. Pearce" <spearce@spearce.org> writes:
> >>
> >> > So why is it legal to omit the tagger header from a tag?
> >> >
> >> > E.g. the Linux kernel tag v2.6.12 has no tagger header:
> >>
> >> We didn't.add tagger line until c818566 ([PATCH] Update tags to record who
> >> made them, 2005-07-14), which is v0.99.1~9
> >>
> >> Linux 2.6.12 is a lot older than that. v2.6.13-rc4 in late July is the
> >> first one with tagger.
> >
> > Ugh. So its like the 100640 or whatever mode tags in the kernel
> > trees that are also considered bogus by today's standards, but have
> > to be allowed because of the kernel history.
>
> Yeah; don't we have "fsck --strict" or something to take the distinction
> into account, though? I don't recall if lack of tagger triggers the check
> offhand and I am too lazy to check.
I don't think it does under --strict. But yea, we do have --strict
for the non-kernel repositories.
This came up because Gerrit Code Review asks JGit to do fsck during
receive of objects from a client, JGit's tag fsck is too strict
and demands a "tagger " header, but someone was trying to push this
old tag from the Linux kernel into an empty repository.
I'll have to relax our tag fsck code in JGit and make the header
optional.
--
Shawn.
^ permalink raw reply
* Re: why is tagger header optional?
From: Jeff King @ 2010-01-19 6:44 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20100119060946.GA23212@spearce.org>
On Mon, Jan 18, 2010 at 10:09:46PM -0800, Shawn O. Pearce wrote:
> So why is it legal to omit the tagger header from a tag?
>
> E.g. the Linux kernel tag v2.6.12 has no tagger header:
I think you just answered your own question. We must support tagger-less
tags because they exist in important projects like the kernel. :)
> [...]
> Is there a version of Git floating around that doesn't create a
> tagger header when creating a signed tag? WTF?
Everything prior to c818566 ([PATCH] Update tags to record who made
them, 2005-07-14). So probably nothing that anybody is using now, but
v2.6.12 was one of the first tags made.
> Looking at tag.c's parse_tag_buffer(), the variable sig_line seems
> to be expected to point at the "tagger " header (given its name),
> but its not actually validated as such.
Actually, that variable name predates the patch above, so I suspect
"sig" meant "GPG signature". At any rate, as you can see, git doesn't
verify it, and the code for "git show v2.6.12" in
builtin-log.c:show_object handles the taggerless case as well. I don't
think anything else actually looks at the tagger. verify-tag treats the
signed data as opaque, and just shows the identity of the actual signer.
-Peff
^ permalink raw reply
* Re: [PATCH v3] rev-parse --namespace
From: Jeff King @ 2010-01-19 6:56 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git, Junio C Hamano
In-Reply-To: <1263808296-30756-1-git-send-email-ilari.liusvaara@elisanet.fi>
On Mon, Jan 18, 2010 at 11:51:36AM +0200, Ilari Liusvaara wrote:
> Add --namespace=<namespace> option to rev-parse and everything that
> accepts its options. This option matches all refs in some subnamespace
> of refs hierarchy.
Thanks, I think the code in this version looks good, but:
> --- /dev/null
> +++ b/t/t6018-rev-list-namespace.sh
> [...]
> +compare () {
> + # Split arguments on whitespace.
> + git $1 $2 | sort >expected &&
> + git $1 $3 | sort >actual &&
> + cmp expected actual
> +}
Please use test_cmp instead of regular cmp.
Also, do you really need to sort? The internal ref code keeps the ref
lists sorted by name, and we merge-sort the packed and loose lists in
do_for_each_ref. So your --namespace should always operate on the refs
in sorted order, producing predictable results.
> +test_expect_success 'setup' '
> +
> + commit master &&
> + git checkout -b subspace/one master
> + commit one &&
> + git checkout -b subspace/two master
> + commit two &&
> + git checkout -b subspace-x master
> + commit subspace-x &&
> + git checkout -b other/three master
> + commit three &&
> + git checkout -b someref master
> + commit some &&
> + git checkout master &&
> + commit master2
> +'
You are still missing some '&&' here to detect setup failures.
-Peff
^ permalink raw reply
* Re: [PATCH v3] Threaded grep
From: Johannes Sixt @ 2010-01-19 7:03 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: Junio C Hamano, Linus Torvalds, Git Mailing List
In-Reply-To: <4c8ef71001181612l72ec73ecmae709fbb475752b0@mail.gmail.com>
Fredrik Kuivinen schrieb:
> On Mon, Jan 18, 2010 at 23:10, Junio C Hamano <gitster@pobox.com> wrote:
>> Fredrik Kuivinen <frekui@gmail.com> writes:
>>> +/* This lock protects all the variables above. */
>>> +static pthread_mutex_t grep_lock = PTHREAD_MUTEX_INITIALIZER;
>> J6t had comments on these initializers; do they also apply to
>> builtin-pack-objects.c?
>
> I believe so, but I do not know. Johannes?
44626dc (MSVC: Windows-native implementation for subset of Pthreads API)
that is currently queued in pu makes the necessary adjustments to
builtin-pack-objects.c.
-- Hannes
^ permalink raw reply
* Re: Unmodified submodules shows up as dirty with 1.6.6.443.gd7346
From: Johannes Sixt @ 2010-01-19 7:13 UTC (permalink / raw)
To: Jacob Helwig; +Cc: Gustaf Hendeby, git, Jens.Lehmann
In-Reply-To: <8c9a061001180802t5ec0d389j2cae9f1771130c36@mail.gmail.com>
Jacob Helwig schrieb:
> If there is no output from git status in the submodule, then git
> status in the superproject shows the submodule as being clean.
> However, if there is _any_ output from git status (untracked files,
> modified files, deleted files, new files), then the superproject shows
> the submodule as being dirty.
But isn't it a bug that a submodule is considered dirty just because an
untracked file appears?
-- Hannes
^ permalink raw reply
* Re: git describe --contains
From: Ramkumar Ramachandra @ 2010-01-19 7:11 UTC (permalink / raw)
To: git
In-Reply-To: <43d8ce651001181537w667f71b7te7fac56b4f562c30@mail.gmail.com>
> Doing: git describe --contains SHA
The problem resides in builtin-describe.c:377. It uses cmd_name_rev from builtin-
name-rev.c. I'm attempting to rewrite this, and have a describe_contains() -
Hopefully, I'll get a patch out by the end of the day.
^ permalink raw reply
* Re: [PATCH 1/2] Add branch --set-upstream
From: Matthieu Moy @ 2010-01-19 7:22 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git, Junio C Hamano
In-Reply-To: <1263847452-29634-2-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> Add --set-upstream option to branch that works like --track, except that
> when branch exists already, its upstream info is changed without changing
> the ref value.
>
> Based-on-patch-from: Matthieu Moy <Matthieu.Moy@imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
I'm short on Git time budget, so I happily let you take over on this
patch.
I'm a bit worried about the vocabulary: my understanding is that
"--track" is considered confusing, and should have been called
"--set-upstream" since the beginning.
With your patch, we have both --set-upstream and --track, with
different meanings. Should we consider --track as semi-deprecated, and
--set-upstream as "--track done right"? If so, you can consider
squashing this to promote your option instead of --track, and perhaps
reword git-branch.txt so that --track says it does like
--set-upstream, and not the other way around.
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index b169836..4c66f19 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -2114,12 +2114,12 @@ linkgit:git-fetch[1] to keep them up-to-date; see
Now create the branches in which you are going to work; these start out
at the current tip of origin/master branch, and should be set up (using
-the --track option to linkgit:git-branch[1]) to merge changes in from
+the --set-upstream option to linkgit:git-branch[1]) to merge changes in from
Linus by default.
-------------------------------------------------
-$ git branch --track test origin/master
-$ git branch --track release origin/master
+$ git branch --set-upstream test origin/master
+$ git branch --set-upstream release origin/master
-------------------------------------------------
These can be easily kept up to date using linkgit:git-pull[1].
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply related
* Branch merge bug
From: Ramkumar Ramachandra @ 2010-01-19 7:22 UTC (permalink / raw)
To: git
A friend showed me this reduced test case. It seems to work fine with
bzr and hg. Is this a git bug?
>: git --version
git version 1.6.3.1
>: git init
Initialized empty Git repository in /Users/lut4rp/Code/tester/.git/
>: echo asdjhaskd >fil
>: git add fil
>: git cm 'initial commit'
[master (root-commit) 9983161] initial commit
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 fil
>: git branch new
>: git branch
* master
new
>: git mv fil fila
>: git status
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# renamed: fil -> fila
#
>: git cm 'renamed to fila'
[master 9de0953] renamed to fila
1 files changed, 0 insertions(+), 0 deletions(-)
rename fil => fila (100%)
>: git co new
Switched to branch 'new'
>: git rm fil
rm 'fil'
>: echo asjhdajkhsdkajhs >fila
>: git add fila
>: git cm 'renamed, heavily modified'
[new 5914271] renamed, heavily modified
2 files changed, 2 insertions(+), 1 deletions(-)
delete mode 100644 fil
create mode 100644 fila
>: git co master
Switched to branch 'master'
>: git merge new
CONFLICT (rename/delete): Rename fil->fila in HEAD and deleted in new
Automatic merge failed; fix conflicts and then commit the result.
>: cat fila
asdjhaskd #### Still the master
branch's content, `merge` apparently did nothing.
>: git show :2:fila
asdjhaskd
>: git show :3:fila
fatal: ambiguous argument ':3:fila': unknown revision or path not in
the working tree.
Use '--' to separate paths from revisions
>: git show new:fila
asjhdajkhsdkajhs
ajksh askjdhaksdh kajshdk asd
^ permalink raw reply
* Re: [PATCH v3] Threaded grep
From: Fredrik Kuivinen @ 2010-01-19 7:34 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.LFD.2.00.1001180947140.13231@localhost.localdomain>
On Mon, Jan 18, 2010 at 10:05:19AM -0800, Linus Torvalds wrote:
> On my machine (4 cores with HT, so 8 threads total), I get the
> following:
>
> [ Note: the --no-ext-grep is because I'm comparing with a git that has
> the original grep optimization, but hasn't removed the external grep
> functionality yet. I just used the same command line when then comparing
> to next+your patch, so don't mind it.
>
> Also, the three examples are chosen to be: "trivial single match",
> "regex single match", and "trivial lots of matches" ]
You may be testing slightly different things: next has 885d211 (grep:
rip out pessimization to use fixmatch()) and bbc09c2 (grep: rip out
support for external grep) was added before. So next+patch uses
regexec and the version with support for external greps uses
strstr. IIRC strstr was a bit faster than regexec on your
box. However, this only explains a small part of the results you are
seeing.
>
> Before (all best-of-five), with the non-threaded internal grep:
>
> - git grep --no-ext-grep qwerty
>
> real 0m0.311s
> user 0m0.164s
> sys 0m0.144s
>
> - git grep --no-ext-grep qwerty.*as
>
> real 0m0.555s
> user 0m0.416s
> sys 0m0.132s
>
> - git grep --no-ext-grep function
>
> real 0m0.461s
> user 0m0.276s
> sys 0m0.180s
>
> After, with threading:
>
> - git grep --no-ext-grep qwerty
>
> real 0m0.152s
> user 0m0.788s
> sys 0m0.212s
>
> - git grep --no-ext-grep qwerty.*as
>
> real 0m0.148s
> user 0m0.724s
> sys 0m0.284s
>
> - git grep --no-ext-grep function
>
> real 0m0.241s
> user 0m1.436s
> sys 0m0.216s
>
> so now it's a clear win in all cases. And as you'd expect, the "complex
> case with single output" is the one that wins most, since it's the one
> that should have the most CPU usage, with the least synchronization.
>
> That said, it still does waste quite a bit of time doing this, and while
> it almost doubles performance in the last case too, it does so at the cost
> of quite a bit of overhead.
>
> (Some overhead is natural, especially since I have HT. Running two threads
> on the same core does _not_ give double the performance, so the CPU time
> almost doubling - because the threads themselves run slower - is not
> unexpected. It's when the CPU time more than quadruples that I suspect
> that it could be improved a bit).
I haven't seen this overhead during my tests. But I'm _guessing_ that
the problem is that the mutex grep_lock is held while the result is
written to stdout. So when we are writing, no significant amount of
work can be done by any thread. Here is a patch to fix this (applies
on to of v3).
diff --git a/builtin-grep.c b/builtin-grep.c
index 8fca7a6..422b0ec 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -139,21 +139,48 @@ static int grep_file_async(struct grep_opt *opt, char *name,
return 0;
}
+/* Mark the work_item as done. The function guarantees that w->done is
+ * set to 1 and that if w is included in a prefix of the range
+ * [todo_done, todo_start) where each work_item has .done == 1, then
+ * w->out will eventually be written to stdout. The writing is done
+ * either by this thread or by the thread which has set 'writing' to
+ * 1.
+ */
static void work_done(struct work_item *w)
{
- int old_done;
+ /* 1 if there is a thread which is writing data to stdout and
+ 0 otherwise. */
+ static int writing = 0;
+ int old_done, write_idx, write_to;
pthread_mutex_lock(&grep_lock);
w->done = 1;
old_done = todo_done;
- for(; todo[todo_done].done && todo_done != todo_start;
- todo_done = (todo_done+1) % ARRAY_SIZE(todo)) {
- w = &todo[todo_done];
- write_or_die(1, w->out.buf, w->out.len);
- if (w->type == WORK_BUF)
- free(w->buf);
-
- free(w->name);
+
+ /* We need a loop here if todo_start is changed while we write
+ * some data. */
+ while (!writing && todo[todo_done].done && todo_done != todo_start) {
+ write_idx = todo_done;
+ write_to = todo_start;
+ writing = 1;
+
+ /* Unlock the mutex while we write the data. This is
+ * safe as writing == 1 and hence there is at most one
+ * thread which writes data at any time. */
+ pthread_mutex_unlock(&grep_lock);
+ for(; todo[write_idx].done && write_idx != write_to;
+ write_idx = (write_idx+1) % ARRAY_SIZE(todo)) {
+ w = &todo[write_idx];
+ write_or_die(1, w->out.buf, w->out.len);
+ if (w->type == WORK_BUF)
+ free(w->buf);
+
+ free(w->name);
+ }
+
+ pthread_mutex_lock(&grep_lock);
+ writing = 0;
+ todo_done = write_idx;
}
if (old_done != todo_done)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox