* Re: [PATCH v5 4/7] reset: add option "--keep" to "git reset"
From: Junio C Hamano @ 2009-12-12 21:46 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: <20091212043259.3930.21831.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. 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 "--keep" option will do what you want.
Having two paragraphs that describe the use case near the top is a big
improvement from older rounds, but this one line is more irritating than
useful, as it tries to be convincing only by being repetitive. The only
new information it adds relative to the first two lines is its name. The
true convincing argument immediately follows in the form of the table
below anyway, so I'd drop this line if I were you.
> The table below shows what happens when running "git reset --option
> target" to reset the HEAD to another commit (as a special case "target"
> could be the same as HEAD) in the cases where "--merge" and "--keep"
> behave differently.
>
> working index HEAD target working index HEAD
> ----------------------------------------------------
> A B C D --keep (disallowed)
> --merge (disallowed)
Ok. the user has partially updated the index and has further changes; we
don't want to lose either of them. During a failed merge, this cannot
happen, so it is a good safety measure on "--merge" side as well.
> A B C C --keep A C C
> --merge (disallowed)
The user started working based on C, and has partially updated the index.
This cannot have come from a failed automerge because A != B, so using
"reset --merge" is likely to be a mistake and we should disallow it as a
safety measure.
> B B C D --keep (disallowed)
> --merge C C C
For --keep this is the same as the first case (except that the "partially
updated the index" happened to be "100% pertially") and it makes sense to
disallow it.
However, I think --merge _should_ have D (target) in all of the three in
the result in this case, as I mentioned in my response to [PATCH 3/7]. Is
that "the bug" you talked about there?
> B B C C --keep B C C
> --merge C C C
A special case of the second one for --keep. For --merge, B can only have
come from the previous merge operation we are resetting away, and matching
all three result to the "C", which is the target, is the right thing to do.
> So as can be seen in the table, "--merge" can discard changes in the
> working tree, while "--keep" does not. So if one wants to avoid using
> "git stash" before and after using "git reset" to save current changes,
> it is better to use "--keep" rather than "--merge".
This is a very flawed and misleading description. These two options serve
two entirely different purposes and use cases, but your "if one wants to
avoid using ... to save current changes" is too broad and does not
distinguish between the two. "--keep" option is valid (or "better") for
only one of them, while it is absolutely the wrong option to use for the
other one. So it may be "better" only half the time and it is wrong in
the other half.
The precondition of using a --merge is the scenario Linus described in
9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01). You started some
mergy operation and ended up in a state where all the differences, either
merged or unmerged, between the HEAD and the index came from the mergy
operation. Also the paths in the work tree that correspond to the index
entries with differences from HEAD are modified by the mergy operation
(either updated with a cleanly resolved merge, or with conflict markers)
and do not match HEAD when "reset --merge" is run, but it is guaranteed
that you didn't have any local modification to them before the mergy
operation. You do want to reset these changes to the work tree away,
while keeping the changes you had before the mergy operation.
"--merge" not only _can_ (as in "has the risk to") discard changes, but in
the "failed merge" scenario, it is absolutely the right thing to do to
discard the changes in the work tree for the paths that have differences
between HEAD and index and to update them to that of the target. Not
updating like "--keep" does is not "better" but is simply _wrong_.
The use case "--keep" wants to support is very different (and you have a
good description near the beginning of the commit log message for people
to judge if the intended use case makes sense). For it, discarding the
changes to the work tree is a wrong thing to do.
> The following table shows what happens on unmerged entries:
>
> working index HEAD target working index HEAD
> ----------------------------------------------------
> X U A B --keep X B B
> --merge X B B
> X U A A --keep X A A
> --merge (disallowed)
>
> In this table X can be any state and U means an unmerged entry.
I am wondering if we should disallow "--keep" if we see unmerged entries
in the index as a minimum safety measure. Failing "reset --keep" when an
unmerged entry exists in the index will save people who are trying to
discard a failed merge (i.e. who should have used --merge) but was somehow
fooled into thinking that "--keep" is a better "--merge".
It also is tempting to say that we should forbid "reset --merge" without
an unmerged entry in the index, but that wouldn't work. A mergy operation
would have left unmerged entries in the index initially before giving the
control back to the user, but the user may have used "edit && git add" to
resolve them, and then decided that it is not worth committing. By the
time "reset --merge" is run, there may not be any unmerged path left in
the index.
I am suggesting extra safety measures primarily because I am worried that
people will get confused by these two similar looking options that are
meant for entirely different use cases. An obvious alternative solution
to avoid the confusion is not to add "--keep" in the first place. While I
think that is rather a cop-out than a solution, it might make more sense.
It is hopeless to educate users to pick the right one, if even the author
of the new option mistakenly thinks that "--keep is merely a better
version of --merge".
My preference is at this point to first have patches [1/7] to [3/7] to
update "reset --merge" (I am not sure about the "--mixed in $GIT_DIR"
change, though), with a new follow-up patch [3.5/7] to fix "reset --merge"
to reset paths that were cloberred by the merge to the target (not HEAD),
and start cooking these changes in 'pu' and then 'next'.
That will lay groundwork of using unpack_trees() in "reset" and after they
stabilize, build new modes like "--keep" on top of it.
^ permalink raw reply
* Re: [PATCH v5 3/7] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Junio C Hamano @ 2009-12-12 21:46 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: <20091212043259.3930.82570.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> As Daniel Barkalow found, there is a difference between this new
> version and the old one. The old version gives an error for
> "git reset --merge" with unmerged entries and the new version does
> not. But this can be seen as a bug fix,...
I sense that there is one crucial sentence missing here for a reader to
judge the "can be seen as a bug fix" claim. Instead of giving an error
and stopping, what _does_ this version do? If it ran "rm -rf" on the
entire work tree instead of giving an error, it wouldn't be a bugfix, for
example ;-)
> In fact there is still an error with unmerge entries if we reset
> the unmerge entries to the same state as HEAD. So the bug is not
> completely fixed.
"If we reset to HEAD then it is a bug"---and what the patch actually does
is...?
I agree with the analysis in the log message of 9e8ecea (Add 'merge' mode
to 'git reset', 2008-12-01):
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.
If I have a local change that I know is unrelated to a merge I am
going to do, I can imagine I'd work like this:
$ git pull some-where
... Some paths conflict, others merge cleanly and update
... the index, but the overall change looks horrible, and
... I decide to discard the whole thing.
$ git reset --merge HEAD
In this case HEAD is the target. Similarly, if I have a local change that
I know is unrelated to a series of patches I am applying:
$ git am -3 topic.mbox
... A few patches apply cleanly, and then the series fail
... with conflict. The overall change looks horrible, and
... I decide to discard the whole thing.
$ git reset --merge @{3.minutes.ago}
In this case, the target is the commit that I was at before starting to
apply the series---i.e. different from HEAD.
In either case, because "merge" (triggered by "pull") and "am -3" honor
the promise with the user that:
(1) no mergy (aka "integration") command stuffs an unmerged entry to an
index that is dirty with respect to HEAD (this should also apply to
"cherry-pick" and "rebase -m/-i" even though the last one is often
stricter than it is absolutely necessary); and
(2) every mergy command verifies that the index entry and the path in the
work tree do not have any local change before they are updated by it;
resetting can safely overwrite both the index entry and the path in the
work tree with contents taken from the commit we are switching to.
The updated test seems to be testing that "reset --merge HEAD^" does make
the index match the target, but it only checks "is there _any_ change",
and does not even test "which path kept the change and which path got
cloberred".
Ideally it should test "Is the change what we expect to have? Did we keep
what we should have kept, instead of clobbering? Did we discard the
changes to the path that the failed merge cloberred?", all of the three.
> diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
> index 8190da1..6afaf73 100755
> --- a/t/t7110-reset-merge.sh
> +++ b/t/t7110-reset-merge.sh
> @@ -79,10 +79,12 @@ test_expect_success 'setup 2 different branches' '
> git commit -a -m "change in branch2"
> '
>
> -test_expect_success '"reset --merge HEAD^" fails with pending merge' '
> +test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
> test_must_fail git merge branch1 &&
> - test_must_fail git reset --merge HEAD^ &&
> - git reset --hard HEAD
> + git reset --merge HEAD^ &&
> + test -z "$(git diff --cached)" &&
> + test -n "$(git diff)" &&
> + git reset --hard HEAD@{1}
> '
>
> test_expect_success '"reset --merge HEAD" fails with pending merge' '
> --
> 1.6.6.rc1.8.gd33ec
^ permalink raw reply
* Re: [PATCH/RFC] ignore unknown color configuration
From: Junio C Hamano @ 2009-12-12 21:45 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091212122524.GA17547@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> When parsing the config file, if there is a value that is
> syntactically correct but unused, we generally ignore it.
> This lets non-core porcelains store arbitrary information in
> the config file, and it means that configuration files can
> be shared between new and old versions of git (the old
> versions might simply ignore certain configuration).
>
> The one exception to this is color configuration; if we
> encounter a color.{diff,branch,status}.$slot variable, we
> die if it is not one of the recognized slots (presumably as
> a safety valve for user misconfiguration).
This reminds me of the issue an earlier patch with a good intention but a
horrible consequence wanted to address.
http://thread.gmane.org/gmane.comp.version-control.git/125925/focus=127629
> This patch loosens the check to match the rest of
> git-config; unknown color slots are simply ignored.
I am of two minds, even though I am slightly in favor than against the
change.
This is a sane thing to do, as "slot" is part of the name of the variable,
and we generally do not warn upon seeing a misspelled variable name (it
makes it worse that "func" is not even misspelled but merely unknown to
older version of git in your scenario).
On the other hand, I suspect that most people would apprecfiate if their
git pointed out "diff.color.finc? What do you mean?" before they waste
30 minutes wondering why the new feature in 1.6.6 does not work for them.
^ permalink raw reply
* Re: [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir
From: Junio C Hamano @ 2009-12-12 21:45 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: <20091212043259.3930.98779.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> It is strange and fragile that a mixed reset is disallowed in a bare
> repo but is allowed in a .git dir. So this patch simplifies things
> by only allowing soft resets when not in a working tree.
I would not mind what you said were "I find the difference strange", as it
is a fairly subjective word. But if you say "fragile", you would need to
defend the use of the word better. What kind of misuse does it invite,
and what grave consequences such misuses would cause? I do not see any
fragility and I would want to understand it better so that I can write
about it in the Release Note to warn people and encourage them to upgrade.
More importantly, I think the difference between the two actually makes
sense. A bare repository by definition does _not_ have any work tree so
there is no point in having the index file in there. On the other hand,
even though it is not necessary to "cd .git && git reset HEAD^", I don't
see a strong reason why it needs to be disallowed.
On the other hand, I don't see a strong reason why such a use needs to be
supported, other than "we've allowed it for a long time, and people may
have hooks (they typically start in $GIT_DIR) that rely on it", which by
itself is not something strong enough to veto the change. It is only a
minor incompatibility and I can be persuaded to listen to a pros-and-cons
argument.
An honest justification might have been "This change to disallow a mixed
reset in $GIT_DIR of a repository with a work tree will break existing
scripts, but I think it is not widely used _for such and such reasons_,
and can easily be worked around. On the other hand, this change vastly
simplifies the reimplementation of 'reset' _because X and Y and Z_".
> This patch is also needed to speed up "git reset" by using
> unpack_tree() directly (instead of execing "git read-tree").
It is very unclear _why_ it is "needed" from this description.
^ permalink raw reply
* Re: [PATCH 0/3] Update advice in commit/status output
From: Junio C Hamano @ 2009-12-12 18:27 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, git
In-Reply-To: <fabb9a1e0912120813h1c41b7bfg4525f8f01e04ddb1@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> Heya,
>
> On Sat, Dec 12, 2009 at 10:02, Junio C Hamano <gitster@pobox.com> wrote:
>> In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
>> while merging.
>
>>From the patches I'm guessing you mean "In either case" instead?
Correct.
^ permalink raw reply
* Re: [PATCH 1/3] octopus: make merge process simpler to follow
From: Junio C Hamano @ 2009-12-12 20:46 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Stephen Boyd, git, Junio C Hamano, Jari Aalto
In-Reply-To: <200912122056.59297.j6t@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> On Samstag, 12. Dezember 2009, Stephen Boyd wrote:
>> + pretty_name="$(eval echo \$GITHEAD_$SHA1)"
>
> eval pretty_name=\$GITHEAD_$SHA1
>
> :)
True, and also there can be scripts that call merge-octupos directly
without setting "GITHEAD_$commit", so it would probably need something
like
: ${pretty_name:=$SHA1}
after that.
^ permalink raw reply
* Re: [PATCH 0/3] Update advice in commit/status output
From: Sverre Rabbelier @ 2009-12-12 16:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1260608523-15579-1-git-send-email-gitster@pobox.com>
Heya,
On Sat, Dec 12, 2009 at 10:02, Junio C Hamano <gitster@pobox.com> wrote:
> In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
> while merging.
>From the patches I'm guessing you mean "In either case" instead?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH v5 2/7] reset: add a few tests for "git reset --merge"
From: Junio C Hamano @ 2009-12-12 23:30 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: <20091212043259.3930.19134.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> +test_description='Tests for "git reset --merge"'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'creating initial files' '
> + echo "line 1" >> file1 &&
> + echo "line 2" >> file1 &&
> + echo "line 3" >> file1 &&
> + cp file1 file2 &&
> + git add file1 file2 &&
> + test_tick &&
> + git commit -m "Initial commit"
> +'
5-char indent is somewhat unusual; please fix it. Also you may want to
tag the initial one and other key commits for later use.
> +test_expect_success 'reset --merge is ok with changes in file it does not touch' '
> + echo "line 4" >> file1 &&
> + echo "line 4" >> file2 &&
> + test_tick &&
> + git commit -m "add line 4" file1 &&
IOW, move the above four lines to the end of the first "setup", and tag as
necessary like this:
test_expect_success setup '
for i in 1 2 3; do echo line $i; done >file1 &&
cat file1 >file2 &&
git add file1 file2 &&
test_tick &&
git commit -m "Initial commit" &&
git tag initial &&
echo line 4 >>file1 &&
cat file1 >file2 &&
test_tick &&
git commit -m "add line 4 to file1" file1 &&
git tag second
'
And the rest should become its own test, starting from a known state.
> + git reset --merge HEAD^ &&
> + ! grep 4 file1 &&
> + grep 4 file2 &&
Switching from the second commit to the initial one should succeed because
the only local change is in file2 that is unrelated. This should also check
if HEAD points at the "initial" commit after "reset", and the index
matches the HEAD commit.
> + git reset --merge HEAD@{1} &&
> + grep 4 file1 &&
> + grep 4 file2
> +'
And switching back to the second commit should recover. This should be a
separate test, and should make sure it begins in a known state even if the
"reset --merge HEAD^" in the previous test failed. This should also
check:
test $(git rev-parse HEAD) = $(git rev-parse second) &&
test -z "$(git diff --cached)"
If any of these two "reset --merge" is broken, the next test will start at
an unknown commit, hence you should add
git reset --hard second &&
at the beginning of the next test. You may also need to copy file1 to
file2 to recreate the local change as well.
The idea is not to assume that tests will succeed and 'recovery' step at
the end will run; instead, make sure each test starts itself in a known
state. The way you wrote "reset --hard" at the end of some tests for
recovery is fragile. There is no guarantee that each test run to the end,
executing these recovery parts. Instead, have a corresponding set-up at
the beginning of each test as necessary.
> +test_expect_success 'reset --merge discards changes added to index (1)' '
> + echo "line 5" >> file1 &&
> + git add file1 &&
So at this point, file2 has an extra "line 4", and file1 has a change to
be committed. And we reset to the initial commit.
> + git reset --merge HEAD^ &&
> + ! grep 4 file1 &&
> + ! grep 5 file1 &&
> + grep 4 file2 &&
And that will make file1 match that of initial, while keeping the change
to file2 intact. The same comment about missing checks applies. The
remainder of this test should be a separate test.
> + echo "line 5" >> file2 &&
> + git add file2 &&
> + git reset --merge HEAD@{1} &&
> + ! grep 4 file2 &&
> + ! grep 5 file1 &&
> + grep 4 file1
> +'
Starting at 'initial' but with two lines added to file2 and the index
updated to it, we reset to 'second' (spell these out with tags, instead of
relying on reflog, so that you do not assume all the previous tests have
passed). Both files should match those of 'second'. The same comment
about missing checks applies.
> +test_expect_success 'reset --merge discards changes added to index (2)' '
> + echo "line 4" >> file2 &&
> + git add file2 &&
> + git reset --merge HEAD^ &&
> + ! grep 4 file2 &&
> + git reset --merge HEAD@{1} &&
> + ! grep 4 file2 &&
> + grep 4 file1
> +'
The same comment about the missing 'make sure it begins in a known state'
applies, but I don't see the point of this (2). Is there any fundamental
difference of the set-up this one tests, compared to the earlier test?
> +test_expect_success 'reset --merge fails with changes in file it touches' '
> + 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 --merge HEAD^ 2>err.log &&
> + grep file1 err.log | grep "not uptodate" &&
Hmm, this is something we had a patch for to give different messages from
plumbing and Porcelain.
> + git reset --hard HEAD^
Lose this 'recover at the end'. There is no guarantee the each test run
to the end. Instead, have a corresponding set-up at the beginning of the
next one.
I'd re-review the rest after this is rerolled.
^ permalink raw reply
* [PATCH/RFC] ignore unknown color configuration
From: Jeff King @ 2009-12-12 12:25 UTC (permalink / raw)
To: git
When parsing the config file, if there is a value that is
syntactically correct but unused, we generally ignore it.
This lets non-core porcelains store arbitrary information in
the config file, and it means that configuration files can
be shared between new and old versions of git (the old
versions might simply ignore certain configuration).
The one exception to this is color configuration; if we
encounter a color.{diff,branch,status}.$slot variable, we
die if it is not one of the recognized slots (presumably as
a safety valve for user misconfiguration). This behavior
has existed since 801235c (diff --color: use
$GIT_DIR/config, 2006-06-24), but hasn't yet caused a
problem. No porcelain has wanted to store extra colors, and
we once a color area (like color.diff) has been introduced,
we've never changed the set of color slots.
However, that changed recently with the addition of
color.diff.func. Now a user with color.diff.func in their
config can no longer freely switch between v1.6.6 and older
versions; the old versions will complain about the existence
of the variable.
This patch loosens the check to match the rest of
git-config; unknown color slots are simply ignored. This
doesn't fix this particular problem, as the older version
(without this patch) is the problem, but it at least
prevents it from happening again in the future.
Signed-off-by: Jeff King <peff@peff.net>
---
I don't know if it is worth trying to fix the problem for
color.diff.func. It would require renaming the variable to something
outside of the color.diff hierarchy, which is quite ugly. I don't know
how common a problem this is. Personally I switch between git versions a
lot doing git development (uncommon), but I also ran into it because I
share my ~/.gitconfig across several machines (which may be much more
common).
I suppose we could provide an undocumented alias of "color.difffunc",
and then only people who ran into this problem would have to be exposed
to the monstrosity. Normal people would use color.diff.func and be none
the wiser. But that feels awfully hack-ish to me.
builtin-branch.c | 4 +++-
builtin-commit.c | 4 +++-
diff.c | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 05e876e..c87e63b 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -65,7 +65,7 @@ static int parse_branch_color_slot(const char *var, int ofs)
return BRANCH_COLOR_LOCAL;
if (!strcasecmp(var+ofs, "current"))
return BRANCH_COLOR_CURRENT;
- die("bad config variable '%s'", var);
+ return -1;
}
static int git_branch_config(const char *var, const char *value, void *cb)
@@ -76,6 +76,8 @@ static int git_branch_config(const char *var, const char *value, void *cb)
}
if (!prefixcmp(var, "color.branch.")) {
int slot = parse_branch_color_slot(var, 13);
+ if (slot < 0)
+ return 0;
if (!value)
return config_error_nonbool(var);
color_parse(value, var, branch_colors[slot]);
diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..326cd63 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -890,7 +890,7 @@ static int parse_status_slot(const char *var, int offset)
return WT_STATUS_NOBRANCH;
if (!strcasecmp(var+offset, "unmerged"))
return WT_STATUS_UNMERGED;
- die("bad config variable '%s'", var);
+ return -1;
}
static int git_status_config(const char *k, const char *v, void *cb)
@@ -910,6 +910,8 @@ static int git_status_config(const char *k, const char *v, void *cb)
}
if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
int slot = parse_status_slot(k, 13);
+ if (slot < 0)
+ return -1;
if (!v)
return config_error_nonbool(k);
color_parse(v, k, s->color_palette[slot]);
diff --git a/diff.c b/diff.c
index d952686..08bbd3e 100644
--- a/diff.c
+++ b/diff.c
@@ -63,7 +63,7 @@ static int parse_diff_color_slot(const char *var, int ofs)
return DIFF_WHITESPACE;
if (!strcasecmp(var+ofs, "func"))
return DIFF_FUNCINFO;
- die("bad config variable '%s'", var);
+ return -1;
}
static int git_config_rename(const char *var, const char *value)
@@ -122,6 +122,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
int slot = parse_diff_color_slot(var, 11);
+ if (slot < 0)
+ return 0;
if (!value)
return config_error_nonbool(var);
color_parse(value, var, diff_colors[slot]);
--
1.6.6.rc2.18.gbc86b8
^ permalink raw reply related
* [ANNOUNCE] CGIT 0.8.3.1
From: Lars Hjemli @ 2009-12-12 11:17 UTC (permalink / raw)
To: Git Mailing List
A new bugfix-release of cgit is now available.
Clone: git://hjemli.net/pub/git/cgit
Browse: http://hjemli.net/git/cgit
Changes since 0.8.3
===================
Danijel Tašov (1):
Nov is the correct abbreviation
Geoff Johnstone (1):
Fix repolist search links with virtual root
Lars Hjemli (2):
shared.c: return original errno
CGIT 0.8.3.1
Loui Chang (2):
cgitrc.5.txt: Add mansource and manmanual.
cgitrc.5.txt: Change repo.group to section in example config.
Martins Polakovs (1):
Fix segfault on ppc when browsing tree
Rys Sommefeldt (1):
Close fd on error in readfile()
Sami Kyöstilä (1):
Don't crash when a repo-specific readme file is used
^ permalink raw reply
* [PATCH] bash: Support new 'git fetch' options
From: Björn Gustavsson @ 2009-12-12 10:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
Support the new options --all, --prune, and --dry-run for
'git fetch'.
As the --multiple option was primarily introduced to enable
'git remote update' to be re-implemented in terms of 'git fetch'
(16679e37) and is not likely to be used much from the command
line, it does not seems worthwhile to complicate the code
(to support completion of multiple remotes) to handle it.
Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
contrib/completion/git-completion.bash | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7c18b0c..fbfa5f2 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -417,7 +417,17 @@ __git_complete_remote_or_refspec ()
while [ $c -lt $COMP_CWORD ]; do
i="${COMP_WORDS[c]}"
case "$i" in
- --all|--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
+ --mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
+ --all)
+ case "$cmd" in
+ push) no_complete_refspec=1 ;;
+ fetch)
+ COMPREPLY=()
+ return
+ ;;
+ *) ;;
+ esac
+ ;;
-*) ;;
*) remote="$i"; break ;;
esac
@@ -1002,7 +1012,7 @@ _git_difftool ()
__git_fetch_options="
--quiet --verbose --append --upload-pack --force --keep --depth=
- --tags --no-tags
+ --tags --no-tags --all --prune --dry-run
"
_git_fetch ()
--
1.6.6.rc2
^ permalink raw reply related
* Re: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
From: Junio C Hamano @ 2009-12-12 9:24 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091212091556.GA30509@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Dec 12, 2009 at 01:02:02AM -0800, Junio C Hamano wrote:
>
>> When the desired resolution is to remove the path, "git rm <path>" is the
>> command the user needs to use. Just like in "Changed but not updated"
>> section, suggest to use "git add/rm" as appropriate.
>
> I no longer even see these messages due to advice.statushints, but the
> overall direction of the series looks sane to me.
>
> However:
>
>> - color_fprintf_ln(s->fp, c, "# (use \"git add <file>...\" to mark resolution)");
>> + color_fprintf_ln(s->fp, c, "# (use \"git add/rm <file>...\" as appropriately to mark resolution)");
>
> This should be "as appropriate".
Heh, I kant speel. Fixed and re-pushed out on 'pu'.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
From: Nanako Shiraishi @ 2009-12-12 9:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>
Quoting Junio C Hamano <gitster@pobox.com>
> # On branch side
> # Unmerged paths:
> # (use "git reset HEAD <file>..." to unstage)
> -# (use "git add <file>..." to mark resolution)
> +# (use "git add/rm <file>..." as appropriately to mark resolution)
Shouldn't this either be "as appropriate" or just "appropriately"?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
From: Jeff King @ 2009-12-12 9:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>
On Sat, Dec 12, 2009 at 01:02:02AM -0800, Junio C Hamano wrote:
> When the desired resolution is to remove the path, "git rm <path>" is the
> command the user needs to use. Just like in "Changed but not updated"
> section, suggest to use "git add/rm" as appropriate.
I no longer even see these messages due to advice.statushints, but the
overall direction of the series looks sane to me.
However:
> - color_fprintf_ln(s->fp, c, "# (use \"git add <file>...\" to mark resolution)");
> + color_fprintf_ln(s->fp, c, "# (use \"git add/rm <file>...\" as appropriately to mark resolution)");
This should be "as appropriate".
-Peff
^ permalink raw reply
* [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <1260608523-15579-2-git-send-email-gitster@pobox.com>
When the desired resolution is to remove the path, "git rm <path>" is the
command the user needs to use. Just like in "Changed but not updated"
section, suggest to use "git add/rm" as appropriate.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t7060-wtstatus.sh | 2 +-
wt-status.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 7b5db80..6c1af26 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -32,7 +32,7 @@ cat >expect <<EOF
# On branch side
# Unmerged paths:
# (use "git reset HEAD <file>..." to unstage)
-# (use "git add <file>..." to mark resolution)
+# (use "git add/rm <file>..." as appropriately to mark resolution)
#
# deleted by us: foo
#
diff --git a/wt-status.c b/wt-status.c
index 3fdcf97..5271b6a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -52,7 +52,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
else
color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
- color_fprintf_ln(s->fp, c, "# (use \"git add <file>...\" to mark resolution)");
+ color_fprintf_ln(s->fp, c, "# (use \"git add/rm <file>...\" as appropriately to mark resolution)");
color_fprintf_ln(s->fp, c, "#");
}
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* [PATCH 3/3] status/commit: do not suggest "reset HEAD <path>" while merging
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>
Suggesting "'reset HEAD <path>' to unstage" is dead wrong if we are about
to record a merge commit. For either an unmerged path (i.e. with
unresolved conflicts), or an updated path, it would result in discarding
what the other branch did.
Note that we do not do anything special in a case where we are amending a
merge. The user is making an evil merge starting from an already
committed merge, and running "reset HEAD <path>" is the right way to get
rid of the local edit that has been added to the index.
Once "reset --unresolve <path>" becomes available, we might want to
suggest it for a merged path that has unresolve information, but until
then, just remove the incorrect advice.
We might also want to suggest "checkout --conflict <path>" to revert the
file in the work tree to the state of failed automerge for an unmerged
path, but we never did that, and this commit does not change that.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-commit.c | 2 ++
t/t7060-wtstatus.sh | 1 -
wt-status.c | 14 ++++++++++----
wt-status.h | 1 +
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 17dd462..7218454 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -960,6 +960,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
read_cache();
refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+ s.in_merge = in_merge;
wt_status_collect(&s);
if (s.relative_paths)
@@ -1056,6 +1057,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_commit_config, &s);
in_merge = file_exists(git_path("MERGE_HEAD"));
+ s.in_merge = in_merge;
if (s.use_color == -1)
s.use_color = git_use_color_default;
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 6c1af26..6dd7077 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -31,7 +31,6 @@ test_expect_success 'Report new path with conflict' '
cat >expect <<EOF
# On branch side
# Unmerged paths:
-# (use "git reset HEAD <file>..." to unstage)
# (use "git add/rm <file>..." as appropriately to mark resolution)
#
# deleted by us: foo
diff --git a/wt-status.c b/wt-status.c
index 5271b6a..3f62c44 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -47,8 +47,11 @@ void wt_status_prepare(struct wt_status *s)
static void wt_status_print_unmerged_header(struct wt_status *s)
{
const char *c = color(WT_STATUS_HEADER, s);
+
color_fprintf_ln(s->fp, c, "# Unmerged paths:");
- if (!s->is_initial)
+ if (s->in_merge)
+ ;
+ else if (!s->is_initial)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
else
color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
@@ -59,12 +62,14 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
static void wt_status_print_cached_header(struct wt_status *s)
{
const char *c = color(WT_STATUS_HEADER, s);
+
color_fprintf_ln(s->fp, c, "# Changes to be committed:");
- if (!s->is_initial) {
+ if (s->in_merge)
+ ; /* NEEDSWORK: use "git reset --unresolve"??? */
+ else if (!s->is_initial)
color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
- } else {
+ else
color_fprintf_ln(s->fp, c, "# (use \"git rm --cached <file>...\" to unstage)");
- }
color_fprintf_ln(s->fp, c, "#");
}
@@ -72,6 +77,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,
int has_deleted)
{
const char *c = color(WT_STATUS_HEADER, s);
+
color_fprintf_ln(s->fp, c, "# Changed but not updated:");
if (!has_deleted)
color_fprintf_ln(s->fp, c, "# (use \"git add <file>...\" to update what will be committed)");
diff --git a/wt-status.h b/wt-status.h
index a4bddcf..c60f40a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -34,6 +34,7 @@ struct wt_status {
const char **pathspec;
int verbose;
int amend;
+ int in_merge;
int nowarn;
int use_color;
int relative_paths;
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* [PATCH 0/3] Update advice in commit/status output
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <7vk4wtysyu.fsf@alter.siamese.dyndns.org>
Jay Soffian noticed that we give "git reset HEAD <path>" as an instruction
to get rid of the local change that has already been added to the index
even when <path> is unmerged, or it is merged and we are about to commit a
merge.
In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
while merging.
This miniseries updates the advices given in status/commit. It applies on
top of the jk/1.7.0-status topic, and has trivial conflicts in wt-status.c
with the jk/unwanted-advices topic that has already graduated to 'maint'.
Junio C Hamano (3):
commit/status: check $GIT_DIR/MERGE_HEAD only once
commit/status: "git add <path>" is not necessarily how to resolve
status/commit: do not suggest "reset HEAD <path>" while merging
builtin-commit.c | 12 ++++++------
t/t7060-wtstatus.sh | 3 +--
wt-status.c | 16 +++++++++++-----
wt-status.h | 1 +
4 files changed, 19 insertions(+), 13 deletions(-)
^ permalink raw reply
* [PATCH 1/3] commit/status: check $GIT_DIR/MERGE_HEAD only once
From: Junio C Hamano @ 2009-12-12 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <1260608523-15579-1-git-send-email-gitster@pobox.com>
The code checked for the MERGE_HEAD file to see if we were about
to commit a merge twice in the codepath; also one of them used a
variable merge_head_sha1[] which was set but was never used.
Just check it once, but do so also in "git status", too, as
we will be using this for status generation in the next patch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-commit.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index b39295f..17dd462 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -36,7 +36,7 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static unsigned char head_sha1[20], merge_head_sha1[20];
+static unsigned char head_sha1[20];
static char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
static struct lock_file index_lock; /* real index */
@@ -319,7 +319,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
*/
commit_style = COMMIT_PARTIAL;
- if (file_exists(git_path("MERGE_HEAD")))
+ if (in_merge)
die("cannot do a partial commit during a merge.");
memset(&partial, 0, sizeof(partial));
@@ -758,9 +758,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (get_sha1("HEAD", head_sha1))
initial_commit = 1;
- if (!get_sha1("MERGE_HEAD", merge_head_sha1))
- in_merge = 1;
-
/* Sanity check options */
if (amend && initial_commit)
die("You have nothing to amend.");
@@ -951,6 +948,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_status_config, &s);
+ in_merge = file_exists(git_path("MERGE_HEAD"));
argc = parse_options(argc, argv, prefix,
builtin_status_options,
builtin_status_usage, 0);
@@ -1057,10 +1055,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_commit_config, &s);
+ in_merge = file_exists(git_path("MERGE_HEAD"));
if (s.use_color == -1)
s.use_color = git_use_color_default;
-
argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
prefix, &s);
if (dry_run) {
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* Re: [PATCH 1/3] octopus: make merge process simpler to follow
From: Junio C Hamano @ 2009-12-12 8:53 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Jari Aalto
In-Reply-To: <1260578339-30750-1-git-send-email-bebarino@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Its not very easy to understand what heads are being merged given
> the current output of an octopus merge. Fix this by replacing the
> sha1 with the (usually) better description in GITHEAD_<SHA1>.
>
> Suggested-by: Jari Aalto <jari.aalto@cante.net>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>
> Maybe this will work? At least it will replace the sha1 with
> whatever is given on the command line.
This shows what was given from the command line instead of the
commit object name. I think it makes sense to not even make this
dependent on any option. Also I don't think anybody misses the
commit object name in the output---after all, that is all internal
to git and is different from what the user gave us anyway (unless
the user did give us the full 40-char object name, in which case
the value of GITHEAD_<SHA1> would be that name, and we will show it,
which is also fine).
No, I haven't applied nor run it yet. The above is purely from my cursory
reading of the code.
Anyway, I'll keep this in my inbox or queue it on 'pu' if I had time.
^ permalink raw reply
* Re: FEATURE REQUEST: Announce branch name with merge comamnd
From: Jari Aalto @ 2009-12-12 8:35 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0912111735v474a9d3k5a24024c2d51587b@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> writes:
> On Fri, Dec 11, 2009 at 19:55, Jari Aalto <jari.aalto@cante.net> wrote:
>
>> Please announce the branch name being merged so that the listing is
>> easier to follow (possibly only with --verbose, -v option). Add
>> "Branch: <name>" just before the merge is attempted. somethiglike this
>>
>> Branch: bug--manpage-fix-hyphen
>> Trying simple merge with c87c49b1e413e5dc378d7e6b16951761a1e82f6d
>
> It is not exactly "easier" to follow in your case. It is more
> text and there is no immediately visible cue that the two
> lines are related. You have to give the observer this information.
> Put reference name and SHA-1 on the same line?
That would work too. As long as the branch name appears there
(optionally only with -v in effect).
Jari
^ permalink raw reply
* [PATCH RESEND] gitk: add "--no-replace-objects" option
From: Christian Couder @ 2009-12-12 4:52 UTC (permalink / raw)
To: Paul Mackerras
Cc: git, Michael J Gruber, Jakub Narebski, Johannes Sixt, bill lam,
Andreas Schwab, Junio C Hamano
Replace refs are useful to change some git objects after they
have started to be shared between different repositories. One
might want to ignore them to see the original state, and
"--no-replace-objects" option can be used from the command
line to do so.
This option simply sets the GIT_NO_REPLACE_OBJECTS environment
variable, and that is enough to make gitk ignore replace refs.
The GIT_NO_REPLACE_OBJECTS is set to "1" instead of "" as it is
safer on some platforms, thanks to Johannes Sixt and Michael J
Gruber.
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
gitk-git/gitk | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
I previously sent this patch as part of a series:
http://thread.gmane.org/gmane.comp.version-control.git/133423/focus=133427
and it looks like it has been lost.
Thanks in advance.
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 364c7a8..86dff0f 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -130,7 +130,7 @@ proc unmerged_files {files} {
}
proc parseviewargs {n arglist} {
- global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs
+ global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
set vdatemode($n) 0
set vmergeonly($n) 0
@@ -210,6 +210,9 @@ proc parseviewargs {n arglist} {
# git rev-parse doesn't understand --merge
lappend revargs --gitk-symmetric-diff-marker MERGE_HEAD...HEAD
}
+ "--no-replace-objects" {
+ set env(GIT_NO_REPLACE_OBJECTS) "1"
+ }
"-*" {
# Other flag arguments including -<n>
if {[string is digit -strict [string range $arg 1 end]]} {
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 2/7] reset: add a few tests for "git reset --merge"
From: Christian Couder @ 2009-12-12 4:32 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: <20091212042042.3930.54783.chriscool@tuxfamily.org>
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>
---
t/t7110-reset-merge.sh | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 94 insertions(+), 0 deletions(-)
create mode 100755 t/t7110-reset-merge.sh
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
new file mode 100755
index 0000000..8190da1
--- /dev/null
+++ b/t/t7110-reset-merge.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Christian Couder
+#
+
+test_description='Tests for "git reset --merge"'
+
+. ./test-lib.sh
+
+test_expect_success 'creating initial files' '
+ echo "line 1" >> file1 &&
+ echo "line 2" >> file1 &&
+ echo "line 3" >> file1 &&
+ cp file1 file2 &&
+ git add file1 file2 &&
+ test_tick &&
+ git commit -m "Initial commit"
+'
+
+test_expect_success 'reset --merge is ok with changes in file it does not touch' '
+ echo "line 4" >> file1 &&
+ echo "line 4" >> file2 &&
+ test_tick &&
+ git commit -m "add line 4" file1 &&
+ git reset --merge HEAD^ &&
+ ! grep 4 file1 &&
+ grep 4 file2 &&
+ git reset --merge HEAD@{1} &&
+ grep 4 file1 &&
+ grep 4 file2
+'
+
+test_expect_success 'reset --merge discards changes added to index (1)' '
+ echo "line 5" >> file1 &&
+ git add file1 &&
+ git reset --merge HEAD^ &&
+ ! grep 4 file1 &&
+ ! grep 5 file1 &&
+ grep 4 file2 &&
+ echo "line 5" >> file2 &&
+ git add file2 &&
+ git reset --merge HEAD@{1} &&
+ ! grep 4 file2 &&
+ ! grep 5 file1 &&
+ grep 4 file1
+'
+
+test_expect_success 'reset --merge discards changes added to index (2)' '
+ echo "line 4" >> file2 &&
+ git add file2 &&
+ git reset --merge HEAD^ &&
+ ! grep 4 file2 &&
+ git reset --merge HEAD@{1} &&
+ ! grep 4 file2 &&
+ grep 4 file1
+'
+
+test_expect_success 'reset --merge fails with changes in file it touches' '
+ 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 --merge HEAD^ 2>err.log &&
+ grep file1 err.log | grep "not uptodate" &&
+ git reset --hard HEAD^
+'
+
+test_expect_success 'setup 2 different branches' '
+ git branch branch1 &&
+ git branch branch2 &&
+ git checkout branch1 &&
+ echo "line 5 in branch1" >> file1 &&
+ test_tick &&
+ git commit -a -m "change in branch1" &&
+ git checkout branch2 &&
+ echo "line 5 in branch2" >> file1 &&
+ test_tick &&
+ git commit -a -m "change in branch2"
+'
+
+test_expect_success '"reset --merge HEAD^" fails with pending merge' '
+ test_must_fail git merge branch1 &&
+ test_must_fail git reset --merge HEAD^ &&
+ git reset --hard HEAD
+'
+
+test_expect_success '"reset --merge HEAD" fails with pending merge' '
+ test_must_fail git merge branch1 &&
+ test_must_fail git reset --merge HEAD &&
+ git reset --hard HEAD
+'
+
+test_done
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 5/7] reset: add test cases for "--keep" option
From: Christian Couder @ 2009-12-12 4:32 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: <20091212042042.3930.54783.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 HEAD" is disallowed while it works using
"--keep" instead.
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 | 62 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 61 insertions(+), 1 deletions(-)
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 6afaf73..bd2ef94 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'
. ./test-lib.sh
@@ -30,6 +30,20 @@ test_expect_success 'reset --merge is ok with changes in file it does not touch'
grep 4 file2
'
+test_expect_success 'reset --keep is ok with changes in file it does not touch' '
+ git reset --hard HEAD^ &&
+ echo "line 4" >> file1 &&
+ echo "line 4" >> file2 &&
+ test_tick &&
+ git commit -m "add line 4" file1 &&
+ git reset --keep HEAD^ &&
+ ! grep 4 file1 &&
+ grep 4 file2 &&
+ git reset --keep HEAD@{1} &&
+ grep 4 file1 &&
+ grep 4 file2
+'
+
test_expect_success 'reset --merge discards changes added to index (1)' '
echo "line 5" >> file1 &&
git add file1 &&
@@ -55,6 +69,25 @@ test_expect_success 'reset --merge discards changes added to index (2)' '
grep 4 file1
'
+test_expect_success 'reset --keep fails with changes in index in files it touches' '
+ echo "line 4" >> file2 &&
+ echo "line 5" >> file1 &&
+ git add file1 &&
+ test_must_fail git reset --keep HEAD^ &&
+ git reset --hard HEAD
+'
+
+test_expect_success 'reset --keep keeps changes it does not touch' '
+ echo "line 4" >> file2 &&
+ git add file2 &&
+ git reset --keep HEAD^ &&
+ grep 4 file2 &&
+ git reset --keep HEAD@{1} &&
+ grep 4 file2 &&
+ grep 4 file1 &&
+ git reset --hard HEAD
+'
+
test_expect_success 'reset --merge fails with changes in file it touches' '
echo "line 5" >> file1 &&
test_tick &&
@@ -66,6 +99,17 @@ test_expect_success 'reset --merge fails with changes in file it touches' '
git reset --hard HEAD^
'
+test_expect_success 'reset --keep fails with changes in file it touches' '
+ 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" &&
+ git reset --hard HEAD^
+'
+
test_expect_success 'setup 2 different branches' '
git branch branch1 &&
git branch branch2 &&
@@ -87,10 +131,26 @@ test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
git reset --hard HEAD@{1}
'
+test_expect_success '"reset --keep HEAD^" is ok with pending merge' '
+ test_must_fail git merge branch1 &&
+ git reset --keep HEAD^ &&
+ test -z "$(git diff --cached)" &&
+ test -n "$(git diff)" &&
+ git reset --hard HEAD@{1}
+'
+
test_expect_success '"reset --merge HEAD" fails with pending merge' '
test_must_fail git merge branch1 &&
test_must_fail git reset --merge HEAD &&
git reset --hard HEAD
'
+test_expect_success '"reset --keep HEAD" is ok with pending merge' '
+ test_must_fail git merge branch1 &&
+ git reset --keep HEAD &&
+ test -z "$(git diff --cached)" &&
+ test -n "$(git diff)" &&
+ git reset --hard HEAD
+'
+
test_done
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 7/7] Documentation: reset: add some tables to describe the different options
From: Christian Couder @ 2009-12-12 4:32 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: <20091212042042.3930.54783.chriscool@tuxfamily.org>
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>
---
Documentation/git-reset.txt | 72 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 72 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index f8724d0..4ded9e3 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -72,6 +72,78 @@ 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.
+
+ 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)
+ --keep (disallowed)
+
+ 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)
+ --keep A C C
+
+ 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
+ --keep (disallowed)
+
+ 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
+ --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
+--keep target" will put the file in state B in the working tree
+and in state C in the index and HEAD.
+
+The following tables show what happens when there are unmerged
+entries:
+
+ working index HEAD target working index HEAD
+ ----------------------------------------------------
+ X U A B --soft (disallowed)
+ --mixed X B B
+ --hard B B B
+ --merge X B B
+ --keep X 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)
+ --keep X A A
+
+X means any state and U means an unmerged index.
+
Examples
--------
--
1.6.6.rc1.8.gd33ec
^ permalink raw reply related
* [PATCH v5 3/7] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2009-12-12 4:32 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: <20091212042042.3930.54783.chriscool@tuxfamily.org>
From: Stephan Beyer <s-beyer@gmx.net>
This patch makes "reset_index_file()" call "unpack_trees()" directly
instead of forking and execing "git read-tree". So the code is more
efficient.
And it's also easier to see which unpack_tree() options will be used,
as we don't need to follow "git read-tree"'s command line parsing
which is quite complex.
As Daniel Barkalow found, there is a difference between this new
version and the old one. The old version gives an error for
"git reset --merge" with unmerged entries and the new version does
not. But this can be seen as a bug fix, because "--merge" was the
only "git reset" option with this behavior and this behavior was
not documented.
In fact there is still an error with unmerge entries if we reset
the unmerge entries to the same state as HEAD. So the bug is not
completely fixed.
The code comes from the sequencer GSoC project:
git://repo.or.cz/git/sbeyer.git
(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
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 | 41 ++++++++++++++++++++++++++++++-----------
t/t7110-reset-merge.sh | 8 +++++---
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index 14bdb03..ac38aaa 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -18,6 +18,8 @@
#include "tree.h"
#include "branch.h"
#include "parse-options.h"
+#include "unpack-trees.h"
+#include "cache-tree.h"
static const char * const git_reset_usage[] = {
"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
@@ -54,27 +56,44 @@ static inline int is_merge(void)
static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
{
- int i = 0;
- const char *args[6];
+ int nr = 1;
+ int newfd;
+ struct tree_desc desc[2];
+ struct unpack_trees_options opts;
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- args[i++] = "read-tree";
+ memset(&opts, 0, sizeof(opts));
+ opts.head_idx = 1;
+ opts.src_index = &the_index;
+ opts.dst_index = &the_index;
+ opts.fn = oneway_merge;
+ opts.merge = 1;
if (!quiet)
- args[i++] = "-v";
+ opts.verbose_update = 1;
switch (reset_type) {
case MERGE:
- args[i++] = "-u";
- args[i++] = "-m";
+ opts.update = 1;
break;
case HARD:
- args[i++] = "-u";
+ opts.update = 1;
/* fallthrough */
default:
- args[i++] = "--reset";
+ opts.reset = 1;
}
- args[i++] = sha1_to_hex(sha1);
- args[i] = NULL;
- return run_command_v_opt(args, RUN_GIT_CMD);
+ newfd = hold_locked_index(lock, 1);
+
+ read_cache_unmerged();
+
+ 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))
+ return -1;
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ return error("Could not write new index file.");
+
+ return 0;
}
static void print_new_head_line(struct commit *commit)
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 8190da1..6afaf73 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -79,10 +79,12 @@ test_expect_success 'setup 2 different branches' '
git commit -a -m "change in branch2"
'
-test_expect_success '"reset --merge HEAD^" fails with pending merge' '
+test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
test_must_fail git merge branch1 &&
- test_must_fail git reset --merge HEAD^ &&
- git reset --hard HEAD
+ git reset --merge HEAD^ &&
+ test -z "$(git diff --cached)" &&
+ test -n "$(git diff)" &&
+ git reset --hard HEAD@{1}
'
test_expect_success '"reset --merge HEAD" fails with pending merge' '
--
1.6.6.rc1.8.gd33ec
^ 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