* [IRC/patches] Failed octopus merge does not clean up
@ 2008-09-15 22:48 Thomas Rast
2008-09-15 22:49 ` [PATCH] Add test that checks octopus merge cleanup Thomas Rast
2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
0 siblings, 2 replies; 17+ messages in thread
From: Thomas Rast @ 2008-09-15 22:48 UTC (permalink / raw)
To: git; +Cc: Miklos Vajna, Jakub Narebski
[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]
Hi *
James "jammyd" Mulcahy pointed out on IRC that the octopus merge
strategy doesn't properly clean up behind itself. To wit:
git init
echo initial > foo
git add foo
git commit -m initial
echo a > foo
git commit -m a foo
git checkout -b b HEAD^
echo b > foo
git commit -m b foo
git checkout -b c HEAD^
echo c > foo
git commit -m c foo
git checkout master
git merge b c
The merge says
Trying simple merge with 5b3e4bb1c2d88d6967fb575729fbfc86df5eaec9
Simple merge did not work, trying automatic merge.
Auto-merging foo
ERROR: Merge conflict in foo
fatal: merge program failed
Automated merge did not work.
Should not be doing an Octopus.
Merge with strategy octopus failed.
So far so good. However, 'git status' claims
# unmerged: foo
and indeed the contents of 'foo' are the conflicted merge between
'master' and 'b', yet there is no .git/MERGE_HEAD. This behaviour is
identical for 1.5.6 and 1.6.0.2, so it is not caused by the merge
rewrite as a builtin. Shouldn't it either really clean up, or really
leave the repo in a conflicted merge state? (I'm following up with a
patch that turns the above into a test. Octopus doesn't really have
many tests, does it?)
On the code path to the "Merge with strategy %s failed" message,
builtin-merge.c:1134 runs restore_state() which runs reset_hard().
But (as Miklos pointed out) that cannot actually do 'git reset --hard'
because it is possible (though not recommended, see below) to start a
merge with a dirty index.
Jakub mentioned that there are only three index stages for a three-way
merge, so a conflicted n-way (simultaneous) merge is not really
possible.
The merge manpage should warn about merging with uncommitted changes.
It recommends 'git rebase --hard' to abort during conflicts, but does
not mention that this throws away said changes. I'm following up with
a patch for this.
- Thomas
--
Thomas Rast
trast@student.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Add test that checks octopus merge cleanup
2008-09-15 22:48 [IRC/patches] Failed octopus merge does not clean up Thomas Rast
@ 2008-09-15 22:49 ` Thomas Rast
2008-09-15 22:49 ` [PATCH] Documentation: warn against merging in a dirty tree Thomas Rast
2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
1 sibling, 1 reply; 17+ messages in thread
From: Thomas Rast @ 2008-09-15 22:49 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Miklos Vajna
Currently, a conflicted octopus merge leaves the repository in a
"halfway into the merge state", where .git/MERGE_HEAD is missing but
the files are listed as conflicted in the index and have conflict
markers in them. It should either clean up everything, or add a
MERGE_HEAD.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
t/t7607-merge-octopus-cleanup.sh | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/t/t7607-merge-octopus-cleanup.sh b/t/t7607-merge-octopus-cleanup.sh
new file mode 100755
index 0000000..4d82867
--- /dev/null
+++ b/t/t7607-merge-octopus-cleanup.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description='git merge
+
+Testing that conflicting octopus merge fails cleanly.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo initial > foo &&
+ git add foo &&
+ git commit -m initial &&
+ echo a > foo &&
+ git commit -m a foo &&
+ git checkout -b b HEAD^ &&
+ echo b > foo &&
+ git commit -m b foo &&
+ git checkout -b c HEAD^ &&
+ echo c > foo &&
+ git commit -m c foo &&
+ git checkout master
+'
+
+test_expect_failure 'conflicted octopus merge' '
+ test_must_fail git merge b c &&
+ test -z "$(git ls-files --unmerged)" &&
+ test "$(cat foo)" == a &&
+ test ! -f .git/MERGE_HEAD
+'
+
+test_done
--
tg: (1293c95..) t/test-octopus-cleanup (depends on: origin/master)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] Documentation: warn against merging in a dirty tree
2008-09-15 22:49 ` [PATCH] Add test that checks octopus merge cleanup Thomas Rast
@ 2008-09-15 22:49 ` Thomas Rast
2008-09-15 23:42 ` Junio C Hamano
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Rast @ 2008-09-15 22:49 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Miklos Vajna
Merging in a dirty tree is usually a bad idea because you need to
reset --hard to abort; but the docs didn't say anything about it.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-merge.txt | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 685e1fe..3798e16 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -22,6 +22,11 @@ The second syntax (<msg> `HEAD` <remote>) is supported for
historical reasons. Do not use it from the command line or in
new scripts. It is the same as `git merge -m <msg> <remote>`.
+NOTE: Usually it is a bad idea to merge with a dirty tree or index.
+ If you get conflicts and want to abort (instead of resolving),
+ you need to `git reset \--hard` which loses the uncommitted
+ changes.
+
OPTIONS
-------
--
tg: (1293c95..) t/doc-merge-reset (depends on: origin/master)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-15 22:48 [IRC/patches] Failed octopus merge does not clean up Thomas Rast
2008-09-15 22:49 ` [PATCH] Add test that checks octopus merge cleanup Thomas Rast
@ 2008-09-15 23:36 ` Junio C Hamano
2008-09-15 23:47 ` Thomas Rast
2008-09-16 0:20 ` Junio C Hamano
1 sibling, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2008-09-15 23:36 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Miklos Vajna, Jakub Narebski
Thomas Rast <trast@student.ethz.ch> writes:
> The merge says
>
> Trying simple merge with 5b3e4bb1c2d88d6967fb575729fbfc86df5eaec9
> Simple merge did not work, trying automatic merge.
> Auto-merging foo
> ERROR: Merge conflict in foo
> fatal: merge program failed
> Automated merge did not work.
> Should not be doing an Octopus.
> Merge with strategy octopus failed.
>
> So far so good. However, 'git status' claims
> ... This behaviour is
> identical for 1.5.6 and 1.6.0.2, so it is not caused by the merge
> rewrite as a builtin. Shouldn't it either really clean up, or really
> leave the repo in a conflicted merge state? (I'm following up with a
> patch that turns the above into a test. Octopus doesn't really have
> many tests, does it?)
Your analysis is correct --- this has been the correct/established
behaviour of Octopus from day one.
Read the second from the last line of what you were told by git. Run "git
reset --hard" after that, perhaps.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Documentation: warn against merging in a dirty tree
2008-09-15 22:49 ` [PATCH] Documentation: warn against merging in a dirty tree Thomas Rast
@ 2008-09-15 23:42 ` Junio C Hamano
2008-09-15 23:53 ` Avery Pennarun
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2008-09-15 23:42 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Jakub Narebski, Miklos Vajna
Thomas Rast <trast@student.ethz.ch> writes:
> Merging in a dirty tree is usually a bad idea because you need to
> reset --hard to abort; but the docs didn't say anything about it.
Strictly speaking, you do not have to worry about anything if (1) all of
your work tree changes are easily reproducible (Linus's keeping a new
EXTRAVERSION in his Makefile, never staged nor committed is an often cited
example), or (2) you know beforehand that a merge with the other party
will not touch the part you have local changes that you care about.
In other words, you need to know what you are doing, and a warning with
"usually it is a bad idea" would be an appropriate thing to do.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
@ 2008-09-15 23:47 ` Thomas Rast
2008-09-16 0:20 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Thomas Rast @ 2008-09-15 23:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Miklos Vajna, Jakub Narebski
[-- Attachment #1: Type: text/plain, Size: 701 bytes --]
Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
[...]
> > Should not be doing an Octopus.
[...]
> > Shouldn't it either really clean up, or really
> > leave the repo in a conflicted merge state?
[...]
> Your analysis is correct --- this has been the correct/established
> behaviour of Octopus from day one.
Including not cleaning up the half-merged state? If the answer is
"yes", then so be it, merge-octopus has been on this project longer
than I have ;-)
However,
> Run "git reset --hard" after that, perhaps.
wasn't immediately obvious to the end-user (jammyd in this case),
which started the discussion.
--
Thomas Rast
trast@student.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Documentation: warn against merging in a dirty tree
2008-09-15 23:42 ` Junio C Hamano
@ 2008-09-15 23:53 ` Avery Pennarun
2008-09-16 0:06 ` Junio C Hamano
2008-09-18 15:15 ` Linus Torvalds
0 siblings, 2 replies; 17+ messages in thread
From: Avery Pennarun @ 2008-09-15 23:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git, Jakub Narebski, Miklos Vajna
On Mon, Sep 15, 2008 at 7:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
>> Merging in a dirty tree is usually a bad idea because you need to
>> reset --hard to abort; but the docs didn't say anything about it.
>
> Strictly speaking, you do not have to worry about anything if (1) all of
> your work tree changes are easily reproducible (Linus's keeping a new
> EXTRAVERSION in his Makefile, never staged nor committed is an often cited
> example), or (2) you know beforehand that a merge with the other party
> will not touch the part you have local changes that you care about.
>
> In other words, you need to know what you are doing, and a warning with
> "usually it is a bad idea" would be an appropriate thing to do.
But how do you abort a *failed* merge in a situation like Linus's
example? "git reset --hard HEAD" would destroy the unstaged Makefile
change.
I would love to know the answer to this for my own work, and I guess
it would be relevant to the documentation too.
Have fun,
Avery
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Documentation: warn against merging in a dirty tree
2008-09-15 23:53 ` Avery Pennarun
@ 2008-09-16 0:06 ` Junio C Hamano
2008-09-18 15:15 ` Linus Torvalds
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2008-09-16 0:06 UTC (permalink / raw)
To: Avery Pennarun
Cc: Junio C Hamano, Thomas Rast, git, Jakub Narebski, Miklos Vajna
"Avery Pennarun" <apenwarr@gmail.com> writes:
> But how do you abort a *failed* merge in a situation like Linus's
> example? "git reset --hard HEAD" would destroy the unstaged Makefile
> change.
"All of your work tree changes are easily reproducible" implies you do not
mind losing them, Ok?
Also "git reset HEAD" (that is, without --hard) would not touch the work
tree changes. You need to remove the work-tree cruft left by new files
yourself, if you go this route, though. New files are rare enough so it
may be more appropriate (and you can "git clean -n $that_subdirectory" to
enumerate them).
It all depends on your workflow.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
2008-09-15 23:47 ` Thomas Rast
@ 2008-09-16 0:20 ` Junio C Hamano
2008-09-16 22:53 ` Jakub Narebski
1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2008-09-16 0:20 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Miklos Vajna, Jakub Narebski
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <trast@student.ethz.ch> writes:
>
>> The merge says
>>
>> Trying simple merge with 5b3e4bb1c2d88d6967fb575729fbfc86df5eaec9
>> Simple merge did not work, trying automatic merge.
>> Auto-merging foo
>> ERROR: Merge conflict in foo
>> fatal: merge program failed
>> Automated merge did not work.
>> Should not be doing an Octopus.
>> Merge with strategy octopus failed.
> ...
> Read the second from the last line of what you were told by git. Run "git
> reset --hard" after that, perhaps.
By the way, there are three failure modes in Octopus.
(0) your index (not work tree) is dirty; this is not limited to octopus
merge but any merge will be refused;
(1) while it merges other heads one-by-one, it gets conflicts on an
earlier one, before it even attempts to merge all of them. Recording
the heads that it so far attempted to merge in MERGE_HEAD is wrong
(the result won't be an Octopus the end user wanted to carete), and
recording all the heads the user gave in MERGE_HEAD is even more
wrong (it hasn't even looked at the later heads yet, so there is no
way for the index or work tree to contain anything from them).
The above is hitting this case.
(2) it gets conflicts while merging the _last_ head. It records
MERGE_HEAD and allows you to record the resolved result.
I think originally we treated (1) and (2) the same way, because an Octopus
is to record the most trivial merge with more than 2 legs, and a rough
definition of "the most trivial" is "tracks of totally independent works;
you could merge them one by one and _in any order_, and the result won't
matter because they are logically independent. However if they are _so_
independent, why not record them as merged in one step (i.e. octopus),
instead of insisting on recording in what order you merged them".
Obviously, if you get a conflict during Octopus creation, they were not
tracks of totally independent works, and that is where the "Should not" in
the message comes from.
We later relaxed it to allow a conflict at the last step, not because
recording an Octopus with nontrivial merge is particuarly a good idea we
should encourage, but because there simply weren't reason not to.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-16 0:20 ` Junio C Hamano
@ 2008-09-16 22:53 ` Jakub Narebski
2008-09-17 6:45 ` Andreas Ericsson
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Narebski @ 2008-09-16 22:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git, Miklos Vajna
Dnia wtorek 16. września 2008 02:20, Junio C Hamano napisał:
> Junio C Hamano <gitster@pobox.com> writes:
>> Thomas Rast <trast@student.ethz.ch> writes:
>>
>>> The merge says
>>>
>>> Trying simple merge with 5b3e4bb1c2d88d6967fb575729fbfc86df5eaec9
>>> Simple merge did not work, trying automatic merge.
>>> Auto-merging foo
>>> ERROR: Merge conflict in foo
>>> fatal: merge program failed
>>> Automated merge did not work.
>>> Should not be doing an Octopus.
>>> Merge with strategy octopus failed.
>> ...
>> Read the second from the last line of what you were told by git. Run "git
>> reset --hard" after that, perhaps.
The problem, as far as I understand it, is not that octopus merge fails.
The problem is that it fails halfway, and it leaves working are in
inconsistent state: git-status output with its "unmerged" suggests that
we are in the middle of the merge, but we are not.
> By the way, there are three failure modes in Octopus.
>
> (0) your index (not work tree) is dirty; this is not limited to octopus
> merge but any merge will be refused;
IIRC the idea of stashing away changes, doing merge, and then unstashing
was shot down as encouraging bad workflows, and more often than not
leading only to mess in workdir and index.
> (1) while it merges other heads one-by-one, it gets conflicts on an
> earlier one, before it even attempts to merge all of them. Recording
> the heads that it so far attempted to merge in MERGE_HEAD is wrong
> (the result won't be an Octopus the end user wanted to carete), and
> recording all the heads the user gave in MERGE_HEAD is even more
> wrong (it hasn't even looked at the later heads yet, so there is no
> way for the index or work tree to contain anything from them).
>
> The above is hitting this case.
IMVHO the correct solution would be to rollback changes to the state
before attempting a merge. I'd rather 'octopus' did its thing as
transaction; contrary to ordinary merges if merge fails it doesn't
mean necessary that it is in the state of resolvable conflict, state
we can stop at. Perhaps (if it is still a shell script, doing git-stash
at beginning, and either dropping or popping the stash at the end;
or just saving old index if it is built-in).
BTW. does it mean that "git merge a b" might be not the same as
"git merge b a"?
> (2) it gets conflicts while merging the _last_ head. It records
> MERGE_HEAD and allows you to record the resolved result.
>
> I think originally we treated (1) and (2) the same way, because an Octopus
> is to record the most trivial merge with more than 2 legs, and a rough
> definition of "the most trivial" is "tracks of totally independent works;
> you could merge them one by one and _in any order_, and the result won't
> matter because they are logically independent. However if they are _so_
> independent, why not record them as merged in one step (i.e. octopus),
> instead of insisting on recording in what order you merged them".
>
> Obviously, if you get a conflict during Octopus creation, they were not
> tracks of totally independent works, and that is where the "Should not" in
> the message comes from.
>
> We later relaxed it to allow a conflict at the last step, not because
> recording an Octopus with nontrivial merge is particuarly a good idea we
> should encourage, but because there simply weren't reason not to.
Well, octopus merge could simply fail when merge is nontrivial (not
limited to being tree-level merge only).
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-16 22:53 ` Jakub Narebski
@ 2008-09-17 6:45 ` Andreas Ericsson
2008-09-17 8:11 ` Jakub Narebski
0 siblings, 1 reply; 17+ messages in thread
From: Andreas Ericsson @ 2008-09-17 6:45 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Thomas Rast, git, Miklos Vajna
Jakub Narebski wrote:
> Dnia wtorek 16. września 2008 02:20, Junio C Hamano napisał:
>> Junio C Hamano <gitster@pobox.com> writes:
>>> Thomas Rast <trast@student.ethz.ch> writes:
>>>
>>>> The merge says
>>>>
>>>> Trying simple merge with 5b3e4bb1c2d88d6967fb575729fbfc86df5eaec9
>>>> Simple merge did not work, trying automatic merge.
>>>> Auto-merging foo
>>>> ERROR: Merge conflict in foo
>>>> fatal: merge program failed
>>>> Automated merge did not work.
>>>> Should not be doing an Octopus.
>>>> Merge with strategy octopus failed.
>>> ...
>>> Read the second from the last line of what you were told by git. Run "git
>>> reset --hard" after that, perhaps.
>
> The problem, as far as I understand it, is not that octopus merge fails.
> The problem is that it fails halfway, and it leaves working are in
> inconsistent state: git-status output with its "unmerged" suggests that
> we are in the middle of the merge, but we are not.
>
>> By the way, there are three failure modes in Octopus.
>>
>> (0) your index (not work tree) is dirty; this is not limited to octopus
>> merge but any merge will be refused;
>
> IIRC the idea of stashing away changes, doing merge, and then unstashing
> was shot down as encouraging bad workflows, and more often than not
> leading only to mess in workdir and index.
>
>> (1) while it merges other heads one-by-one, it gets conflicts on an
>> earlier one, before it even attempts to merge all of them. Recording
>> the heads that it so far attempted to merge in MERGE_HEAD is wrong
>> (the result won't be an Octopus the end user wanted to carete), and
>> recording all the heads the user gave in MERGE_HEAD is even more
>> wrong (it hasn't even looked at the later heads yet, so there is no
>> way for the index or work tree to contain anything from them).
>>
>> The above is hitting this case.
>
> IMVHO the correct solution would be to rollback changes to the state
> before attempting a merge. I'd rather 'octopus' did its thing as
> transaction; contrary to ordinary merges if merge fails it doesn't
> mean necessary that it is in the state of resolvable conflict, state
> we can stop at. Perhaps (if it is still a shell script, doing git-stash
> at beginning, and either dropping or popping the stash at the end;
> or just saving old index if it is built-in).
>
>
> BTW. does it mean that "git merge a b" might be not the same as
> "git merge b a"?
>
No. Git merges all the sub-things together and then merges the result
of that jumble into the branch you're on.
Someone might have to correct me on that, but that's as far as I've
understood it.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-17 6:45 ` Andreas Ericsson
@ 2008-09-17 8:11 ` Jakub Narebski
2008-09-17 15:59 ` Miklos Vajna
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Narebski @ 2008-09-17 8:11 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, Thomas Rast, git, Miklos Vajna
Andreas Ericsson wrote:
> Jakub Narebski wrote:
> > Junio C Hamano wrote:
>>> (1) while it merges other heads one-by-one, it gets conflicts on an
>>> earlier one, before it even attempts to merge all of them. Recording
>>> the heads that it so far attempted to merge in MERGE_HEAD is wrong
>>> (the result won't be an Octopus the end user wanted to carete), and
>>> recording all the heads the user gave in MERGE_HEAD is even more
>>> wrong (it hasn't even looked at the later heads yet, so there is no
>>> way for the index or work tree to contain anything from them).
>>>
>>> The above is hitting this case.
[...]
>> BTW. does it mean that "git merge a b" might be not the same as
>> "git merge b a"?
>>
>
> No. Git merges all the sub-things together and then merges the result
> of that jumble into the branch you're on.
>
> Someone might have to correct me on that, but that's as far as I've
> understood it.
>From what I understand from above explanation, and from thread on git
mailing list about better implementation of and documenting finding
merge bases for multiple heads, I think octopus merge is done by merging
[reduced] heads one by one into given branch.
This means that "git merge a b" does internally "git merge a; git merge b"
as I understand it.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-17 8:11 ` Jakub Narebski
@ 2008-09-17 15:59 ` Miklos Vajna
2008-09-17 16:40 ` Andreas Ericsson
0 siblings, 1 reply; 17+ messages in thread
From: Miklos Vajna @ 2008-09-17 15:59 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Andreas Ericsson, Junio C Hamano, Thomas Rast, git
[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]
On Wed, Sep 17, 2008 at 10:11:02AM +0200, Jakub Narebski <jnareb@gmail.com> wrote:
> >> BTW. does it mean that "git merge a b" might be not the same as
> >> "git merge b a"?
> >>
> >
> > No. Git merges all the sub-things together and then merges the result
> > of that jumble into the branch you're on.
> >
> > Someone might have to correct me on that, but that's as far as I've
> > understood it.
>
> From what I understand from above explanation, and from thread on git
> mailing list about better implementation of and documenting finding
> merge bases for multiple heads, I think octopus merge is done by merging
> [reduced] heads one by one into given branch.
>
> This means that "git merge a b" does internally "git merge a; git merge b"
> as I understand it.
Sure, but given that both 'a' and 'b' merged (so none of them is subset
of the other, for example so that reduce_heads() would drop one of them)
the order of the parents will be different so the resulting commit will
differ. The resulting tree will no, I think.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [IRC/patches] Failed octopus merge does not clean up
2008-09-17 15:59 ` Miklos Vajna
@ 2008-09-17 16:40 ` Andreas Ericsson
0 siblings, 0 replies; 17+ messages in thread
From: Andreas Ericsson @ 2008-09-17 16:40 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jakub Narebski, Junio C Hamano, Thomas Rast, git
Miklos Vajna wrote:
> On Wed, Sep 17, 2008 at 10:11:02AM +0200, Jakub Narebski <jnareb@gmail.com> wrote:
>>>> BTW. does it mean that "git merge a b" might be not the same as
>>>> "git merge b a"?
>>>>
>>> No. Git merges all the sub-things together and then merges the result
>>> of that jumble into the branch you're on.
>>>
>>> Someone might have to correct me on that, but that's as far as I've
>>> understood it.
>> From what I understand from above explanation, and from thread on git
>> mailing list about better implementation of and documenting finding
>> merge bases for multiple heads, I think octopus merge is done by merging
>> [reduced] heads one by one into given branch.
>>
>> This means that "git merge a b" does internally "git merge a; git merge b"
>> as I understand it.
>
> Sure, but given that both 'a' and 'b' merged (so none of them is subset
> of the other, for example so that reduce_heads() would drop one of them)
> the order of the parents will be different so the resulting commit will
> differ. The resulting tree will no, I think.
I got it wrong (not wrt reduced heads, but still). My apologies.
If octopus (the program/strategy/whatever) continues to merge after a
branch conflicting against the currently checked out branch (let's call
it "master"), the resulting tree may not differ, but then again, it might.
OTOH, if octopus quits the merge after having encountered a conflict, the
order the branches to merge were passed will always have an impact.
Let's say you have two branches, "clean" and "conflict". Which one is
which should be obvious here.
git merge clean conflict
will produce a tree with 'master', 'clean' and a conflicted merge of
'conflict', while
git merge conflict clean
will produce a tree with 'master' and a conflicted merge of 'conflict'.
In short, backing out the entire merge in case of a conflict is almost
certainly the only sane thing to do. If one branch depends on another
to merge cleanly, a different approach should have been taken (depending
branch should have been rebased onto the dependent one prior to merging),
but the merge *might* succeed depending on in which order the other
branches are given as arguments. It's not a very clever idea to merge
something like that though, as bisection will invariably have to mark the
entire depending branch as BAD, although the merge itself could obviously
be a good one.
Clearly, an octopus merge should not be undertaken without knowing very
well what it is one is merging in.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Documentation: warn against merging in a dirty tree
2008-09-15 23:53 ` Avery Pennarun
2008-09-16 0:06 ` Junio C Hamano
@ 2008-09-18 15:15 ` Linus Torvalds
2008-09-18 18:18 ` Avery Pennarun
2008-09-19 20:28 ` Junio C Hamano
1 sibling, 2 replies; 17+ messages in thread
From: Linus Torvalds @ 2008-09-18 15:15 UTC (permalink / raw)
To: Avery Pennarun
Cc: Junio C Hamano, Thomas Rast, git, Jakub Narebski, Miklos Vajna
On Mon, 15 Sep 2008, Avery Pennarun wrote:
>
> But how do you abort a *failed* merge in a situation like Linus's
> example? "git reset --hard HEAD" would destroy the unstaged Makefile
> change.
As mentioned by others, sometimes you are simply willing to take the risk.
If I have dirty data, I still want to merge, because (a) my dirty data is
a _convenience_ and (b) the risk of me having to do a "git reset" is
pretty low anyway.
That said, it's actually kind of sad that we don't expose a real
capability that the git plumbing does have. Namely
git read-tree -u -m HEAD ORIG_HEAD
should do the right thing if you want to undo a merge (except it doesn't
actually write ORIG_HEAD to be the new head: you could use "git reset"
with --soft to do that, or just git update-ref).
So it _may_ be that something like
[alias]
undo = !git read-tree -u -m HEAD ORIG_HEAD && git reset --soft ORIG_HEAD
would actually give you "git undo".
So we have the technology, and we just don't _expose_ that capability as a
"git reset" thing. And we probably should. In fact, that is often the
thing people really want, and it would have made sense to have it as the
default action, but the initial design for "git reset" was literally as a
way to get you out of a sticky corner when you had unmerged entries and
you just wanted to throw away crud.
NOTE NOTE NOTE! I did _not_ test that the git read-tree thing actually
works, or that the above alias does the right thing. Caveat testor! You're
on your own. But I agree that we should have something like that.
Linus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Documentation: warn against merging in a dirty tree
2008-09-18 15:15 ` Linus Torvalds
@ 2008-09-18 18:18 ` Avery Pennarun
2008-09-19 20:28 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Avery Pennarun @ 2008-09-18 18:18 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Thomas Rast, git, Jakub Narebski, Miklos Vajna
On Thu, Sep 18, 2008 at 11:15 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, 15 Sep 2008, Avery Pennarun wrote:
>>
>> But how do you abort a *failed* merge in a situation like Linus's
>> example? "git reset --hard HEAD" would destroy the unstaged Makefile
>> change.
>
> As mentioned by others, sometimes you are simply willing to take the risk.
> If I have dirty data, I still want to merge, because (a) my dirty data is
> a _convenience_ and (b) the risk of me having to do a "git reset" is
> pretty low anyway.
In that case, my next question is how you pull off (b), because that's
*way* better than just being able to undo when I get myself into
trouble :) I do and then reset test merges all the time.
> That said, it's actually kind of sad that we don't expose a real
> capability that the git plumbing does have. Namely
>
> git read-tree -u -m HEAD ORIG_HEAD
>
> should do the right thing if you want to undo a merge (except it doesn't
> actually write ORIG_HEAD to be the new head: you could use "git reset"
> with --soft to do that, or just git update-ref).
Hmm,
$ git read-tree -u -m HEAD ORIG_HEAD
fatal: you need to resolve your current index first
It appears that the above would be great for undoing a
*non*conflicting merge, but that's not as important ;)
> So it _may_ be that something like
>
> [alias]
> undo = !git read-tree -u -m HEAD ORIG_HEAD && git reset --soft ORIG_HEAD
>
> would actually give you "git undo".
>
> So we have the technology, and we just don't _expose_ that capability as a
> "git reset" thing. And we probably should. In fact, that is often the
> thing people really want, and it would have made sense to have it as the
> default action, but the initial design for "git reset" was literally as a
> way to get you out of a sticky corner when you had unmerged entries and
> you just wanted to throw away crud.
Note that if we were going to do an undo, it would be nice to be
careful about allowing multiple consecutive undos. "git undo; git
undo;" shouldn't be a no-op, it should undo two things. At least,
that's how the rest of the world (okay, my text editor) works. "git
redo" could be the opposite of "git undo".
I imagine some trick using the reflog would thus be better here than
updating ORIG_HEAD.
Since that's a lot of work, I'd propose having a first pass at the
undo operation use ORIG_HEAD and then just delete or rename ORIG_HEAD,
so that 'git undo; git undo' is meaningless.
Have fun,
Avery
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Documentation: warn against merging in a dirty tree
2008-09-18 15:15 ` Linus Torvalds
2008-09-18 18:18 ` Avery Pennarun
@ 2008-09-19 20:28 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2008-09-19 20:28 UTC (permalink / raw)
To: Linus Torvalds
Cc: Avery Pennarun, Thomas Rast, git, Jakub Narebski, Miklos Vajna
Linus Torvalds <torvalds@linux-foundation.org> writes:
> That said, it's actually kind of sad that we don't expose a real
> capability that the git plumbing does have. Namely
>
> git read-tree -u -m HEAD ORIG_HEAD
I do not think this is quite enough. "read-tree --reset -u" is probably
closer, but it may discard the entries added (and got conflicted) by the
merge before actually updating the working tree (the right thing there
would be to remove them).
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-09-19 20:29 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-15 22:48 [IRC/patches] Failed octopus merge does not clean up Thomas Rast
2008-09-15 22:49 ` [PATCH] Add test that checks octopus merge cleanup Thomas Rast
2008-09-15 22:49 ` [PATCH] Documentation: warn against merging in a dirty tree Thomas Rast
2008-09-15 23:42 ` Junio C Hamano
2008-09-15 23:53 ` Avery Pennarun
2008-09-16 0:06 ` Junio C Hamano
2008-09-18 15:15 ` Linus Torvalds
2008-09-18 18:18 ` Avery Pennarun
2008-09-19 20:28 ` Junio C Hamano
2008-09-15 23:36 ` [IRC/patches] Failed octopus merge does not clean up Junio C Hamano
2008-09-15 23:47 ` Thomas Rast
2008-09-16 0:20 ` Junio C Hamano
2008-09-16 22:53 ` Jakub Narebski
2008-09-17 6:45 ` Andreas Ericsson
2008-09-17 8:11 ` Jakub Narebski
2008-09-17 15:59 ` Miklos Vajna
2008-09-17 16:40 ` Andreas Ericsson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).