This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1658

memfilectx: make changectx argument mandatory in constructor (API)
ClosedPublic

Authored by martinvonz on Dec 11 2017, 12:47 PM.

Details

Reviewers
durin42
yuja
Group Reviewers
hg-reviewers
Commits
rHG8a0cac20a1ad: memfilectx: make changectx argument mandatory in constructor (API)
rHG79392b35e947: memfilectx: make changectx argument mandatory in constructor (API)
Summary

committablefilectx has three subclasses: workingfilectx, memfilectx,
and overlayfilectx. committablefilectx takes an optional (change) ctx
instance to its constructor. If it's provided, it's set on the
instance as self._changectx. If not, that property is supposed to be
defined by the class. However, only workingfilectx does that. The
other two will have the property undefined if it's not passed in the
constructor. That seems bad to me. This patch makes the changectx
argument to the memfilectx constructor mandatory because that fixes
the failure I ran into. It seems like we should also fix the
overlayfilectx case.

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.Dec 11 2017, 12:47 PM
martinvonz updated this revision to Diff 4358.Dec 11 2017, 12:56 PM
durin42 accepted this revision.Dec 11 2017, 1:14 PM
This revision is now accepted and ready to land.Dec 11 2017, 1:14 PM
durin42 requested changes to this revision.Dec 11 2017, 1:35 PM

This seems to break test-rebase-inmemory.t - could you take a look?

This revision now requires changes to proceed.Dec 11 2017, 1:35 PM
martinvonz updated this revision to Diff 4365.Dec 11 2017, 3:33 PM
martinvonz added a comment.Dec 11 2017, 3:35 PM
In D1658#28406, @durin42 wrote:

This seems to break test-rebase-inmemory.t - could you take a look?

Sorry, I was lazy and didn't run the tests after the last rebase. Phil's new test-rebase-inmemory.t caught a bug (thanks!). I also found another bug in synthrepo that I have now fixed.

yuja added a subscriber: yuja.Dec 12 2017, 8:36 AM

Looks good, but can you flag this as (API) change?

martinvonz updated this revision to Diff 4381.Dec 12 2017, 2:34 PM
martinvonz retitled this revision from memfilectx: make changectx argument mandatory in constructor to memfilectx: make changectx argument mandatory in constructor (API).Dec 12 2017, 2:39 PM
martinvonz added a comment.Dec 12 2017, 2:39 PM
In D1658#28514, @yuja wrote:

Looks good, but can you flag this as (API) change?

Good point. Done.

yuja accepted this revision.Dec 13 2017, 7:36 AM
Closed by commit rHG79392b35e947: memfilectx: make changectx argument mandatory in constructor (API) (authored by martinvonz). · Explain WhyDec 13 2017, 8:05 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 4418

contrib/synthrepo.py

data = '%s contents\n' % path data = '%s contents\n' % path
files[path] = data files[path] = data
dir = os.path.dirname(path) dir = os.path.dirname(path)
while dir and dir not in dirs: while dir and dir not in dirs:
dirs.add(dir) dirs.add(dir)
dir = os.path.dirname(dir) dir = os.path.dirname(dir)
def filectxfn(repo, memctx, path): def filectxfn(repo, memctx, path):
return context.memfilectx(repo, path, files[path]) return context.memfilectx(repo, memctx, path, files[path])
ui.progress(_synthesizing, None) ui.progress(_synthesizing, None)
message = 'synthesized wide repo with %d files' % (len(files),) message = 'synthesized wide repo with %d files' % (len(files),)
mc = context.memctx(repo, [pctx.node(), nullid], message, mc = context.memctx(repo, [pctx.node(), nullid], message,
files.iterkeys(), filectxfn, ui.username(), files.iterkeys(), filectxfn, ui.username(),
'%d %d' % util.makedate()) '%d %d' % util.makedate())
initnode = mc.commit() initnode = mc.commit()
if ui.debugflag: if ui.debugflag:
path.append(random.choice(words)) path.append(random.choice(words))
pathstr = '/'.join(filter(None, path)) pathstr = '/'.join(filter(None, path))
data = '\n'.join(makeline() data = '\n'.join(makeline()
for __ in xrange(pick(linesinfilesadded))) + '\n' for __ in xrange(pick(linesinfilesadded))) + '\n'
changes[pathstr] = data changes[pathstr] = data
def filectxfn(repo, memctx, path): def filectxfn(repo, memctx, path):
if path not in changes: if path not in changes:
return None return None
return context.memfilectx(repo, path, changes[path]) return context.memfilectx(repo, memctx, path, changes[path])
if not changes: if not changes:
continue continue
if revs: if revs:
date = repo['tip'].date()[0] + pick(interarrival) date = repo['tip'].date()[0] + pick(interarrival)
else: else:
date = time.time() - (86400 * count) date = time.time() - (86400 * count)
# dates in mercurial must be positive, fit in 32-bit signed integers. # dates in mercurial must be positive, fit in 32-bit signed integers.
date = min(0x7fffffff, max(0, date)) date = min(0x7fffffff, max(0, date))

