* [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: [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: [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: [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
* 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: [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: [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
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).