D8243: copies: stop recoding buggy file merge when new file overwrite an old one

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recoding buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The `BD/DB` and `BF/FB` cases in `tests/test-copies-chain-merge.t` shown an
  inconsistent behavior when merging two branches where one plainly replace a file
  untouched on the other side.
 
  We do the minimal amount of work in the commit code to fix this bug. The case is
  narrow enough that the performance impact does not scare me right now.
 
  My priority is to get the behavior fixed with the minimal code changes. (Because
  I need to behavior to be right to make more progress on the changeset centric
  algorithm). We can iterate later on a more efficient approach (updating the
  merge state to record the situation at the time it is initially detected).
 
  The `tests/test-convert-hg-source.t` is affected because it also created such
  graphs.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

AFFECTED FILES
  mercurial/localrepo.py
  tests/test-convert-hg-source.t
  tests/test-copies-chain-merge.t

CHANGE DETAILS

diff --git a/tests/test-copies-chain-merge.t b/tests/test-copies-chain-merge.t
--- a/tests/test-copies-chain-merge.t
+++ b/tests/test-copies-chain-merge.t
@@ -212,7 +212,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBDm-0 simple merge - one way'
   $ hg up 'desc("d-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -313,8 +313,7 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
+       2      22 c72365ee036f 000000000000 000000000000
   $ hg up 'desc("b-1")'
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("f-2")'
@@ -322,7 +321,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBFm-0 simple merge - one way'
   $ hg up 'desc("f-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -598,17 +597,12 @@
 - one with change to an unrelated file
 - one deleting and recreating the change
 
-Note:
-| In this case, one of the merge wrongly record a merge while there is none.
-| This lead to bad copy tracing information to be dug up.
-
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mBDm-0")'
   M d
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mDBm-0")'
   M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mBDm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mDBm-0")'
   M b
   $ hg status --copies --rev 'desc("i-2")' --rev 'desc("mBDm-0")'
@@ -618,18 +612,14 @@
   M b
   M d
 
-The bugs makes recorded copy is different depending of where we started the merge from since
+
+The result must be the same for both merges
 
   $ hg manifest --debug --rev 'desc("mBDm-0")' | grep '644   d'
-  0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 644   d
+  b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("mDBm-0")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
 
-The 0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 entry is wrong, since the file was
-deleted on one side (then recreate) and untouched on the other side, no "merge"
-has happened. The resulting `d` file is the untouched version from branch `D`,
-not a merge.
-
   $ hg manifest --debug --rev 'desc("d-2")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("b-1")' | grep '644   d'
@@ -638,29 +628,16 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
-       4      23 863d9bc49190 01c2f5eabdc4 c72365ee036f
-       5      25 7bded9d9da1f 01c2f5eabdc4 000000000000
-       6      26 f04cac32d703 b004912a8510 7bded9d9da1f
-       7      27 d7a5eafb9322 7bded9d9da1f b004912a8510
-       8      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
-
-(That output if wrong, since no merge actually happened).
+       2      22 c72365ee036f 000000000000 000000000000
+       3      25 7bded9d9da1f 01c2f5eabdc4 000000000000
+       4      26 f04cac32d703 b004912a8510 7bded9d9da1f
+       5      27 d7a5eafb9322 7bded9d9da1f b004912a8510
+       6      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
 
   $ hg log -Gfr 'desc("mBDm-0")' d
-  o    15 mBDm-0 simple merge - one way]
-  |\
-  o :  14 d-2 re-add d]
-  :/
-  o  2 i-2: c -move-> d]
+  o  14 d-2 re-add d]
   |
-  o  1 i-1: a -move-> c]
-  |
-  o  0 i-0 initial commit: a b]
-  
-
-This output is correct
+  ~
 
   $ hg log -Gfr 'desc("mDBm-0")' d
   o  14 d-2 re-add d]
@@ -670,7 +647,6 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBDm-0")'
   M b
   A d
-    a (true !)
   R a
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mDBm-0")'
   M b
@@ -746,8 +722,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBFm-0")'
   M b
   A d
-    a (true !)
-    h (false !)
+    h
   R a
   R h
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mFBm-0")'
@@ -761,7 +736,6 @@
   R h
   $ hg status --copies --rev 'desc("f-2")' --rev 'desc("mBFm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("f-1")' --rev 'desc("mBFm-0")'
   M b
   M d