hgext/convert/hg.py

return None return None
data, mode = source.getfile(f, v) data, mode = source.getfile(f, v)
if data is None: if data is None:
return None return None
if f == '.hgtags': if f == '.hgtags':
data = self._rewritetags(source, revmap, data) data = self._rewritetags(source, revmap, data)
if f == '.hgsubstate': if f == '.hgsubstate':
data = self._rewritesubstate(source, data) data = self._rewritesubstate(source, data)
return context.memfilectx(self.repo, f, data, 'l' in mode, return context.memfilectx(self.repo, memctx, f, data, 'l' in mode,
'x' in mode, copies.get(f)) 'x' in mode, copies.get(f))
pl = [] pl = []
for p in parents: for p in parents:
if p not in pl: if p not in pl:
pl.append(p) pl.append(p)
parents = pl parents = pl
nparents = len(parents) nparents = len(parents)
if s[1] not in oldtags: if s[1] not in oldtags:
newtags.add(s[1].strip()) newtags.add(s[1].strip())
if not newtags: if not newtags:
return None, None return None, None
data = "".join(newlines) data = "".join(newlines)
def getfilectx(repo, memctx, f): def getfilectx(repo, memctx, f):
return context.memfilectx(repo, f, data, False, False, None) return context.memfilectx(repo, memctx, f, data, False, False, None)
self.ui.status(_("updating tags\n")) self.ui.status(_("updating tags\n"))
date = "%s 0" % int(time.mktime(time.gmtime())) date = "%s 0" % int(time.mktime(time.gmtime()))
extra = {'branch': self.tagsbranch} extra = {'branch': self.tagsbranch}
ctx = context.memctx(self.repo, (tagparent, None), "update tags", ctx = context.memctx(self.repo, (tagparent, None), "update tags",
[".hgtags"], getfilectx, "convert-repo", date, [".hgtags"], getfilectx, "convert-repo", date,
extra) extra)
node = self.repo.commitctx(ctx) node = self.repo.commitctx(ctx)

hgext/histedit.py

# prune files which were reverted by the updates # prune files which were reverted by the updates
files = [f for f in files if not cmdutil.samefile(f, last, base)] files = [f for f in files if not cmdutil.samefile(f, last, base)]
# commit version of these files as defined by head # commit version of these files as defined by head
headmf = last.manifest() headmf = last.manifest()
def filectxfn(repo, ctx, path): def filectxfn(repo, ctx, path):
if path in headmf: if path in headmf:
fctx = last[path] fctx = last[path]
flags = fctx.flags() flags = fctx.flags()
mctx = context.memfilectx(repo, mctx = context.memfilectx(repo, ctx,
fctx.path(), fctx.data(), fctx.path(), fctx.data(),
islink='l' in flags, islink='l' in flags,
isexec='x' in flags, isexec='x' in flags,
copied=copied.get(path)) copied=copied.get(path))
return mctx return mctx
return None return None
if commitopts.get('message'): if commitopts.get('message'):

hgext/largefiles/lfcommands.py

