Issue1561

Title unnecessary merge conflicts in rebase
Priority bug Status resolved
Superseder merge uses wrong ancestor (and consequently does the wrong thing)
View: 1327
Nosy List astratto, cboos, friedrich, jcoomes, mpm, tonfa
Assigned To astratto Topics rebase

Created on 2009-03-17.08:30:09 by friedrich, last changed 2009-11-11.21:05:36 by cboos.

Files
File name Uploaded Type Edit Remove
enforce_newancestor_usage.diff cboos, 2009-11-09.12:48:22 text/plain
rebase_merge.sh friedrich, 2009-03-17.08:46:40 application/x-shellscript
Messages
msg10954 (view) Author: cboos Date: 2009-11-11.21:05:35
Fixed in main (http://selenic.com/repo/hg/rev/49efeed49c94) for Mercurial 1.4.
msg10936 (view) Author: cboos Date: 2009-11-09.22:41:32
A fixed patch, along with a test case, is currently proposed on the mailing 
list.
msg10931 (view) Author: cboos Date: 2009-11-09.12:48:22
The enforce_newancestor_usage.diff patch fixes both msg10930 and the original 
msg8847 use cases.

Let me know if I should also post on mercurial-devel or if attaching the patch 
here is enough (I seem to remember the "dead on arrival" advice ;-) ).
msg10930 (view) Author: cboos Date: 2009-11-09.11:28:57
I don't think this is the same as issue1327.

The problem discussed here has nothing to do with picking the wrong "real" 
ancestor (it always did, in the examples here), it's rather that merge doesn't 
use the ancestor that rebase wants it to use, which might be different from the 
"real" ancestor.

The result is that overlapping changes in the same file occurring in successive 
rebased changesets will generate unnecessary merge conflicts.

I'll reproduce below a very short test case, which I've reported on the mailing 
list last summer (http://selenic.com/pipermail/mercurial-devel/2009-
August/014939.html).

----
hg init testrebasequeue
cd testrebasequeue
echo test > file
hg ci -Am 'initial version, patches will first be on top of that'
echo more data >> file
hg ci -m 'preparing target for rebase'
hg up 0
echo some leading text > file
echo test >> file
hg qnew -f add_first_line
echo some leading line > file
echo test >> file
hg qnew -f modify_first_line
HGMERGE=internal:merge hg rebase --debug --base qbase --dest 1
----

Testing with:

$ hg version
Mercurial Distributed SCM (version 1.3.1+383-6cb1808e5ae7)

(i.e. 27 changesets after f8ca4035a949).

Just before the rebase operation, we have:
----
@  3:[mq]: modify_first_line 9848f5759da7
|
o  2:[mq]: add_first_line 18d95753c96e
|
| o  1:preparing target for rebase b5eff2a0194f
|/
o  0:initial version, patches will first be on top of that f14da50a19b2
----

Rebasing the first patch works, but not the second:
----
...
rebasing 2:18d95753c96e
...
my file@b5eff2a0194f+ other file@18d95753c96e ancestor file@f14da50a19b2
...
rebasing 3:9848f5759da7
...
resolving manifests
 ...
 ancestor 18d95753c96e local b932c6d7da51+ remote 9848f5759da7
...
my file@b932c6d7da51+ other file@9848f5759da7 ancestor file@f14da50a19b2
----

So the correct "rebase" ancestor (18d95753c96e) was found during the "resolving 
manifests" step, but for performing the merge, the "real" ancestor 
(f14da50a19b2) is used.