@@ -779,16 +753,10 @@
 The following graphlog is wrong, the "a -> c -> d" chain was overwritten and should not appear.
 
   $ hg log -Gfr 'desc("mBFm-0")' d
-  o    23 mBFm-0 simple merge - one way]
-  |\
-  o :  22 f-2: rename i -> d]
-  | :
-  o :  21 f-1: rename h -> i]
-  :/
-  o  2 i-2: c -move-> d]
+  o  22 f-2: rename i -> d]
   |
-  o  1 i-1: a -move-> c]
-  |
+  o  21 f-1: rename h -> i]
+  :
   o  0 i-0 initial commit: a b]
   
 
diff --git a/tests/test-convert-hg-source.t b/tests/test-convert-hg-source.t
--- a/tests/test-convert-hg-source.t
+++ b/tests/test-convert-hg-source.t
@@ -62,9 +62,9 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ cd new
@@ -76,7 +76,7 @@
 #if execbit
   $ hg bookmarks
      premerge1                 3:973ef48a98a4
-     premerge2                 8:91d107c423ba
+     premerge2                 8:c4968fdf2e5d
 #else
 Different hash because no x bit
   $ hg bookmarks
@@ -96,19 +96,19 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ hg -R new log -G -T '{rev} {desc}'
   o  8 mark baz executable
   |
-  o  7 Added tag this for changeset c56a7f387039
+  o  7 Added tag this for changeset 706614b458c1
   |
   o  6 Removed tag that
   |
-  o  5 Added tag that for changeset 88586c4e9f02
+  o  5 Added tag that for changeset 8601262d7472
   |
   o    4 merge remote copy
   |\
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2849,6 +2849,39 @@
                 fparent1, fparent2 = fparent2, nullid
             elif fparent2 in fparentancestors:
                 fparent2 = nullid
+            elif not fparentancestors:
+                # The two file seems entirely unrelated, one might be a full
+                # replacement of the other.
+                #
+                # At that point, the logic get complicated enough that we reuse
+                # the existing one.
+                #
+                # Ideally, we would store this information in the mergestate a
+                # simply reuse it. It could probably simplify the surounding
+                # code too.
+                p1ctx = fctx._changectx.p1()
+                p2ctx = fctx._changectx.p2()
+                matcher = matchmod.exact([fctx.path()])
+                if self.ui.configlist(b'merge', b'preferancestor') == [b'*']:
+                    cahs = self.changelog.commonancestorsheads(
+                        p1ctx.node(), p2ctx.node()
+                    )
+                    pas = [self[anc] for anc in (sorted(cahs) or [nullid])]
+                else:
+                    pas = [p1ctx.ancestor(p2ctx, warn=False)]
+                actionbyfile, __, __ = mergemod.calculateupdates(
+                    self,
+                    p1ctx,
+                    p2ctx,
+                    pas,
+                    branchmerge=True,
+                    force=True,
+                    acceptremote=False,
+                    followcopies=True,
+                    matcher=matcher,
+                )
+                if actionbyfile[fctx.path()][0] == mergemod.ACTION_GET:
+                    fparent1, fparent2 = fparent2, nullid
 
         # is the file changed?
         text = fctx.data()



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recoding buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
marmoute updated this revision to Diff 20550.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8243?vs=20533&id=20550

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

AFFECTED FILES
  mercurial/localrepo.py
  tests/test-convert-hg-source.t
  tests/test-copies-chain-merge.t

CHANGE DETAILS

diff --git a/tests/test-copies-chain-merge.t b/tests/test-copies-chain-merge.t
--- a/tests/test-copies-chain-merge.t
+++ b/tests/test-copies-chain-merge.t
@@ -212,7 +212,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBDm-0 simple merge - one way'
   $ hg up 'desc("d-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -313,8 +313,7 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
+       2      22 c72365ee036f 000000000000 000000000000
   $ hg up 'desc("b-1")'
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("f-2")'
@@ -322,7 +321,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBFm-0 simple merge - one way'
   $ hg up 'desc("f-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -598,17 +597,12 @@
 - one with change to an unrelated file
 - one deleting and recreating the change
 