except error.LookupError: except error.LookupError:
return None return None
renamed = fctx.renamed() renamed = fctx.renamed()
if renamed: if renamed:
# standin is always a largefile because largefile-ness # standin is always a largefile because largefile-ness
# doesn't change after rename or copy # doesn't change after rename or copy
renamed = lfutil.standin(renamed[0]) renamed = lfutil.standin(renamed[0])
return context.memfilectx(repo, f, lfiletohash[srcfname] + '\n', return context.memfilectx(repo, memctx, f,
lfiletohash[srcfname] + '\n',
'l' in fctx.flags(), 'x' in fctx.flags(), 'l' in fctx.flags(), 'x' in fctx.flags(),
renamed) renamed)
else: else:
return _getnormalcontext(repo, ctx, f, revmap) return _getnormalcontext(repo, ctx, f, revmap)
# Commit # Commit
_commitcontext(rdst, parents, ctx, dstfiles, getfilectx, revmap) _commitcontext(rdst, parents, ctx, dstfiles, getfilectx, revmap)
return None return None
renamed = fctx.renamed() renamed = fctx.renamed()
if renamed: if renamed:
renamed = renamed[0] renamed = renamed[0]
data = fctx.data() data = fctx.data()
if f == '.hgtags': if f == '.hgtags':
data = _converttags (repo.ui, revmap, data) data = _converttags (repo.ui, revmap, data)
return context.memfilectx(repo, f, data, 'l' in fctx.flags(), return context.memfilectx(repo, ctx, f, data, 'l' in fctx.flags(),
'x' in fctx.flags(), renamed) 'x' in fctx.flags(), renamed)
# Remap tag data using a revision map # Remap tag data using a revision map
def _converttags(ui, revmap, data): def _converttags(ui, revmap, data):
newdata = [] newdata = []
for line in data.splitlines(): for line in data.splitlines():
try: try:
id, name = line.split(' ', 1) id, name = line.split(' ', 1)

hgext/uncommit.py

