Patchwork D6244: copies: don't include copies that are not in source in directory move

login
register
mail settings
Submitter phabricator
Date April 16, 2019, 5:16 p.m.
Message ID <differential-rev-PHID-DREV-7cgviegi6odsbeqdgxrf-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39645/
State Superseded
Headers show

Comments

phabricator - April 16, 2019, 5:16 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I've been working on a rewrite of mergecopies(). I compared the output
  of the rewritten version with the current version. I noticed that
  between FIREFOX_NIGHTLY_59_END and FIREFOX_BETA_60_BASE in the
  mozilla-unified repo, there were many copies that the current version
  detected that the rewritten version did not. One example was
  js/src/gc/Iteration.h -> js/src/gc/PublicIterators.h. Then I realized
  that js/src/gc/Iteration.h doesn't even exist in
  FIREFOX_NIGHTLY_59_END.
  
  This patch adds a filtering step for the "fullcopy" dict. It turns out
  that that change also affects the test for issue5020 in
  test-merge-criss-cross.t. The 'dm' action no longer happens there. At
  first I thought that the test case change meant that this patch was
  broken, but I think it's actually correct tha the 'dm' action should
  not happen there. The result of the bid merge is still the same.
  
  I suspect this filtering is a better solution for the issue than
  https://phab.mercurial-scm.org/rHG41f6af50c0d8824600b1112da932dcdf399e8d5d (merge: fix crash on criss cross merge with dir move and
  delete (issue5020), 2017-01-31). I also suspect that it was broken
  just a few months earlier by https://phab.mercurial-scm.org/rHGa005c33d0bd785745a905e87f34e90885d0f8cc4 (mergecopies: add logic to
  process incomplete data, 2016-10-04). Note that bid merge had been
  enabled for a few years at that point, since https://phab.mercurial-scm.org/rHG19903277f0355e3e4b1a991e7fc4b51f71cfd553 (merge: use
  bid merge by default (BC), 2014-10-01).
  
  This patch is still just a workaround. It will be cleaned up soon
  (with the rewrite of mergecopies()). But doing this in a separate
  patch makes later patches easier to understand and gives a place to
  explain why this is changing.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/copies.py
  tests/test-merge-criss-cross.t

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/test-merge-criss-cross.t b/tests/test-merge-criss-cross.t
--- a/tests/test-merge-criss-cross.t
+++ b/tests/test-merge-criss-cross.t
@@ -423,17 +423,10 @@ 
      d1/b
     unmatched files in other:
      d2/b
-    all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'd1/b' -> dst: 'd2/b' 
-    checking for directory renames
-     discovered dir src: 'd1/' -> dst: 'd2/'
-     pending file src: 'd1/a' -> dst: 'd2/a'
-     pending file src: 'd1/b' -> dst: 'd2/b'
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 11b5b303e36c, local: c0ef19750a22+, remote: 6ca01f7342b9
-   d2/a: remote directory rename - move from d1/a -> dm
-   d2/b: remote directory rename, both created -> m
+   d2/b: remote created -> g
   
   calculating bids for ancestor 154e6000f54e
     searching for copies back to rev 3
@@ -453,8 +446,7 @@ 
   auction for merging merge bids
    d1/a: consensus for r
    d1/b: consensus for r
-   d2/a: consensus for dm
-   d2/b: picking 'get' action
+   d2/b: consensus for g
   end of auction
   
    d1/a: other deleted -> r
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -682,6 +682,11 @@ 
         if len(fl) == 2 and fl[0] == fl[1]:
             copy[fl[0]] = of # not actually divergent, just matching renames
 
+    # Sometimes we get invalid copies here (the "and not remotebase" in
+    # _checkcopies() seems suspicious). Filter them out.
+    for dst, src in fullcopy.copy().items():
+        if src not in mb:
+            del fullcopy[dst]
     if fullcopy and repo.ui.debugflag:
         repo.ui.debug("  all copies found (* = to merge, ! = divergent, "
                       "% = renamed and deleted):\n")