-Note:
-| In this case, one of the merge wrongly record a merge while there is none.
-| This lead to bad copy tracing information to be dug up.
-
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mBDm-0")'
   M d
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mDBm-0")'
   M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mBDm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mDBm-0")'
   M b
   $ hg status --copies --rev 'desc("i-2")' --rev 'desc("mBDm-0")'
@@ -618,18 +612,14 @@
   M b
   M d
 
-The bugs makes recorded copy is different depending of where we started the merge from since
+
+The result must be the same for both merges
 
   $ hg manifest --debug --rev 'desc("mBDm-0")' | grep '644   d'
-  0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 644   d
+  b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("mDBm-0")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
 
-The 0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 entry is wrong, since the file was
-deleted on one side (then recreate) and untouched on the other side, no "merge"
-has happened. The resulting `d` file is the untouched version from branch `D`,
-not a merge.
-
   $ hg manifest --debug --rev 'desc("d-2")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("b-1")' | grep '644   d'
@@ -638,29 +628,16 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
-       4      23 863d9bc49190 01c2f5eabdc4 c72365ee036f
-       5      25 7bded9d9da1f 01c2f5eabdc4 000000000000
-       6      26 f04cac32d703 b004912a8510 7bded9d9da1f
-       7      27 d7a5eafb9322 7bded9d9da1f b004912a8510
-       8      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
-
-(That output if wrong, since no merge actually happened).
+       2      22 c72365ee036f 000000000000 000000000000
+       3      25 7bded9d9da1f 01c2f5eabdc4 000000000000
+       4      26 f04cac32d703 b004912a8510 7bded9d9da1f
+       5      27 d7a5eafb9322 7bded9d9da1f b004912a8510
+       6      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
 
   $ hg log -Gfr 'desc("mBDm-0")' d
-  o    15 mBDm-0 simple merge - one way]
-  |\
-  o :  14 d-2 re-add d]
-  :/
-  o  2 i-2: c -move-> d]
+  o  14 d-2 re-add d]
   |
-  o  1 i-1: a -move-> c]
-  |
-  o  0 i-0 initial commit: a b]
-  
-
-This output is correct
+  ~
 
   $ hg log -Gfr 'desc("mDBm-0")' d
   o  14 d-2 re-add d]
@@ -670,7 +647,6 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBDm-0")'
   M b
   A d
-    a (true !)
   R a
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mDBm-0")'
   M b
@@ -746,8 +722,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBFm-0")'
   M b
   A d
-    a (true !)
-    h (false !)
+    h
   R a
   R h
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mFBm-0")'
@@ -761,7 +736,6 @@
   R h
   $ hg status --copies --rev 'desc("f-2")' --rev 'desc("mBFm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("f-1")' --rev 'desc("mBFm-0")'
   M b
   M d
@@ -779,16 +753,10 @@
 The following graphlog is wrong, the "a -> c -> d" chain was overwritten and should not appear.
 
   $ hg log -Gfr 'desc("mBFm-0")' d
-  o    23 mBFm-0 simple merge - one way]
-  |\
-  o :  22 f-2: rename i -> d]
-  | :
-  o :  21 f-1: rename h -> i]
-  :/
-  o  2 i-2: c -move-> d]
+  o  22 f-2: rename i -> d]
   |
-  o  1 i-1: a -move-> c]
-  |
+  o  21 f-1: rename h -> i]
+  :
   o  0 i-0 initial commit: a b]
   
 
diff --git a/tests/test-convert-hg-source.t b/tests/test-convert-hg-source.t
--- a/tests/test-convert-hg-source.t
+++ b/tests/test-convert-hg-source.t
@@ -62,9 +62,9 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ cd new
@@ -76,7 +76,7 @@
 #if execbit
   $ hg bookmarks
      premerge1                 3:973ef48a98a4
-     premerge2                 8:91d107c423ba
+     premerge2                 8:c4968fdf2e5d
 #else
 Different hash because no x bit
   $ hg bookmarks
@@ -96,19 +96,19 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ hg -R new log -G -T '{rev} {desc}'
   o  8 mark baz executable
   |
-  o  7 Added tag this for changeset c56a7f387039
+  o  7 Added tag this for changeset 706614b458c1
   |
   o  6 Removed tag that
   |