It's like astratto wrote in msg9727 below:
  actually the ancestor returned by rebasemerge
  is the correct one (the first). The line with my... is returned by filemerge
  that seems to override the modified ancestor with a wrong one (it's the real
  ancestor and we don't need it).

Can we reopen?
msg10925 (view) Author: mpm Date: 2009-11-08.18:08:25
I'm marking this as superceded by issue1327 and closing it.
msg9759 (view) Author: astratto Date: 2009-06-25.15:25:53
Bad news. I've had some spare time to take a deeper look at this, but I still
haven't found a way to solve it.
Using an absolutely dirty hack I'm able to get filemerge to use the correct
ancestor:
merge against 2:7e1afe9214b2
resolving manifests
 ancestor 0a6620c3c26a local 0b69665d48b4+ remote 7e1afe9214b2
picked tool 'internal:merge' for a (binary False symlink False)
my a@0b69665d48b4+ other a@7e1afe9214b2 ancestor a@0a6620c3c26a

but again I get:
warning: conflicts during merge.
merging a failed!
abort: fix unresolved conflicts with hg resolve then run hg rebase --continue

Starting from the next week I'll be very busy again. I'm afraid I won't work on
this before 1.3.

As a future reference: mpm said to take a peek at issue1327
I'm adding mpm to the nosy list...
msg9727 (view) Author: astratto Date: 2009-06-22.17:34:01
I don't think that's the problem, actually the ancestor returned by rebasemerge
is the correct one (the first). The line with my... is returned by filemerge
that seems to override the modified ancestor with a wrong one (it's the real
ancestor and we don't need it).

Initial situation:

@  3:66559d44c7db3a5155bbaf8c7a364504a42932e2 D                               
|                                                                             
| o  2:7e1afe9214b270fa61150d316a28758ca92f956d C                             
| |                                                                           
| o  1:0a6620c3c26a757c498308a9f6773d154681bde0 B                             
|/                                                                            
o  0:1e635d440a7348d13e5fafba3efc35d9a54c2cb1 A                               

rebase onto 3 starting from 1                                                 
merge against 1:0a6620c3c26a
first revision, do not change ancestor
resolving manifests
 ancestor 1e635d440a73 local 66559d44c7db+ remote 0a6620c3c26a
picked tool 'internal:merge' for a (binary False symlink False)
my a@66559d44c7db+ other a@0a6620c3c26a ancestor a@1e635d440a73

Intermediate situation:
o  4:0b69665d48b4b5cacd2eb0e92d8cfa5a6a1d2f16 B                               
|                                                                             
@  3:66559d44c7db3a5155bbaf8c7a364504a42932e2 D                               
|                                                                             
| o  2:7e1afe9214b270fa61150d316a28758ca92f956d C                             
| |                                                                           
| o  1:0a6620c3c26a757c498308a9f6773d154681bde0 B                             
|/                                                                            
o  0:1e635d440a7348d13e5fafba3efc35d9a54c2cb1 A                               
                                                                              
merge against 2:7e1afe9214b2
Invoked newancestor
Using 0a6620c3c26a757c498308a9f6773d154681bde0 instead of
1e635d440a7348d13e5fafba3efc35d9a54c2cb1
resolving manifests
 ancestor 0a6620c3c26a local 0b69665d48b4+ remote 7e1afe9214b2
picked tool 'internal:merge' for a (binary False symlink False)
my a@0b69665d48b4+ other a@7e1afe9214b2 ancestor a@1e635d440a73

The line "Invoked newancestor" shows which revision has been returned by
rebasemerge.
I think we need to find a way to prevent filemerge from using the original ancestor.
msg9487 (view) Author: friedrich Date: 2009-06-11.00:03:45
I would show the glog immediately after the faild rebase:

@  4[tip]   0b69665d48b4   1970-01-01 00:00 +0000   test
|    B
|
o  3:0   66559d44c7db   1970-01-01 00:00 +0000   test
|    D
|
| @  2   7e1afe9214b2   1970-01-01 00:00 +0000   test
| |    C
| |
| o  1   0a6620c3c26a   1970-01-01 00:00 +0000   test
|/     B
|
o  0   1e635d440a73   1970-01-01 00:00 +0000   test
     A
msg9486 (view) Author: friedrich Date: 2009-06-11.00:00:34
I located the problem:

% hg version
Mercurial Distributed SCM (version 1.2.1)

% sh rebase_merge.sh
[...]
rebasing 2:7e1afe9214b2
 future parents are 4 and -1
 already in target
 merge against 2:7e1afe9214b2
resolving manifests
 overwrite False partial False
 ancestor 0a6620c3c26a local 0b69665d48b4+ remote 7e1afe9214b2
  searching for copies back to rev 1
 a: versions differ -> m
preserving a for resolve of a
couldn't find merge tool hgmerge
picked tool 'internal:merge' for a (binary False symlink False)
merging a
my a@0b69665d48b4+ other a@7e1afe9214b2 ancestor a@1e635d440a73
warning: conflicts during merge.
merging a failed!

and

% hg -R testrepo glog          
@  3[tip]:0   66559d44c7db   1970-01-01 00:00 +0000   test
|    D
|
| o  2   7e1afe9214b2   1970-01-01 00:00 +0000   test
| |    C
| |
| o  1   0a6620c3c26a   1970-01-01 00:00 +0000   test
|/     B
|
o  0   1e635d440a73   1970-01-01 00:00 +0000   test
     A

At the line

  ancestor 0a6620c3c26a local 0b69665d48b4+ remote 7e1afe9214b2

the ancastor is right. But at the merge

  my a@0b69665d48b4+ other a@7e1afe9214b2 ancestor a@1e635d440a73

the ancastor is wrong! The cause is in hgext/rebase.py line 23 (introduced
in 45495d784ad6):

...
def rebasemerge(repo, rev, first=False):
    'return the correct ancestor'
    oldancestor = ancestor.ancestor

    def newancestor(a, b, pfunc):
        ancestor.ancestor = oldancestor
        anc = ancestor.ancestor(a, b, pfunc)
        if b == rev:
            return repo[rev].parents()[0].rev()
        return ancestor.ancestor(a, b, pfunc)

    if not first:
        ancestor.ancestor = newancestor
    else:
        repo.ui.debug(_("first revision, do not change ancestor\n"))
...

The design idea at line 30:

  if b == rev:

is right but the implemtation is buggy because 'b' could be a normal 
rev (thats ok), like in mercurial/revlog.py line 1106:

  c = ancestor.ancestor(self.rev(a), self.rev(b), parents)

But 'b' could also be a tuple (that's bad), like in mercurial/context.py
line 463:

  a, b = (self._path, self._filenode), (fc2._path, fc2._filenode)
  v = ancestor.ancestor(a, b, parents)

Any idea?
msg8847 (view) Author: friedrich Date: 2009-03-17.08:46:40
The rebase command should do firstly all the merges and _then_ all the reverts.
The following rebase session fails (with unnecessary merge conflicts) when the
rebasing changesets modifies the same lines:

#!/bin/sh

makerepo() {
rm -rf testrepo
hg init testrepo
cd testrepo

echo "A" > a
echo "" >> a
hg commit -Am 'A' -u test -d '0 0'

sed -i -e 's/A/B/' a
hg commit -m 'B' -u test -d '1 0'

sed -i -e 's/B/C/' a
hg commit -Am 'C' -u test -d '2 0'

hg up 0
echo "D" >> a
hg commit -Am 'D' -u test -d '3 0'

hg glog --template '{rev}:{desc} {node|short}\n'
}

makerepo
echo
echo "This rebase fails"
hg rebase --debug -s 1 -d 3

hg rebase -a

makerepo
echo
echo "Manual rebase"
hg up 3
hg merge 1 --debug
hg ci -m 'Merge with B'
hg merge 2 --debug
hg ci -m 'Merge with C'

hg up 3
hg revert -ar 4
hg ci -m 'Rebased B'
hg revert -ar 5
hg ci -m 'Rebased C'
hg strip 4
echo
echo "The result from the manual rebase"
hg glog --template '{rev}:{desc} {node|short}\n'
echo
echo "The manual rebased file a:"
cat a
History
Date User Action Args
2009-11-27 07:03:14cbooslinkissue1830 superseder
2009-11-25 13:03:23cbooslinkissue1301 superseder
2009-11-11 21:05:36cboossetstatus: in-progress -> resolved
nosy: mpm, tonfa, cboos, jcoomes, astratto, friedrich
messages: + msg10954
2009-11-09 22:41:33cboossetnosy: mpm, tonfa, cboos, jcoomes, astratto, friedrich
messages: + msg10936
2009-11-09 13:12:25tonfasetnosy: + tonfa
2009-11-09 12:48:22cboossetstatus: chatting -> in-progress
nosy: mpm, cboos, jcoomes, astratto, friedrich
messages: + msg10931
files: + enforce_newancestor_usage.diff
2009-11-09 11:28:58cboossetstatus: resolved -> chatting
nosy: mpm, cboos, jcoomes, astratto, friedrich
messages: + msg10930
2009-11-08 18:08:25mpmsetstatus: chatting -> resolved
nosy: mpm, cboos, jcoomes, astratto, friedrich
superseder: + merge uses wrong ancestor (and consequently does the wrong thing)
messages: + msg10925
2009-09-01 07:10:33cboossetnosy: + cboos
2009-06-25 15:25:54astrattosetnosy: + mpm
messages: + msg9759
title: rebase bug -> unnecessary merge conflicts in rebase
2009-06-22 17:34:01astrattosettopic: + rebase
assignedto: astratto
messages: + msg9727
2009-06-11 00:03:45friedrichsetmessages: + msg9487
2009-06-11 00:00:36friedrichsetmessages: + msg9486
2009-03-17 16:16:40jcoomessetnosy: + jcoomes
2009-03-17 08:47:43friedrichsetnosy: + astratto
2009-03-17 08:46:40friedrichsetfiles: + rebase_merge.sh
messages: + msg8847
2009-03-17 08:44:44friedrichsetfiles: - rebase_merge.sh
2009-03-17 08:44:38friedrichsetmessages: - msg8846
2009-03-17 08:44:03friedrichsetfiles: + rebase_merge.sh
messages: + msg8846
2009-03-17 08:31:02friedrichsetstatus: unread -> chatting
messages: - msg8845
2009-03-17 08:30:09friedrichcreate