This is an archive of the discontinued Mercurial Phabricator instance.

Differential D444

merge: move some of the logic in batchget() to workingfilectx
ClosedPublic

Authored by phillco on Aug 18 2017, 12:45 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG65ae54582713: merge: move some of the logic in batchget() to workingfilectx
Summary

We will use this logic in two places with in-memory merge.

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

phillco created this revision.Aug 18 2017, 12:45 PM
phillco updated this revision to Diff 1226.Aug 23 2017, 3:38 PM
phillco updated this revision to Diff 1309.Aug 25 2017, 5:01 PM
phillco updated this revision to Diff 1459.Aug 30 2017, 5:12 PM
martinvonz added a subscriber: martinvonz.Aug 31 2017, 3:29 AM
martinvonz added inline comments.
mercurial/context.py
1971–1973

self.repo.wvfs gets repeated often enough that it's probably worth extracting to a local variable

1975–1977

I'd prefer to see this addition in the patch that needs it. At this point it's not clear what this is for.

Also, with this code here, it seems a little unnecessary for the block above to delete parent directories if we're just going to create them again here. Am I reading that right?

mercurial/merge.py
1153–1154

nit: wctx[f] involves enough work that it's probably worth extracting it to a variable?

phillco updated this revision to Diff 1478.Aug 31 2017, 1:59 PM
phillco marked 2 inline comments as done.Aug 31 2017, 2:00 PM
martinvonz added inline comments.Aug 31 2017, 3:32 PM
mercurial/context.py
1967–1968

I'll drop the the parenthesis in flight

phillco added inline comments.Aug 31 2017, 4:44 PM
mercurial/context.py
1967–1968

Thanks.

Closed by commit rHG65ae54582713: merge: move some of the logic in batchget() to workingfilectx (authored by phillco). · Explain WhyAug 31 2017, 5:22 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/context.py (8 lines)
Mmercurial/merge.py (4 lines)

Diff 1516

mercurial/context.py

"""wraps unlink for a repo's working directory""" """wraps unlink for a repo's working directory"""
self._repo.wvfs.unlinkpath(self._path, ignoremissing=ignoremissing) self._repo.wvfs.unlinkpath(self._path, ignoremissing=ignoremissing)
def write(self, data, flags, backgroundclose=False): def write(self, data, flags, backgroundclose=False):
"""wraps repo.wwrite""" """wraps repo.wwrite"""
self._repo.wwrite(self._path, data, flags, self._repo.wwrite(self._path, data, flags,
backgroundclose=backgroundclose) backgroundclose=backgroundclose)
def clearunknown(self):
"""Removes conflicting items in the working directory so that
``write()`` can be called successfully.
martinvonzUnsubmitted
Not Done

I'll drop the the parenthesis in flight

martinvonz: I'll drop the the parenthesis in flight
phillcoAuthorUnsubmitted
Not Done

Thanks.

phillco: Thanks.
"""
wvfs = self._repo.wvfs
if wvfs.isdir(self._path) and not wvfs.islink(self._path):
wvfs.removedirs(self._path)
martinvonzUnsubmitted
Done

self.repo.wvfs gets repeated often enough that it's probably worth extracting to a local variable

martinvonz: self.repo.wvfs gets repeated often enough that it's probably worth extracting to a local…
def setflags(self, l, x): def setflags(self, l, x):
self._repo.wvfs.setflags(self._path, l, x) self._repo.wvfs.setflags(self._path, l, x)
class workingcommitctx(workingctx): class workingcommitctx(workingctx):
martinvonzUnsubmitted
Done

I'd prefer to see this addition in the patch that needs it. At this point it's not clear what this is for.

Also, with this code here, it seems a little unnecessary for the block above to delete parent directories if we're just going to create them again here. Am I reading that right?

martinvonz: I'd prefer to see this addition in the patch that needs it. At this point it's not clear what…
"""A workingcommitctx object makes access to data related to """A workingcommitctx object makes access to data related to
the revision being committed convenient. the revision being committed convenient.
This hides changes in the working directory, if they aren't This hides changes in the working directory, if they aren't
committed in this context. committed in this context.
""" """
def __init__(self, repo, changes, def __init__(self, repo, changes,
text="", user=None, date=None, extra=None): text="", user=None, date=None, extra=None):

mercurial/merge.py

absf = repo.wjoin(f) absf = repo.wjoin(f)
orig = scmutil.origpath(ui, repo, absf) orig = scmutil.origpath(ui, repo, absf)
try: try:
if repo.wvfs.isfileorlink(f): if repo.wvfs.isfileorlink(f):
util.rename(absf, orig) util.rename(absf, orig)
except OSError as e: except OSError as e:
if e.errno != errno.ENOENT: if e.errno != errno.ENOENT:
raise raise
wctx[f].clearunknown()
if repo.wvfs.isdir(f) and not repo.wvfs.islink(f):
repo.wvfs.removedirs(f)
wctx[f].write(fctx(f).data(), flags, backgroundclose=True) wctx[f].write(fctx(f).data(), flags, backgroundclose=True)
martinvonzUnsubmitted
Not Done

nit: wctx[f] involves enough work that it's probably worth extracting it to a variable?

martinvonz: nit: wctx[f] involves enough work that it's probably worth extracting it to a variable?
if i == 100: if i == 100:
yield i, f yield i, f
i = 0 i = 0
i += 1 i += 1
if i > 0: if i > 0:
yield i, f yield i, f
def applyupdates(repo, actions, wctx, mctx, overwrite, labels=None): def applyupdates(repo, actions, wctx, mctx, overwrite, labels=None):