-  o  5 Added tag that for changeset 88586c4e9f02
+  o  5 Added tag that for changeset 8601262d7472
   |
   o    4 merge remote copy
   |\
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2852,6 +2852,39 @@
                 fparent1, fparent2 = fparent2, nullid
             elif fparent2 in fparentancestors:
                 fparent2 = nullid
+            elif not fparentancestors:
+                # The two file seems entirely unrelated, one might be a full
+                # replacement of the other.
+                #
+                # At that point, the logic get complicated enough that we reuse
+                # the existing one.
+                #
+                # Ideally, we would store this information in the mergestate a
+                # simply reuse it. It could probably simplify the surounding
+                # code too.
+                p1ctx = fctx._changectx.p1()
+                p2ctx = fctx._changectx.p2()
+                matcher = matchmod.exact([fctx.path()])
+                if self.ui.configlist(b'merge', b'preferancestor') == [b'*']:
+                    cahs = self.changelog.commonancestorsheads(
+                        p1ctx.node(), p2ctx.node()
+                    )
+                    pas = [self[anc] for anc in (sorted(cahs) or [nullid])]
+                else:
+                    pas = [p1ctx.ancestor(p2ctx, warn=False)]
+                actionbyfile, __, __ = mergemod.calculateupdates(
+                    self,
+                    p1ctx,
+                    p2ctx,
+                    pas,
+                    branchmerge=True,
+                    force=True,
+                    acceptremote=False,
+                    followcopies=True,
+                    matcher=matcher,
+                )
+                if actionbyfile[fctx.path()][0] == mergemod.ACTION_GET:
+                    fparent1, fparent2 = fparent2, nullid
 
         # is the file changed?
         text = fctx.data()



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recoding buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
martinvonz added inline comments.

INLINE COMMENTS

> localrepo.py:2868-2874
> +                if self.ui.configlist(b'merge', b'preferancestor') == [b'*']:
> +                    cahs = self.changelog.commonancestorsheads(
> +                        p1ctx.node(), p2ctx.node()
> +                    )
> +                    pas = [self[anc] for anc in (sorted(cahs) or [nullid])]
> +                else:
> +                    pas = [p1ctx.ancestor(p2ctx, warn=False)]

Is this correct when grafting (not merging), such as when rebasing? Is it worth having a test case for that? Is it even possible to get here in that case? Maybe when rebasing a merge commit?

> test-copies-chain-merge.t:663
> -
> -This output is correct
>  

Does the removal of this comment mean that it's no longer correct? I don't think that's what you meant, so should the comment remain (or even not have been added until now)?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recoding buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added inline comments.

INLINE COMMENTS

> martinvonz wrote in test-copies-chain-merge.t:663
> Does the removal of this comment mean that it's no longer correct? I don't think that's what you meant, so should the comment remain (or even not have been added until now)?

The previous output is no longer wrong, so we no longer need to specify the next one is wrong.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recoding buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
martinvonz added inline comments.

INLINE COMMENTS

> marmoute wrote in test-copies-chain-merge.t:663
> The previous output is no longer wrong, so we no longer need to specify the next one is wrong.

Ah, so that comment meant to say that that "unlike the test case just before, this one is correct". That wasn't clear to me. But no need to change anything since it's going away anyway.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added a comment.
marmoute retitled this revision from "copies: stop recoding buggy file merge when new file overwrite an old one" to "copies: stop recording buggy file merge when new file overwrite an old one".
marmoute updated this revision to Diff 20595.


  series update after Martin feedback

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8243?vs=20550&id=20595

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

AFFECTED FILES
  mercurial/localrepo.py
  tests/test-convert-hg-source.t
  tests/test-copies-chain-merge.t

CHANGE DETAILS

diff --git a/tests/test-copies-chain-merge.t b/tests/test-copies-chain-merge.t
--- a/tests/test-copies-chain-merge.t
+++ b/tests/test-copies-chain-merge.t
@@ -212,7 +212,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBDm-0 simple merge - one way'
   $ hg up 'desc("d-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -314,8 +314,7 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
+       2      22 c72365ee036f 000000000000 000000000000
   $ hg up 'desc("b-1")'
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("f-2")'
@@ -323,7 +322,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBFm-0 simple merge - one way'
   $ hg up 'desc("f-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -537,17 +536,12 @@
 - one with change to an unrelated file
 - one deleting and recreating the change
 
