This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8887

mergeresult: add `files()` and use it
ClosedPublic

Authored by pulkit on Aug 5 2020, 8:11 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGe5b4061f32be: mergeresult: add `files()` and use it
Summary

files() will return a list of files on which an action needs to be performed.

This is a step to stop exposing the underlying map to the user of this object.

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

pulkit created this revision.Aug 5 2020, 8:11 AM
pulkit updated this revision to Diff 22308.Aug 6 2020, 7:16 AM
indygreg accepted this revision.Aug 8 2020, 12:33 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/merge.py
1327

A generator is passed in here. I suspect this is fine. But just wanted to call it out since most other consumers of files() consume the generator immediately.

This revision is now accepted and ready to land.Aug 8 2020, 12:33 PM
indygreg added a comment.Aug 8 2020, 12:51 PM

FWIW I encountered the following test failure when running this series locally:

--- /home/gps/src/hg-committed/tests/test-remotefilelog-prefetch.t
+++ /home/gps/src/hg-committed/tests/test-remotefilelog-prefetch.t.err
@@ -241,7 +241,6 @@
   ? files fetched over 1 fetches - (? misses, 0.00% hit ratio) over * (glob)
   abort: z2@109c3a557a73: not found in manifest! (?)
   $ find $CACHEDIR -type f | sort
-  $TESTTMP/hgcache/master/11/f6ad8ec52a2984abaafd7c3b516503785c2072/ef95c5376f34698742fe34f315fd82136f8f68c0
   $TESTTMP/hgcache/master/39/5df8f7c51f007019cb30201c49e884b46b92fa/69a1b67522704ec122181c0890bd16e9d3e7516a
   $TESTTMP/hgcache/master/95/cb0bfd2977c761298d9624e4b4d4c72a39974a/076f5e2225b3ff0400b98c92aa6cdf403ee24cca
   $TESTTMP/hgcache/repos

But the failure wasn't reproducible when running the test in isolation. It might be a race condition somewhere. Possibly due to prefetch() now taking a generator after this patch.

pulkit added a commit: rHGe5b4061f32be: mergeresult: add `files()` and use it.Aug 8 2020, 12:56 PM
Closed by commit rHGe5b4061f32be: mergeresult: add `files()` and use it (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/largefiles/overrides.py (2 lines)
Mmercurial/merge.py (56 lines)

Diff 22345

hgext/largefiles/overrides.py

repo, p1, p2, pas, branchmerge, force, acceptremote, *args, **kwargs repo, p1, p2, pas, branchmerge, force, acceptremote, *args, **kwargs
) )
if overwrite: if overwrite:
return mresult return mresult
# Convert to dictionary with filename as key and action as value. # Convert to dictionary with filename as key and action as value.
lfiles = set() lfiles = set()
for f in mresult.actions: for f in mresult.files():
splitstandin = lfutil.splitstandin(f) splitstandin = lfutil.splitstandin(f)
if splitstandin is not None and splitstandin in p1: if splitstandin is not None and splitstandin in p1:
lfiles.add(splitstandin) lfiles.add(splitstandin)
elif lfutil.standin(f) in p1: elif lfutil.standin(f) in p1:
lfiles.add(f) lfiles.add(f)
for lfile in sorted(lfiles): for lfile in sorted(lfiles):
standin = lfutil.standin(lfile) standin = lfutil.standin(lfile)

mercurial/merge.py

