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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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 22308

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()