-Note:
-| In this case, one of the merge wrongly record a merge while there is none.
-| This lead to bad copy tracing information to be dug up.
-
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mBDm-0")'
   M d
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mDBm-0")'
   M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mBDm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mDBm-0")'
   M b
   $ hg status --copies --rev 'desc("i-2")' --rev 'desc("mBDm-0")'
@@ -557,18 +551,14 @@
   M b
   M d
 
-The bugs makes recorded copy is different depending of where we started the merge from since
+
+The result must be the same for both merges
 
   $ hg manifest --debug --rev 'desc("mBDm-0")' | grep '644   d'
-  0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 644   d
+  b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("mDBm-0")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
 
-The 0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 entry is wrong, since the file was
-deleted on one side (then recreate) and untouched on the other side, no "merge"
-has happened. The resulting `d` file is the untouched version from branch `D`,
-not a merge.
-
   $ hg manifest --debug --rev 'desc("d-2")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("b-1")' | grep '644   d'
@@ -577,29 +567,16 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
-       4      23 863d9bc49190 01c2f5eabdc4 c72365ee036f
-       5      25 7bded9d9da1f 01c2f5eabdc4 000000000000
-       6      26 f04cac32d703 b004912a8510 7bded9d9da1f
-       7      27 d7a5eafb9322 7bded9d9da1f b004912a8510
-       8      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
-
-(This `hg log` output if wrong, since no merge actually happened).
+       2      22 c72365ee036f 000000000000 000000000000
+       3      25 7bded9d9da1f 01c2f5eabdc4 000000000000
+       4      26 f04cac32d703 b004912a8510 7bded9d9da1f
+       5      27 d7a5eafb9322 7bded9d9da1f b004912a8510
+       6      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
 
   $ hg log -Gfr 'desc("mBDm-0")' d
-  o    15 mBDm-0 simple merge - one way]
-  |\
-  o :  14 d-2 re-add d]
-  :/
-  o  2 i-2: c -move-> d]
+  o  14 d-2 re-add d]
   |
-  o  1 i-1: a -move-> c]
-  |
-  o  0 i-0 initial commit: a b h]
-  
-
-This `hg log` output is correct
+  ~
 
   $ hg log -Gfr 'desc("mDBm-0")' d
   o  14 d-2 re-add d]
@@ -609,7 +586,6 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBDm-0")'
   M b
   A d
-    a
   R a
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mDBm-0")'
   M b
@@ -685,8 +661,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBFm-0")'
   M b
   A d
-    a (true !)
-    h (false !)
+    h
   R a
   R h
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mFBm-0")'
@@ -700,7 +675,6 @@
   R h
   $ hg status --copies --rev 'desc("f-2")' --rev 'desc("mBFm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("f-1")' --rev 'desc("mBFm-0")'
   M b
   M d
@@ -718,16 +692,10 @@
 The following graphlog is wrong, the "a -> c -> d" chain was overwritten and should not appear.
 
   $ hg log -Gfr 'desc("mBFm-0")' d
-  o    23 mBFm-0 simple merge - one way]
-  |\
-  o :  22 f-2: rename i -> d]
-  | :
-  o :  21 f-1: rename h -> i]
-  :/
-  o  2 i-2: c -move-> d]
+  o  22 f-2: rename i -> d]
   |
-  o  1 i-1: a -move-> c]
-  |
+  o  21 f-1: rename h -> i]
+  :
   o  0 i-0 initial commit: a b h]
   
 
diff --git a/tests/test-convert-hg-source.t b/tests/test-convert-hg-source.t
--- a/tests/test-convert-hg-source.t
+++ b/tests/test-convert-hg-source.t
@@ -62,9 +62,9 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ cd new
@@ -76,7 +76,7 @@
 #if execbit
   $ hg bookmarks
      premerge1                 3:973ef48a98a4
-     premerge2                 8:91d107c423ba
+     premerge2                 8:c4968fdf2e5d
 #else
 Different hash because no x bit
   $ hg bookmarks
@@ -96,19 +96,19 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ hg -R new log -G -T '{rev} {desc}'
   o  8 mark baz executable
   |
