( )⚙ D6244 copies: don't include copies that are not in source in directory move

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6244

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

Authored by martinvonz on Apr 16 2019, 1:16 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG31abb9d713af: copies: don't include copies that are not in source in directory move
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
41f6af50c0d8 (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 a005c33d0bd7 (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 19903277f035 (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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Apr 16 2019, 1:16 PM
martinvonz updated this revision to Diff 14819.Apr 17 2019, 6:20 PM
Closed by commit rHG31abb9d713af: copies: don't include copies that are not in source in directory move (authored by martinvonz). · Explain WhyApr 17 2019, 7:26 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/copies.py (5 lines)
Mtests/test-merge-criss-cross.t (12 lines)

Diff 14772

mercurial/copies.py

ic = remainder[f] ic = remainder[f]
if ic[0] in (m1 if dirtyc1 else m2): if ic[0] in (m1 if dirtyc1 else m2):
# backed-out rename on one side, but watch out for deleted files # backed-out rename on one side, but watch out for deleted files
bothdiverge[f] = ic bothdiverge[f] = ic
for of, fl in bothdiverge.items(): for of, fl in bothdiverge.items():
if len(fl) == 2 and fl[0] == fl[1]: if len(fl) == 2 and fl[0] == fl[1]:
copy[fl[0]] = of # not actually divergent, just matching renames 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: if fullcopy and repo.ui.debugflag:
repo.ui.debug(" all copies found (* = to merge, ! = divergent, " repo.ui.debug(" all copies found (* = to merge, ! = divergent, "
"% = renamed and deleted):\n") "% = renamed and deleted):\n")
for f in sorted(fullcopy): for f in sorted(fullcopy):
note = "" note = ""
if f in copy: if f in copy:
note += "*" note += "*"
if f in divergeset: if f in divergeset:

tests/test-merge-criss-cross.t

calculating bids for ancestor 11b5b303e36c calculating bids for ancestor 11b5b303e36c
searching for copies back to rev 3 searching for copies back to rev 3
unmatched files in local: unmatched files in local:
d1/a d1/a
d1/b d1/b
unmatched files in other: unmatched files in other:
d2/b 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 resolving manifests
branchmerge: True, force: False, partial: False branchmerge: True, force: False, partial: False
ancestor: 11b5b303e36c, local: c0ef19750a22+, remote: 6ca01f7342b9 ancestor: 11b5b303e36c, local: c0ef19750a22+, remote: 6ca01f7342b9
d2/a: remote directory rename - move from d1/a -> dm d2/b: remote created -> g
d2/b: remote directory rename, both created -> m
calculating bids for ancestor 154e6000f54e calculating bids for ancestor 154e6000f54e
searching for copies back to rev 3 searching for copies back to rev 3
unmatched files in other: unmatched files in other:
d2/b d2/b
all copies found (* = to merge, ! = divergent, % = renamed and deleted): all copies found (* = to merge, ! = divergent, % = renamed and deleted):
src: 'd1/b' -> dst: 'd2/b' src: 'd1/b' -> dst: 'd2/b'
checking for directory renames checking for directory renames
discovered dir src: 'd1/' -> dst: 'd2/' discovered dir src: 'd1/' -> dst: 'd2/'
resolving manifests resolving manifests
branchmerge: True, force: False, partial: False branchmerge: True, force: False, partial: False
ancestor: 154e6000f54e, local: c0ef19750a22+, remote: 6ca01f7342b9 ancestor: 154e6000f54e, local: c0ef19750a22+, remote: 6ca01f7342b9
d1/a: other deleted -> r d1/a: other deleted -> r
d1/b: other deleted -> r d1/b: other deleted -> r
d2/b: remote created -> g d2/b: remote created -> g
auction for merging merge bids auction for merging merge bids
d1/a: consensus for r d1/a: consensus for r
d1/b: consensus for r d1/b: consensus for r
d2/a: consensus for dm d2/b: consensus for g
d2/b: picking 'get' action
end of auction end of auction
d1/a: other deleted -> r d1/a: other deleted -> r
removing d1/a removing d1/a
d1/b: other deleted -> r d1/b: other deleted -> r
removing d1/b removing d1/b
d2/b: remote created -> g d2/b: remote created -> g
getting d2/b getting d2/b
1 files updated, 0 files merged, 2 files removed, 0 files unresolved 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
(branch merge, don't forget to commit) (branch merge, don't forget to commit)