def collectconflicts(conflicts, config): def collectconflicts(conflicts, config):
if config == b'abort': if config == b'abort':
abortconflicts.update(conflicts) abortconflicts.update(conflicts)
elif config == b'warn': elif config == b'warn':
warnconflicts.update(conflicts) warnconflicts.update(conflicts)
checkunknowndirs = _unknowndirschecker() checkunknowndirs = _unknowndirschecker()
for f, args, msg in mresult.getactions( for f in mresult.files(
[ (
mergestatemod.ACTION_CREATED, mergestatemod.ACTION_CREATED,
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
] )
): ):
if _checkunknownfile(repo, wctx, mctx, f): if _checkunknownfile(repo, wctx, mctx, f):
fileconflicts.add(f) fileconflicts.add(f)
elif pathconfig and f not in wctx: elif pathconfig and f not in wctx:
path = checkunknowndirs(repo, wctx, f) path = checkunknowndirs(repo, wctx, f)
if path is not None: if path is not None:
pathconflicts.add(path) pathconflicts.add(path)
for f, args, msg in mresult.getactions( for f, args, msg in mresult.getactions(
""" """
Check for case-folding collisions. Check for case-folding collisions.
""" """
# If the repo is narrowed, filter out files outside the narrowspec. # If the repo is narrowed, filter out files outside the narrowspec.
narrowmatch = repo.narrowmatch() narrowmatch = repo.narrowmatch()
if not narrowmatch.always(): if not narrowmatch.always():
pmmf = set(wmf.walk(narrowmatch)) pmmf = set(wmf.walk(narrowmatch))
if mresult: if mresult:
for f, actionsfortype in pycompat.iteritems(mresult.actions): for f in list(mresult.files()):
if not narrowmatch(f): if not narrowmatch(f):
mresult.removefile(f) mresult.removefile(f)
else: else:
# build provisional merged manifest up # build provisional merged manifest up
pmmf = set(wmf) pmmf = set(wmf)
if mresult: if mresult:
# KEEP and EXEC are no-op # KEEP and EXEC are no-op
for f, args, msg in mresult.getactions( for f in mresult.files(
( (
mergestatemod.ACTION_ADD, mergestatemod.ACTION_ADD,
mergestatemod.ACTION_ADD_MODIFIED, mergestatemod.ACTION_ADD_MODIFIED,
mergestatemod.ACTION_FORGET, mergestatemod.ACTION_FORGET,
mergestatemod.ACTION_GET, mergestatemod.ACTION_GET,
mergestatemod.ACTION_CHANGED_DELETED, mergestatemod.ACTION_CHANGED_DELETED,
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
) )
): ):
pmmf.add(f) pmmf.add(f)
for f, args, msg in mresult.getactions([mergestatemod.ACTION_REMOVE]): for f in mresult.files((mergestatemod.ACTION_REMOVE,)):
pmmf.discard(f) pmmf.discard(f)
for f, args, msg in mresult.getactions( for f, args, msg in mresult.getactions(
[mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL] [mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL]
): ):
f2, flags = args f2, flags = args
pmmf.discard(f2) pmmf.discard(f2)
pmmf.add(f) pmmf.add(f)
for f, args, msg in mresult.getactions( for f in mresult.files((mergestatemod.ACTION_LOCAL_DIR_RENAME_GET,)):
[mergestatemod.ACTION_LOCAL_DIR_RENAME_GET]
):
pmmf.add(f) pmmf.add(f)
for f, args, msg in mresult.getactions([mergestatemod.ACTION_MERGE]): for f, args, msg in mresult.getactions([mergestatemod.ACTION_MERGE]):
f1, f2, fa, move, anc = args f1, f2, fa, move, anc = args
if move: if move:
pmmf.discard(f1) pmmf.discard(f1)
pmmf.add(f) pmmf.add(f)
# check case-folding collision in provisional merged manifest # check case-folding collision in provisional merged manifest
invalidconflicts = set() invalidconflicts = set()
# The set of directories that contain files that are being created. # The set of directories that contain files that are being created.
createdfiledirs = set() createdfiledirs = set()
# The set of files deleted by all the actions. # The set of files deleted by all the actions.
deletedfiles = set() deletedfiles = set()
for (f, args, msg) in mresult.getactions( for f in mresult.files(
( (
mergestatemod.ACTION_CREATED, mergestatemod.ACTION_CREATED,
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_MERGE, mergestatemod.ACTION_MERGE,
mergestatemod.ACTION_CREATED_MERGE, mergestatemod.ACTION_CREATED_MERGE,
) )
): ):
# This action may create a new local file. # This action may create a new local file.
createdfiledirs.update(pathutil.finddirs(f)) createdfiledirs.update(pathutil.finddirs(f))
if mf.hasdir(f): if mf.hasdir(f):
# The file aliases a local directory. This might be ok if all # The file aliases a local directory. This might be ok if all
# the files in the local directory are being deleted. This # the files in the local directory are being deleted. This
# will be checked once we know what all the deleted files are. # will be checked once we know what all the deleted files are.
remoteconflicts.add(f) remoteconflicts.add(f)
# Track the names of all deleted files. # Track the names of all deleted files.
for (f, args, msg) in mresult.getactions((mergestatemod.ACTION_REMOVE,)): for f in mresult.files((mergestatemod.ACTION_REMOVE,)):
deletedfiles.add(f) deletedfiles.add(f)
for (f, args, msg) in mresult.getactions((mergestatemod.ACTION_MERGE,)): for (f, args, msg) in mresult.getactions((mergestatemod.ACTION_MERGE,)):
f1, f2, fa, move, anc = args f1, f2, fa, move, anc = args
if move: if move:
deletedfiles.add(f1) deletedfiles.add(f1)
for (f, args, msg) in mresult.getactions( for (f, args, msg) in mresult.getactions(
(mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,) (mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,)
): ):
# This is an internal inconsistency within the remote # This is an internal inconsistency within the remote
# manifest. # manifest.
invalidconflicts.add(p) invalidconflicts.add(p)
# Rename all local conflicting files that have not been deleted. # Rename all local conflicting files that have not been deleted.
for p in localconflicts: for p in localconflicts:
if p not in deletedfiles: if p not in deletedfiles:
ctxname = bytes(wctx).rstrip(b'+') ctxname = bytes(wctx).rstrip(b'+')
pnew = util.safename(p, ctxname, wctx, set(mresult.actions.keys())) pnew = util.safename(p, ctxname, wctx, set(mresult.files()))
porig = wctx[p].copysource() or p porig = wctx[p].copysource() or p
mresult.addfile( mresult.addfile(
pnew, pnew,
mergestatemod.ACTION_PATH_CONFLICT_RESOLVE, mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,
(p, porig), (p, porig),
b'local path conflict', b'local path conflict',
) )
mresult.addfile( mresult.addfile(
p, p,
mergestatemod.ACTION_PATH_CONFLICT, mergestatemod.ACTION_PATH_CONFLICT,
(pnew, b'l'), (pnew, b'l'),
b'path conflict', b'path conflict',
) )
if remoteconflicts: if remoteconflicts:
# Check if all files in the conflicting directories have been removed. # Check if all files in the conflicting directories have been removed.
ctxname = bytes(mctx).rstrip(b'+') ctxname = bytes(mctx).rstrip(b'+')
for f, p in _filesindirs(repo, mf, remoteconflicts): for f, p in _filesindirs(repo, mf, remoteconflicts):
if f not in deletedfiles: if f not in deletedfiles:
m, args, msg = mresult.getfile(p) m, args, msg = mresult.getfile(p)
pnew = util.safename( pnew = util.safename(p, ctxname, wctx, set(mresult.files()))
p, ctxname, wctx, set(mresult.actions.keys())
)
if m in ( if m in (
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_MERGE, mergestatemod.ACTION_MERGE,
): ):
# Action was merge, just update target. # Action was merge, just update target.
mresult.addfile(pnew, m, args, msg) mresult.addfile(pnew, m, args, msg)
else: else:
# Action was create, change to renamed get action. # Action was create, change to renamed get action.
def getfile(self, filename, default_return=None): def getfile(self, filename, default_return=None):
""" returns (action, args, msg) about this file """ returns (action, args, msg) about this file
returns default_return if the file is not present """ returns default_return if the file is not present """
if filename in self._filemapping: if filename in self._filemapping:
return self._filemapping[filename] return self._filemapping[filename]
return default_return return default_return
def files(self, actions=None):
""" returns files on which provided action needs to perfromed
If actions is None, all files are returned
"""
# TODO: think whether we should return renamedelete and
# diverge filenames also
if actions is None:
for f in self._filemapping:
yield f
else:
for a in actions:
for f in self._actionmapping[a]:
yield f
def removefile(self, filename): def removefile(self, filename):
""" removes a file from the mergeresult object as the file might """ removes a file from the mergeresult object as the file might
not merging anymore """ not merging anymore """
action, data, message = self._filemapping[filename] action, data, message = self._filemapping[filename]
del self._filemapping[filename] del self._filemapping[filename]
del self._actionmapping[action][filename] del self._actionmapping[action][filename]
def getactions(self, actions, sort=False): def getactions(self, actions, sort=False):
return mresult return mresult
def _resolvetrivial(repo, wctx, mctx, ancestor, mresult): def _resolvetrivial(repo, wctx, mctx, ancestor, mresult):
"""Resolves false conflicts where the nodeid changed but the content """Resolves false conflicts where the nodeid changed but the content
remained the same.""" remained the same."""
# We force a copy of actions.items() because we're going to mutate # We force a copy of actions.items() because we're going to mutate
# actions as we resolve trivial conflicts. # actions as we resolve trivial conflicts.
for f, args, msg in list( for f in list(mresult.files((mergestatemod.ACTION_CHANGED_DELETED,))):
mresult.getactions([mergestatemod.ACTION_CHANGED_DELETED])
):
if f in ancestor and not wctx[f].cmp(ancestor[f]): if f in ancestor and not wctx[f].cmp(ancestor[f]):
# local did change but ended up with same content # local did change but ended up with same content
mresult.addfile( mresult.addfile(
f, mergestatemod.ACTION_REMOVE, None, b'prompt same' f, mergestatemod.ACTION_REMOVE, None, b'prompt same'
) )
for f, args, msg in list( for f in list(mresult.files((mergestatemod.ACTION_DELETED_CHANGED,))):
mresult.getactions([mergestatemod.ACTION_DELETED_CHANGED])
):
if f in ancestor and not mctx[f].cmp(ancestor[f]): if f in ancestor and not mctx[f].cmp(ancestor[f]):
# remote did change but ended up with same content # remote did change but ended up with same content
mresult.removefile(f) # don't get = keep local deleted mresult.removefile(f) # don't get = keep local deleted
def calculateupdates( def calculateupdates(
repo, repo,
wctx, wctx,
def _prefetchfiles(repo, ctx, mresult): def _prefetchfiles(repo, ctx, mresult):
"""Invoke ``scmutil.prefetchfiles()`` for the files relevant to the dict """Invoke ``scmutil.prefetchfiles()`` for the files relevant to the dict
of merge actions. ``ctx`` is the context being merged in.""" of merge actions. ``ctx`` is the context being merged in."""
# Skipping 'a', 'am', 'f', 'r', 'dm', 'e', 'k', 'p' and 'pr', because they # Skipping 'a', 'am', 'f', 'r', 'dm', 'e', 'k', 'p' and 'pr', because they
# don't touch the context to be merged in. 'cd' is skipped, because # don't touch the context to be merged in. 'cd' is skipped, because
# changed/deleted never resolves to something from the remote side. # changed/deleted never resolves to something from the remote side.
files = [] files = mresult.files(
for f, args, msg in mresult.getactions(
[ [
mergestatemod.ACTION_GET, mergestatemod.ACTION_GET,
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_LOCAL_DIR_RENAME_GET, mergestatemod.ACTION_LOCAL_DIR_RENAME_GET,
mergestatemod.ACTION_MERGE, mergestatemod.ACTION_MERGE,
] ]
): )
files.append(f)
prefetch = scmutil.prefetchfiles prefetch = scmutil.prefetchfiles
matchfiles = scmutil.matchfiles matchfiles = scmutil.matchfiles
prefetch( prefetch(
repo, [(ctx.rev(), matchfiles(repo, files),)], repo, [(ctx.rev(), matchfiles(repo, files),)],
indygregUnsubmitted
Not Done

A generator is passed in here. I suspect this is fine. But just wanted to call it out since most other consumers of files() consume the generator immediately.

indygreg: A generator is passed in here. I suspect this is fine. But just wanted to call it out since…
) )
@attr.s(frozen=True) @attr.s(frozen=True)
class updateresult(object): class updateresult(object):
updatedcount = attr.ib() updatedcount = attr.ib()
mergedcount = attr.ib() mergedcount = attr.ib()
removedcount = attr.ib() removedcount = attr.ib()