-  o  7 Added tag this for changeset c56a7f387039
+  o  7 Added tag this for changeset 706614b458c1
   |
   o  6 Removed tag that
   |
-  o  5 Added tag that for changeset 88586c4e9f02
+  o  5 Added tag that for changeset 8601262d7472
   |
   o    4 merge remote copy
   |\
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2850,6 +2850,39 @@
                 fparent1, fparent2 = fparent2, nullid
             elif fparent2 in fparentancestors:
                 fparent2 = nullid
+            elif not fparentancestors:
+                # The two file seems entirely unrelated, one might be a full
+                # replacement of the other.
+                #
+                # At that point, the logic get complicated enough that we reuse
+                # the existing one.
+                #
+                # Ideally, we would store this information in the mergestate a
+                # simply reuse it. It could probably simplify the surounding
+                # code too.
+                p1ctx = fctx._changectx.p1()
+                p2ctx = fctx._changectx.p2()
+                matcher = matchmod.exact([fctx.path()])
+                if self.ui.configlist(b'merge', b'preferancestor') == [b'*']:
+                    cahs = self.changelog.commonancestorsheads(
+                        p1ctx.node(), p2ctx.node()
+                    )
+                    pas = [self[anc] for anc in (sorted(cahs) or [nullid])]
+                else:
+                    pas = [p1ctx.ancestor(p2ctx, warn=False)]
+                actionbyfile, __, __ = mergemod.calculateupdates(
+                    self,
+                    p1ctx,
+                    p2ctx,
+                    pas,
+                    branchmerge=True,
+                    force=True,
+                    acceptremote=False,
+                    followcopies=True,
+                    matcher=matcher,
+                )
+                if actionbyfile[fctx.path()][0] == mergemod.ACTION_GET:
+                    fparent1, fparent2 = fparent2, nullid
 
         # is the file changed?
         text = fctx.data()



To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added inline comments.

INLINE COMMENTS

> martinvonz wrote in localrepo.py:2868-2874
> Is this correct when grafting (not merging), such as when rebasing? Is it worth having a test case for that? Is it even possible to get here in that case? Maybe when rebasing a merge commit?

I don't know, but not existing test broke. Consolidating test for the plain merge case already took a long time. Not having any test failure from exisitng rebase/graft test seemed enough to me while making progress on the correctness of this.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute updated this revision to Diff 20755.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8243?vs=20595&id=20755

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

AFFECTED FILES
  mercurial/localrepo.py
  tests/test-convert-hg-source.t
  tests/test-copies-chain-merge.t

CHANGE DETAILS

diff --git a/tests/test-copies-chain-merge.t b/tests/test-copies-chain-merge.t
--- a/tests/test-copies-chain-merge.t
+++ b/tests/test-copies-chain-merge.t
@@ -212,7 +212,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBDm-0 simple merge - one way'
   $ hg up 'desc("d-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -314,8 +314,7 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
+       2      22 c72365ee036f 000000000000 000000000000
   $ hg up 'desc("b-1")'
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("f-2")'
@@ -323,7 +322,7 @@
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBFm-0 simple merge - one way'
   $ hg up 'desc("f-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -537,17 +536,12 @@
 - one with change to an unrelated file
 - one deleting and recreating the change
 
-Note:
-| In this case, one of the merge wrongly record a merge while there is none.
-| This lead to bad copy tracing information to be dug up.
-
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mBDm-0")'
   M d
   $ hg status --copies --rev 'desc("b-1")' --rev 'desc("mDBm-0")'
   M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mBDm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mDBm-0")'
   M b
   $ hg status --copies --rev 'desc("i-2")' --rev 'desc("mBDm-0")'
@@ -557,18 +551,14 @@
   M b
   M d
 
-The bugs makes recorded copy is different depending of where we started the merge from since
+
+The result must be the same for both merges
 
   $ hg manifest --debug --rev 'desc("mBDm-0")' | grep '644   d'
-  0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 644   d
+  b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("mDBm-0")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
 
-The 0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 entry is wrong, since the file was
-deleted on one side (then recreate) and untouched on the other side, no "merge"
-has happened. The resulting `d` file is the untouched version from branch `D`,
-not a merge.
-
   $ hg manifest --debug --rev 'desc("d-2")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("b-1")' | grep '644   d'
