This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7986

copies: make mergecopies() distinguish between copies on each side
ClosedPublic

Authored by martinvonz on Jan 24 2020, 5:00 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG6ca9f45b32b0: copies: make mergecopies() distinguish between copies on each side
Summary

I find it confusing that most of the dicts returned from
mergecopies() have entries specific to one branch of the merge, but
they're still combined into dict. For example, you can't tell if `copy

{"bar": "foo"}` means that "foo" was copied to "bar" on the first

branch or the second.

It also feels like there are bugs lurking here because we may mistake
which side the copy happened on. However, for most of the dicts, it's
not possible that there is disagreement. For example, renamedelete
keeps track of renames that happened on one side of the merge where
the other side deleted the file. There can't be a disagreement there
(because we record that in the diverge dict instead). For regular
copies/renames, there can be a disagreement. Let's say file "foo" was
copied to "bar" on one branch and file "baz" was copied to "bar" on
the other. Beacause we only return one copy dict, we end up
replacing the {"bar": "foo"} entry by {"bar": "baz"}. The merge
code (manifestmerge()) will then decide that that means "both
renamed from 'baz'". We should probably treat it as a conflict
instead.

The next few patches will make mergecopies() return two instances of
most of the returned copies. That will lead to a bit more code (~40
lines), but I think it makes both copies.mergecopies() and
merge.manifestmerge() clearer.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jan 24 2020, 5:00 PM
martinvonz added a commit: rHG6ca9f45b32b0: copies: make mergecopies() distinguish between copies on each side.Jan 29 2020, 6:18 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG6ca9f45b32b0: copies: make mergecopies() distinguish between copies on each side (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/copies.py (53 lines)

Diff 19682

mercurial/copies.py

inversecopies1 = {} inversecopies1 = {}
inversecopies2 = {} inversecopies2 = {}
for dst, src in copies1.items(): for dst, src in copies1.items():
inversecopies1.setdefault(src, []).append(dst) inversecopies1.setdefault(src, []).append(dst)
for dst, src in copies2.items(): for dst, src in copies2.items():
inversecopies2.setdefault(src, []).append(dst) inversecopies2.setdefault(src, []).append(dst)
copy = {} copy1 = {}
copy2 = {}
diverge = {} diverge = {}
renamedelete = {} renamedelete1 = {}
renamedelete2 = {}
allsources = set(inversecopies1) | set(inversecopies2) allsources = set(inversecopies1) | set(inversecopies2)
for src in allsources: for src in allsources:
dsts1 = inversecopies1.get(src) dsts1 = inversecopies1.get(src)
dsts2 = inversecopies2.get(src) dsts2 = inversecopies2.get(src)
if dsts1 and dsts2: if dsts1 and dsts2:
# copied/renamed on both sides # copied/renamed on both sides
if src not in m1 and src not in m2: if src not in m1 and src not in m2:
# renamed on both sides # renamed on both sides
dsts1 = set(dsts1) dsts1 = set(dsts1)
dsts2 = set(dsts2) dsts2 = set(dsts2)
# If there's some overlap in the rename destinations, we # If there's some overlap in the rename destinations, we
# consider it not divergent. For example, if side 1 copies 'a' # consider it not divergent. For example, if side 1 copies 'a'
# to 'b' and 'c' and deletes 'a', and side 2 copies 'a' to 'c' # to 'b' and 'c' and deletes 'a', and side 2 copies 'a' to 'c'
# and 'd' and deletes 'a'. # and 'd' and deletes 'a'.
if dsts1 & dsts2: if dsts1 & dsts2:
for dst in dsts1 & dsts2: for dst in dsts1 & dsts2:
copy[dst] = src copy1[dst] = src
copy2[dst] = src
else: else:
diverge[src] = sorted(dsts1 | dsts2) diverge[src] = sorted(dsts1 | dsts2)
elif src in m1 and src in m2: elif src in m1 and src in m2:
# copied on both sides # copied on both sides
dsts1 = set(dsts1) dsts1 = set(dsts1)
dsts2 = set(dsts2) dsts2 = set(dsts2)
for dst in dsts1 & dsts2: for dst in dsts1 & dsts2:
copy[dst] = src copy1[dst] = src
copy2[dst] = src
# TODO: Handle cases where it was renamed on one side and copied # TODO: Handle cases where it was renamed on one side and copied
# on the other side # on the other side
elif dsts1: elif dsts1:
# copied/renamed only on side 1 # copied/renamed only on side 1
_checksinglesidecopies( _checksinglesidecopies(
src, dsts1, m1, m2, mb, c2, base, copy, renamedelete src, dsts1, m1, m2, mb, c2, base, copy1, renamedelete1
) )
elif dsts2: elif dsts2:
# copied/renamed only on side 2 # copied/renamed only on side 2
_checksinglesidecopies( _checksinglesidecopies(
src, dsts2, m2, m1, mb, c1, base, copy, renamedelete src, dsts2, m2, m1, mb, c1, base, copy2, renamedelete2
) )
# find interesting file sets from manifests # find interesting file sets from manifests
addedinm1 = m1.filesnotin(mb, repo.narrowmatch()) addedinm1 = m1.filesnotin(mb, repo.narrowmatch())
addedinm2 = m2.filesnotin(mb, repo.narrowmatch()) addedinm2 = m2.filesnotin(mb, repo.narrowmatch())
u1 = sorted(addedinm1 - addedinm2) u1 = sorted(addedinm1 - addedinm2)
u2 = sorted(addedinm2 - addedinm1) u2 = sorted(addedinm2 - addedinm1)
header = b" unmatched files in %s" header = b" unmatched files in %s"
if u1: if u1:
repo.ui.debug(b"%s:\n %s\n" % (header % b'local', b"\n ".join(u1))) repo.ui.debug(b"%s:\n %s\n" % (header % b'local', b"\n ".join(u1)))
if u2: if u2:
repo.ui.debug(b"%s:\n %s\n" % (header % b'other', b"\n ".join(u2))) repo.ui.debug(b"%s:\n %s\n" % (header % b'other', b"\n ".join(u2)))
fullcopy = copies1.copy() fullcopy = copies1.copy()
fullcopy.update(copies2) fullcopy.update(copies2)
if repo.ui.debugflag: if repo.ui.debugflag:
renamedeleteset = set() renamedeleteset = set()
divergeset = set() divergeset = set()
for dsts in diverge.values(): for dsts in diverge.values():
divergeset.update(dsts) divergeset.update(dsts)
for dsts in renamedelete.values(): for dsts in renamedelete1.values():
renamedeleteset.update(dsts)
for dsts in renamedelete2.values():
renamedeleteset.update(dsts) renamedeleteset.update(dsts)
repo.ui.debug( repo.ui.debug(
b" all copies found (* = to merge, ! = divergent, " b" all copies found (* = to merge, ! = divergent, "
b"% = renamed and deleted):\n" b"% = renamed and deleted):\n"
) )
for f in sorted(fullcopy): for f in sorted(fullcopy):
note = b"" note = b""
if f in copy: if f in copy1 or f in copy2:
note += b"*" note += b"*"
if f in divergeset: if f in divergeset:
note += b"!" note += b"!"
if f in renamedeleteset: if f in renamedeleteset:
note += b"%" note += b"%"
repo.ui.debug( repo.ui.debug(
b" src: '%s' -> dst: '%s' %s\n" % (fullcopy[f], f, note) b" src: '%s' -> dst: '%s' %s\n" % (fullcopy[f], f, note)
) )
del renamedeleteset del renamedeleteset
del divergeset del divergeset
repo.ui.debug(b" checking for directory renames\n") repo.ui.debug(b" checking for directory renames\n")
dirmove, movewithdir = _dir_renames(repo, c1, c2, copy, fullcopy, u1, u2) dirmove1, movewithdir2 = _dir_renames(repo, c1, copy1, copies1, u2)
dirmove2, movewithdir1 = _dir_renames(repo, c2, copy2, copies2, u1)
return copy, movewithdir, diverge, renamedelete, dirmove copy1.update(copy2)
renamedelete1.update(renamedelete2)
movewithdir1.update(movewithdir2)
dirmove1.update(dirmove2)
return copy1, movewithdir1, diverge, renamedelete1, dirmove1
def _dir_renames(repo, c1, c2, copy, fullcopy, u1, u2):
def _dir_renames(repo, ctx, copy, fullcopy, addedfiles):
"""Finds moved directories and files that should move with them.
ctx: the context for one of the sides
copy: files copied on the same side (as ctx)
fullcopy: files copied on the same side (as ctx), including those that
merge.manifestmerge() won't care about
addedfiles: added files on the other side (compared to ctx)
"""
# generate a directory move map # generate a directory move map
d1, d2 = c1.dirs(), c2.dirs() d = ctx.dirs()
invalid = set() invalid = set()
dirmove = {} dirmove = {}
# examine each file copy for a potential directory move, which is # examine each file copy for a potential directory move, which is
# when all the files in a directory are moved to a new directory # when all the files in a directory are moved to a new directory
for dst, src in pycompat.iteritems(fullcopy): for dst, src in pycompat.iteritems(fullcopy):
dsrc, ddst = pathutil.dirname(src), pathutil.dirname(dst) dsrc, ddst = pathutil.dirname(src), pathutil.dirname(dst)
if dsrc in invalid: if dsrc in invalid:
# already seen to be uninteresting # already seen to be uninteresting
continue continue
elif dsrc in d1 and ddst in d1: elif dsrc in d and ddst in d:
# directory wasn't entirely moved locally # directory wasn't entirely moved locally
invalid.add(dsrc) invalid.add(dsrc)
elif dsrc in d2 and ddst in d2:
# directory wasn't entirely moved remotely
invalid.add(dsrc)
elif dsrc in dirmove and dirmove[dsrc] != ddst: elif dsrc in dirmove and dirmove[dsrc] != ddst:
# files from the same directory moved to two different places # files from the same directory moved to two different places
invalid.add(dsrc) invalid.add(dsrc)
else: else:
# looks good so far # looks good so far
dirmove[dsrc] = ddst dirmove[dsrc] = ddst
for i in invalid: for i in invalid:
if i in dirmove: if i in dirmove:
del dirmove[i] del dirmove[i]
del d1, d2, invalid del d, invalid
if not dirmove: if not dirmove:
return {}, {} return {}, {}
dirmove = {k + b"/": v + b"/" for k, v in pycompat.iteritems(dirmove)} dirmove = {k + b"/": v + b"/" for k, v in pycompat.iteritems(dirmove)}
for d in dirmove: for d in dirmove:
repo.ui.debug( repo.ui.debug(
b" discovered dir src: '%s' -> dst: '%s'\n" % (d, dirmove[d]) b" discovered dir src: '%s' -> dst: '%s'\n" % (d, dirmove[d])
) )
movewithdir = {} movewithdir = {}
# check unaccounted nonoverlapping files against directory moves # check unaccounted nonoverlapping files against directory moves
for f in u1 + u2: for f in addedfiles:
if f not in fullcopy: if f not in fullcopy:
for d in dirmove: for d in dirmove:
if f.startswith(d): if f.startswith(d):
# new file added in a directory that was moved, move it # new file added in a directory that was moved, move it
df = dirmove[d] + f[len(d) :] df = dirmove[d] + f[len(d) :]
if df not in copy: if df not in copy:
movewithdir[f] = df movewithdir[f] = df
repo.ui.debug( repo.ui.debug(