# Filter copies # Filter copies
copied = copies.pathcopies(base, ctx) copied = copies.pathcopies(base, ctx)
copied = dict((dst, src) for dst, src in copied.iteritems() copied = dict((dst, src) for dst, src in copied.iteritems()
if dst in files) if dst in files)
def filectxfn(repo, memctx, path, contentctx=ctx, redirect=()): def filectxfn(repo, memctx, path, contentctx=ctx, redirect=()):
if path not in contentctx: if path not in contentctx:
return None return None
fctx = contentctx[path] fctx = contentctx[path]
mctx = context.memfilectx(repo, fctx.path(), fctx.data(), mctx = context.memfilectx(repo, memctx, fctx.path(), fctx.data(),
fctx.islink(), fctx.islink(),
fctx.isexec(), fctx.isexec(),
copied=copied.get(path)) copied=copied.get(path))
return mctx return mctx
new = context.memctx(repo, new = context.memctx(repo,
parents=[base.node(), node.nullid], parents=[base.node(), node.nullid],
text=ctx.description(), text=ctx.description(),

mercurial/cmdutil.py

return old.filectx(path) return old.filectx(path)
# Return None for removed files. # Return None for removed files.
if path in wctx.removed(): if path in wctx.removed():
return None return None
fctx = wctx[path] fctx = wctx[path]
flags = fctx.flags() flags = fctx.flags()
mctx = context.memfilectx(repo, mctx = context.memfilectx(repo, ctx_,
fctx.path(), fctx.data(), fctx.path(), fctx.data(),
islink='l' in flags, islink='l' in flags,
isexec='x' in flags, isexec='x' in flags,
copied=copied.get(path)) copied=copied.get(path))
return mctx return mctx
except KeyError: except KeyError:
return None return None
else: else:

mercurial/context.py

if parents[1] is None: if parents[1] is None:
parents = (self._repo[parents[0]], None) parents = (self._repo[parents[0]], None)
else: else:
parents = (self._repo[parents[0]], self._repo[parents[1]]) parents = (self._repo[parents[0]], self._repo[parents[1]])
files = self._cache.keys() files = self._cache.keys()
def getfile(repo, memctx, path): def getfile(repo, memctx, path):
if self._cache[path]['exists']: if self._cache[path]['exists']:
return memfilectx(repo, path, return memfilectx(repo, memctx, path,
self._cache[path]['data'], self._cache[path]['data'],
'l' in self._cache[path]['flags'], 'l' in self._cache[path]['flags'],
'x' in self._cache[path]['flags'], 'x' in self._cache[path]['flags'],
self._cache[path]['copied'], self._cache[path]['copied'])
memctx)
else: else:
# Returning None, but including the path in `files`, is # Returning None, but including the path in `files`, is
# necessary for memctx to register a deletion. # necessary for memctx to register a deletion.
return None return None
return memctx(self._repo, parents, text, files, getfile, date=date, return memctx(self._repo, parents, text, files, getfile, date=date,
extra=extra, user=user, branch=branch, editor=editor) extra=extra, user=user, branch=branch, editor=editor)
def isdirty(self, path): def isdirty(self, path):
""" """
def getfilectx(repo, memctx, path): def getfilectx(repo, memctx, path):
fctx = ctx[path] fctx = ctx[path]
# this is weird but apparently we only keep track of one parent # this is weird but apparently we only keep track of one parent
# (why not only store that instead of a tuple?) # (why not only store that instead of a tuple?)
copied = fctx.renamed() copied = fctx.renamed()
if copied: if copied:
copied = copied[0] copied = copied[0]
return memfilectx(repo, path, fctx.data(), return memfilectx(repo, memctx, path, fctx.data(),
islink=fctx.islink(), isexec=fctx.isexec(), islink=fctx.islink(), isexec=fctx.isexec(),
copied=copied, memctx=memctx) copied=copied)
return getfilectx return getfilectx
def memfilefrompatch(patchstore): def memfilefrompatch(patchstore):
"""Given a patch (e.g. patchstore object) return a memfilectx """Given a patch (e.g. patchstore object) return a memfilectx
This is a convenience method for building a memctx based on a patchstore. This is a convenience method for building a memctx based on a patchstore.
""" """
def getfilectx(repo, memctx, path): def getfilectx(repo, memctx, path):
data, mode, copied = patchstore.getfile(path) data, mode, copied = patchstore.getfile(path)
if data is None: if data is None:
return None return None
islink, isexec = mode islink, isexec = mode
return memfilectx(repo, path, data, islink=islink, return memfilectx(repo, memctx, path, data, islink=islink,
isexec=isexec, copied=copied, isexec=isexec, copied=copied)
memctx=memctx)
return getfilectx return getfilectx
class memctx(committablectx): class memctx(committablectx):
"""Use memctx to perform in-memory commits via localrepo.commitctx(). """Use memctx to perform in-memory commits via localrepo.commitctx().
Revision information is supplied at initialization time while Revision information is supplied at initialization time while
related files data and is made available through a callback related files data and is made available through a callback
return scmutil.status(modified, added, removed, [], [], [], []) return scmutil.status(modified, added, removed, [], [], [], [])
class memfilectx(committablefilectx): class memfilectx(committablefilectx):
"""memfilectx represents an in-memory file to commit. """memfilectx represents an in-memory file to commit.
See memctx and committablefilectx for more details. See memctx and committablefilectx for more details.
""" """
def __init__(self, repo, path, data, islink=False, def __init__(self, repo, changectx, path, data, islink=False,
isexec=False, copied=None, memctx=None): isexec=False, copied=None):
""" """
path is the normalized file path relative to repository root. path is the normalized file path relative to repository root.
data is the file content as a string. data is the file content as a string.
islink is True if the file is a symbolic link. islink is True if the file is a symbolic link.
isexec is True if the file is executable. isexec is True if the file is executable.
copied is the source file path if current file was copied in the copied is the source file path if current file was copied in the
revision being committed, or None.""" revision being committed, or None."""
super(memfilectx, self).__init__(repo, path, None, memctx) super(memfilectx, self).__init__(repo, path, None, changectx)
self._data = data self._data = data
self._flags = (islink and 'l' or '') + (isexec and 'x' or '') self._flags = (islink and 'l' or '') + (isexec and 'x' or '')
self._copied = None self._copied = None
if copied: if copied:
self._copied = (copied, nullid) self._copied = (copied, nullid)
def data(self): def data(self):
return self._data return self._data

mercurial/debugcommands.py

p2 = repo[ps[1]] p2 = repo[ps[1]]
for fn in p2: for fn in p2:
if fn.startswith("nf"): if fn.startswith("nf"):
files.append(fn) files.append(fn)
filecontent[fn] = p2[fn].data() filecontent[fn] = p2[fn].data()
def fctxfn(repo, cx, path): def fctxfn(repo, cx, path):
if path in filecontent: if path in filecontent:
return context.memfilectx(repo, path, filecontent[path]) return context.memfilectx(repo, cx, path,
filecontent[path])
return None return None
if len(ps) == 0 or ps[0] < 0: if len(ps) == 0 or ps[0] < 0:
pars = [None, None] pars = [None, None]
elif len(ps) == 1: elif len(ps) == 1:
pars = [nodeids[ps[0]], None] pars = [nodeids[ps[0]], None]
else: else:
pars = [nodeids[p] for p in ps] pars = [nodeids[p] for p in ps]

tests/test-commit.t

verify pathauditor blocks evil filepaths verify pathauditor blocks evil filepaths
$ cat > evil-commit.py <<EOF $ cat > evil-commit.py <<EOF
> from __future__ import absolute_import > from __future__ import absolute_import
> from mercurial import context, hg, node, ui as uimod > from mercurial import context, hg, node, ui as uimod
> notrc = u".h\u200cg".encode('utf-8') + '/hgrc' > notrc = u".h\u200cg".encode('utf-8') + '/hgrc'
> u = uimod.ui.load() > u = uimod.ui.load()
> r = hg.repository(u, '.') > r = hg.repository(u, '.')
> def filectxfn(repo, memctx, path): > def filectxfn(repo, memctx, path):
> return context.memfilectx(repo, path, '[hooks]\nupdate = echo owned') > return context.memfilectx(repo, memctx, path,
> '[hooks]\nupdate = echo owned')
> c = context.memctx(r, [r['tip'].node(), node.nullid], > c = context.memctx(r, [r['tip'].node(), node.nullid],
> 'evil', [notrc], filectxfn, 0) > 'evil', [notrc], filectxfn, 0)
> r.commitctx(c) > r.commitctx(c)
> EOF > EOF
$ $PYTHON evil-commit.py $ $PYTHON evil-commit.py
#if windows #if windows
$ hg co --clean tip $ hg co --clean tip
abort: path contains illegal component: .h\xe2\x80\x8cg\\hgrc (esc) abort: path contains illegal component: .h\xe2\x80\x8cg\\hgrc (esc)
[255] [255]
#else #else
$ hg co --clean tip $ hg co --clean tip
abort: path contains illegal component: .h\xe2\x80\x8cg/hgrc (esc) abort: path contains illegal component: .h\xe2\x80\x8cg/hgrc (esc)
[255] [255]
#endif #endif
$ hg rollback -f $ hg rollback -f
repository tip rolled back to revision 2 (undo commit) repository tip rolled back to revision 2 (undo commit)
$ cat > evil-commit.py <<EOF $ cat > evil-commit.py <<EOF
> from __future__ import absolute_import > from __future__ import absolute_import
> from mercurial import context, hg, node, ui as uimod > from mercurial import context, hg, node, ui as uimod
> notrc = "HG~1/hgrc" > notrc = "HG~1/hgrc"
> u = uimod.ui.load() > u = uimod.ui.load()
> r = hg.repository(u, '.') > r = hg.repository(u, '.')
> def filectxfn(repo, memctx, path): > def filectxfn(repo, memctx, path):
> return context.memfilectx(repo, path, '[hooks]\nupdate = echo owned') > return context.memfilectx(repo, memctx, path,
> '[hooks]\nupdate = echo owned')
> c = context.memctx(r, [r['tip'].node(), node.nullid], > c = context.memctx(r, [r['tip'].node(), node.nullid],
> 'evil', [notrc], filectxfn, 0) > 'evil', [notrc], filectxfn, 0)
> r.commitctx(c) > r.commitctx(c)
> EOF > EOF
$ $PYTHON evil-commit.py $ $PYTHON evil-commit.py
$ hg co --clean tip $ hg co --clean tip
abort: path contains illegal component: HG~1/hgrc abort: path contains illegal component: HG~1/hgrc
[255] [255]
$ hg rollback -f $ hg rollback -f
repository tip rolled back to revision 2 (undo commit) repository tip rolled back to revision 2 (undo commit)
$ cat > evil-commit.py <<EOF $ cat > evil-commit.py <<EOF
> from __future__ import absolute_import > from __future__ import absolute_import
> from mercurial import context, hg, node, ui as uimod > from mercurial import context, hg, node, ui as uimod
> notrc = "HG8B6C~2/hgrc" > notrc = "HG8B6C~2/hgrc"
> u = uimod.ui.load() > u = uimod.ui.load()
> r = hg.repository(u, '.') > r = hg.repository(u, '.')
> def filectxfn(repo, memctx, path): > def filectxfn(repo, memctx, path):
> return context.memfilectx(repo, path, '[hooks]\nupdate = echo owned') > return context.memfilectx(repo, memctx, path,
> '[hooks]\nupdate = echo owned')
> c = context.memctx(r, [r['tip'].node(), node.nullid], > c = context.memctx(r, [r['tip'].node(), node.nullid],
> 'evil', [notrc], filectxfn, 0) > 'evil', [notrc], filectxfn, 0)
> r.commitctx(c) > r.commitctx(c)
> EOF > EOF
$ $PYTHON evil-commit.py $ $PYTHON evil-commit.py
$ hg co --clean tip $ hg co --clean tip
abort: path contains illegal component: HG8B6C~2/hgrc abort: path contains illegal component: HG8B6C~2/hgrc
[255] [255]

tests/test-context.py

d = repo[None]['foo'].date() d = repo[None]['foo'].date()
if os.name == 'nt': if os.name == 'nt':
d = d[:2] d = d[:2]
print("workingfilectx.date = (%d, %d)" % d) print("workingfilectx.date = (%d, %d)" % d)
# test memctx with non-ASCII commit message # test memctx with non-ASCII commit message
def filectxfn(repo, memctx, path): def filectxfn(repo, memctx, path):
return context.memfilectx(repo, "foo", "") return context.memfilectx(repo, memctx, "foo", "")
ctx = context.memctx(repo, ['tip', None], ctx = context.memctx(repo, ['tip', None],
encoding.tolocal("Gr\xc3\xbcezi!"), encoding.tolocal("Gr\xc3\xbcezi!"),
["foo"], filectxfn) ["foo"], filectxfn)
ctx.commit() ctx.commit()
for enc in "ASCII", "Latin-1", "UTF-8": for enc in "ASCII", "Latin-1", "UTF-8":
encoding.encoding = enc encoding.encoding = enc
print("%-8s: %s" % (enc, repo["tip"].description())) print("%-8s: %s" % (enc, repo["tip"].description()))
# test performing a status # test performing a status
def getfilectx(repo, memctx, f): def getfilectx(repo, memctx, f):
fctx = memctx.parents()[0][f] fctx = memctx.parents()[0][f]
data, flags = fctx.data(), fctx.flags() data, flags = fctx.data(), fctx.flags()
if f == 'foo': if f == 'foo':
data += 'bar\n' data += 'bar\n'
return context.memfilectx(repo, f, data, 'l' in flags, 'x' in flags) return context.memfilectx(repo, memctx, f, data, 'l' in flags, 'x' in flags)
ctxa = repo.changectx(0) ctxa = repo.changectx(0)
ctxb = context.memctx(repo, [ctxa.node(), None], "test diff", ["foo"], ctxb = context.memctx(repo, [ctxa.node(), None], "test diff", ["foo"],
getfilectx, ctxa.user(), ctxa.date()) getfilectx, ctxa.user(), ctxa.date())
print(ctxb.status(ctxa)) print(ctxb.status(ctxa))
# test performing a diff on a memctx # test performing a diff on a memctx