@@ -577,29 +567,16 @@
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1      10 b004912a8510 000000000000 000000000000
-       2      15 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
-       4      23 863d9bc49190 01c2f5eabdc4 c72365ee036f
-       5      25 7bded9d9da1f 01c2f5eabdc4 000000000000
-       6      26 f04cac32d703 b004912a8510 7bded9d9da1f
-       7      27 d7a5eafb9322 7bded9d9da1f b004912a8510
-       8      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
-
-(This `hg log` output if wrong, since no merge actually happened).
+       2      22 c72365ee036f 000000000000 000000000000
+       3      25 7bded9d9da1f 01c2f5eabdc4 000000000000
+       4      26 f04cac32d703 b004912a8510 7bded9d9da1f
+       5      27 d7a5eafb9322 7bded9d9da1f b004912a8510
+       6      28 2ed7a51aed47 c72365ee036f 7bded9d9da1f
 
   $ hg log -Gfr 'desc("mBDm-0")' d
-  o    15 mBDm-0 simple merge - one way]
-  |\
-  o :  14 d-2 re-add d]
-  :/
-  o  2 i-2: c -move-> d]
+  o  14 d-2 re-add d]
   |
-  o  1 i-1: a -move-> c]
-  |
-  o  0 i-0 initial commit: a b h]
-  
-
-This `hg log` output is correct
+  ~
 
   $ hg log -Gfr 'desc("mDBm-0")' d
   o  14 d-2 re-add d]
@@ -609,7 +586,6 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBDm-0")'
   M b
   A d
-    a
   R a
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mDBm-0")'
   M b
@@ -685,8 +661,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBFm-0")'
   M b
   A d
-    a (true !)
-    h (false !)
+    h
   R a
   R h
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mFBm-0")'
@@ -700,7 +675,6 @@
   R h
   $ hg status --copies --rev 'desc("f-2")' --rev 'desc("mBFm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("f-1")' --rev 'desc("mBFm-0")'
   M b
   M d
@@ -718,16 +692,10 @@
 The following graphlog is wrong, the "a -> c -> d" chain was overwritten and should not appear.
 
   $ hg log -Gfr 'desc("mBFm-0")' d
-  o    23 mBFm-0 simple merge - one way]
-  |\
-  o :  22 f-2: rename i -> d]
-  | :
-  o :  21 f-1: rename h -> i]
-  :/
-  o  2 i-2: c -move-> d]
+  o  22 f-2: rename i -> d]
   |
-  o  1 i-1: a -move-> c]
-  |
+  o  21 f-1: rename h -> i]
+  :
   o  0 i-0 initial commit: a b h]
   
 
diff --git a/tests/test-convert-hg-source.t b/tests/test-convert-hg-source.t
--- a/tests/test-convert-hg-source.t
+++ b/tests/test-convert-hg-source.t
@@ -62,9 +62,9 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ cd new
@@ -76,7 +76,7 @@
 #if execbit
   $ hg bookmarks
      premerge1                 3:973ef48a98a4
-     premerge2                 8:91d107c423ba
+     premerge2                 8:c4968fdf2e5d
 #else
 Different hash because no x bit
   $ hg bookmarks
@@ -96,19 +96,19 @@
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ hg -R new log -G -T '{rev} {desc}'
   o  8 mark baz executable
   |
-  o  7 Added tag this for changeset c56a7f387039
+  o  7 Added tag this for changeset 706614b458c1
   |
   o  6 Removed tag that
   |
-  o  5 Added tag that for changeset 88586c4e9f02
+  o  5 Added tag that for changeset 8601262d7472
   |
   o    4 merge remote copy
   |\
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2855,6 +2855,39 @@
                 fparent1, fparent2 = fparent2, nullid
             elif fparent2 in fparentancestors:
                 fparent2 = nullid
+            elif not fparentancestors:
+                # The two file seems entirely unrelated, one might be a full
+                # replacement of the other.
+                #
+                # At that point, the logic get complicated enough that we reuse
+                # the existing one.
+                #
+                # Ideally, we would store this information in the mergestate a
+                # simply reuse it. It could probably simplify the surounding
+                # code too.
+                p1ctx = fctx._changectx.p1()
+                p2ctx = fctx._changectx.p2()
+                matcher = matchmod.exact([fctx.path()])
+                if self.ui.configlist(b'merge', b'preferancestor') == [b'*']:
+                    cahs = self.changelog.commonancestorsheads(
+                        p1ctx.node(), p2ctx.node()
+                    )
+                    pas = [self[anc] for anc in (sorted(cahs) or [nullid])]
+                else:
+                    pas = [p1ctx.ancestor(p2ctx, warn=False)]
+                actionbyfile, __, __ = mergemod.calculateupdates(
+                    self,
+                    p1ctx,
+                    p2ctx,
+                    pas,
+                    branchmerge=True,
+                    force=True,
+                    acceptremote=False,
+                    followcopies=True,
+                    matcher=matcher,
+                )
+                if actionbyfile[fctx.path()][0] == mergemod.ACTION_GET:
+                    fparent1, fparent2 = fparent2, nullid
 
         # is the file changed?
         text = fctx.data()



To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
martinvonz added a comment.


  FYI, I'd like to look at this patch, but I'm busy with other things right now (switching back and forth between browser tabs to delay the interview feedback that I'm actually supposed to write), so it will take a bit longer before I'll find time for this one.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
martinvonz added a comment.


  This is pretty ugly and it doesn't seem that the next patch depends on it. You said you'll soon clean it up anyway, so I wonder if should just wait for the better solution instead. It doesn't seem like this fixes a serious bug so we have to rush it. Thoughts?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added a comment.


  In D8243#123559 <https://phab.mercurial-scm.org/D8243#123559>, @martinvonz wrote:
 
  > This is pretty ugly and it doesn't seem that the next patch depends on it. You said you'll soon clean it up anyway, so I wonder if should just wait for the better solution instead. It doesn't seem like this fixes a serious bug so we have to rush it. Thoughts?
 
  My initial motivation to rush the ugly way was "getting the behavior right to compare with the changeset centric one and being able to test performance improvement for the changeset centric one while having access to a specific repository". However, cancelling of all travel has cancelled the window to access that repo. I'll resubmit a cleaner versions soon.
 
  since you did not commented on it, I assume the new behavior is fine by you, right?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
martinvonz added a comment.


  In D8243#124077 <https://phab.mercurial-scm.org/D8243#124077>, @marmoute wrote:
 
  > In D8243#123559 <https://phab.mercurial-scm.org/D8243#123559>, @martinvonz wrote:
  >
  >> This is pretty ugly and it doesn't seem that the next patch depends on it. You said you'll soon clean it up anyway, so I wonder if should just wait for the better solution instead. It doesn't seem like this fixes a serious bug so we have to rush it. Thoughts?
  >
  > My initial motivation to rush the ugly way was "getting the behavior right to compare with the changeset centric one and being able to test performance improvement for the changeset centric one while having access to a specific repository". However, cancelling of all travel has cancelled the window to access that repo. I'll resubmit a cleaner versions soon.
  > since you did not commented on it, I assume the new behavior is fine by you, right?
 
  You mean the new behavior in this patch? Yes, I think I'm fine with it. I initially wasn't sure if the old behavior should be considered a bug, but I think you're right that it should.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
This revision now requires changes to proceed.
durin42 added a comment.
durin42 requested changes to this revision.


  Per irc checkin,  we expect changes on this before review.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers, durin42
Cc: durin42, martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
pulkit added a comment.


  D8392 <https://phab.mercurial-scm.org/D8392> is an attempt to fix the same issue by storing extra information in mergestate.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers, durin42
Cc: pulkit, durin42, martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added a comment.


  In D8243#125293 <https://phab.mercurial-scm.org/D8243#125293>, @pulkit wrote:
 
  > D8392 <https://phab.mercurial-scm.org/D8392> is an attempt to fix the same issue by storing extra information in mergestate.
 
  The approach (storing in the merge state) is the one I planned to implement before handling the task to Pulkit. so +1 on the approach.
 
  (feel free to reuse that very same diff for it.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers, durin42
Cc: pulkit, durin42, martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8243: copies: stop recording buggy file merge when new file overwrite an old one

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added a comment.
marmoute abandoned this revision.


  superseeded by D8392 <https://phab.mercurial-scm.org/D8392>

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8243/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8243

To: marmoute, #hg-reviewers, durin42
Cc: pulkit, durin42, martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel