This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1527

context: add an abstract base class for filectx
AbandonedPublic

Authored by phillco on Nov 27 2017, 10:35 PM.

Download Raw Diff

Details

Reviewers

martinvonz

Group Reviewers

hg-reviewers

Summary

basectx is very well and good, but we have two other filectx-like classes
that do not inherit from it (arbitraryfilectx and absentfilectx`), and
cannot, because basectx is still rather tied to representing a file in a
changectx.

Let's add some structure to these classes with a mininum set of abstract
methods enforced by the ABC module.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

phillco created this revision.Nov 27 2017, 10:35 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptNov 27 2017, 10:35 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

Note that we can't actually make absentfilectx based off this class today because of the context -> merge imprt cycle.

phillco added a child revision: D1211: context: switch ctx() use to changectx().Dec 1 2017, 3:03 AM

phillco mentioned this in D1211: context: switch ctx() use to changectx().

In D1527#25742, @phillco wrote:

Note that we can't actually make absentfilectx based off this class today because of the context -> merge imprt cycle.

What exactly is that cycle? Can we break one of the links with a local import?

mercurial/context.py
761	For a separate patch: I find it weird to refer to these undefined fields (_repo and _path). Can we pass these into the constructor (which is currently not defined) and assign them there?
2646	Can we put them in abstractfilectx if they seems generic enough? We can still created optimized versions in subclasses.
2673	Maybe in a separate patch, you could make this just "return self.data()"?

phillco updated this revision to Diff 4175.Dec 7 2017, 4:22 PM

What exactly is that cycle? Can we break one of the links with a local import?

It's the same filectx -> ctx -> fileset -> merge -> filemerge cycle that frustrated earlier. We can't circumvent it this time (I think) because the import is in the module, not a function.

phillco planned changes to this revision.Dec 11 2017, 3:28 PM

phillco abandoned this revision.Jan 21 2018, 11:22 PM

Revision Contents
Changeset List

			Path	Packages
M			mercurial/context.py (67 lines)
M			mercurial/filemerge.py (12 lines)

Status	Author	Revision
Closed	phillco	D1617 rebase: extract _assignworkingcopy
Closed	phillco	D1616 rebase: disable `inmemory` if the rebaseset contains the working copy
Closed	phillco	D1249 rebase: rerun a rebase on-disk if IMM merge conflicts arise
Closed	phillco	D1248 rebase: add concludememorynode(), and call it when rebasing in-memory
Closed	phillco	D1247 rebase: pass the wctx object (IMM or on-disk) to merge.update
Closed	phillco	D1246 rebase: do not update if IMM; instead, set the overlaywctx's parents
Closed	phillco	D1245 rebase: pass wctx to rebasenode()
Closed	phillco	D1244 overlayworkingctx: invalidate the manifest cache when changing parents
Closed	phillco	D1243 overlayworkingctx: add `_compact()`
Closed	phillco	D1242 overlayworkingctx: add ``tomemctx()`` and ``commit()``
Closed	phillco	D1241 overlayworkingctx: add _auditconflicts to write()
Closed	phillco	D1240 overlayworkingctx: inherit from committablectx instead of workingctx
Closed	phillco	D1239 overlayworkingctx: add a no-op ``clearunknown()``
Closed	phillco	D1238 overlayworkingctx: add _manifest, files(), added(), removed(), modified()
Closed	phillco	D1237 overlayworkingctx: add `_checkexist(path)`
Closed	phillco	D1236 overlayworkingctx: fix a bad reference to `self._path`
Closed	phillco	D1235 overlayworkingctx: track copy information in the context
Closed	phillco	D1234 rebaseruntime: raise InMemoryMergeConflictsError on merge conflicts
Closed	phillco	D1233 rebase: do not bail on uncomitted changes if rebasing in-memory
Closed	phillco	D1232 rebase: add the --inmemory option flag; assign a wctx object for the rebase
Closed	phillco	D1231 overlayworkingctx: move _wrappedctx out of the constructor
Closed	phillco	D1230 overlayworkingctx: make clean() public
Closed	phillco	D1216 overlayworkingctx: remove flushall()
Closed	phillco	D1215 merge: skip subrepo state, update hooks, and updating the dirstate in IMM
Closed	phillco	D1214 merge: don't check for unknown files in IMM
Closed	phillco	D1213 merge: remove calls to flushall()
Closed	phillco	D1212 filemerge: raise InMemoryMergeConflictsError if we hit merge conflicts in IMM
Closed	phillco	D1211 context: switch ctx() use to changectx()
Abandoned	phillco	D1527 context: add an abstract base class for filectx

Diff 3903

mercurial/context.py

	# context.py - changeset and file context objects for mercurial			# context.py - changeset and file context objects for mercurial
	#			#
	# Copyright 2006, 2007 Matt Mackall <mpm@selenic.com>			# Copyright 2006, 2007 Matt Mackall <mpm@selenic.com>
	#			#
	# This software may be used and distributed according to the terms of the			# This software may be used and distributed according to the terms of the
	# GNU General Public License version 2 or any later version.			# GNU General Public License version 2 or any later version.

	from __future__ import absolute_import			from __future__ import absolute_import

				import abc
	import errno			import errno
	import filecmp			import filecmp
	import os			import os
	import re			import re
	import stat			import stat

	from .i18n import _			from .i18n import _
	from .node import (			from .node import (
	match.bad(fn, _('no such file in rev %s') % self)			match.bad(fn, _('no such file in rev %s') % self)

	m = matchmod.badmatch(match, bad)			m = matchmod.badmatch(match, bad)
	return self._manifest.walk(m)			return self._manifest.walk(m)

	def matches(self, match):			def matches(self, match):
	return self.walk(match)			return self.walk(match)

	class basefilectx(object):			class abstractfilectx(object):
				"""Defines the mininum required methods for filectxs"""
				__metaclass__ = abc.ABCMeta

				@abc.abstractmethod
				def data(self): pass
				@abc.abstractmethod
				def cmp(self, other): pass
				@abc.abstractmethod
				def changectx(self): pass
				@abc.abstractmethod
				def flags(self): pass
				@abc.abstractmethod
				def path(self): pass
				@abc.abstractmethod
				def size(self): pass
				@abc.abstractmethod
				def isbinary(self): pass
				@abc.abstractmethod
				def islink(self): pass
				@abc.abstractmethod
				def isexec(self): pass
				@abc.abstractmethod
				def isabsent(self): pass
				@abc.abstractmethod
				def write(self, data, flags): pass
				@abc.abstractmethod
				def setflags(self, l, x): pass

				class basefilectx(abstractfilectx):
	"""A filecontext object represents the common logic for its children:			"""A filecontext object represents the common logic for its children:
	filectx: read-only access to a filerevision that is already present			filectx: read-only access to a filerevision that is already present
	in the repo,			in the repo,
	workingfilectx: a filecontext that represents files from the working			workingfilectx: a filecontext that represents files from the working
	directory,			directory,
	memfilectx: a filecontext that represents files in-memory,			memfilectx: a filecontext that represents files in-memory,
	overlayfilectx: duplicate another filecontext with some fields overridden.			overlayfilectx: duplicate another filecontext with some fields overridden.
	"""			"""
	@propertycache			@propertycache
	def _filelog(self):			def _filelog(self):
	return self._repo.file(self._path)			return self._repo.file(self._path)
				martinvonzUnsubmitted Not Done For a separate patch: I find it weird to refer to these undefined fields (_repo and _path). Can we pass these into the constructor (which is currently not defined) and assign them there? martinvonz: For a separate patch: I find it weird to refer to these undefined fields (_repo and _path). Can…

	@propertycache			@propertycache
	def _changeid(self):			def _changeid(self):
	if r'_changeid' in self.__dict__:			if r'_changeid' in self.__dict__:
	return self._changeid			return self._changeid
	elif r'_changectx' in self.__dict__:			elif r'_changectx' in self.__dict__:
	return self._changectx.rev()			return self._changectx.rev()
	elif r'_descendantrev' in self.__dict__:			elif r'_descendantrev' in self.__dict__:
	return renamed			return renamed

	def children(self):			def children(self):
	# hard for renames			# hard for renames
	c = self._filelog.children(self._filenode)			c = self._filelog.children(self._filenode)
	return [filectx(self._repo, self._path, fileid=x,			return [filectx(self._repo, self._path, fileid=x,
	filelog=self._filelog) for x in c]			filelog=self._filelog) for x in c]

				def setflags(self, l, x):
				raise error.ProgrammingError("filectx is immutable")

				def write(self, data, flags):
				raise error.ProgrammingError("filectx is immutable")

	class committablectx(basectx):			class committablectx(basectx):
	"""A committablectx object provides common functionality for a context that			"""A committablectx object provides common functionality for a context that
	wants the ability to commit, e.g. workingctx or memctx."""			wants the ability to commit, e.g. workingctx or memctx."""
	def __init__(self, repo, text="", user=None, date=None, extra=None,			def __init__(self, repo, text="", user=None, date=None, extra=None,
	changes=None):			changes=None):
	self._repo = repo			self._repo = repo
	self._rev = None			self._rev = None
	self._node = None			self._node = None
	"""wraps unlink for a repo's working directory"""			"""wraps unlink for a repo's working directory"""
	# need to figure out what to do here			# need to figure out what to do here
	del self._changectx[self._path]			del self._changectx[self._path]

	def write(self, data, flags):			def write(self, data, flags):
	"""wraps repo.wwrite"""			"""wraps repo.wwrite"""
	self._data = data			self._data = data

				def setflags(self, l, x):
				raise NotImplementedError

	class overlayfilectx(committablefilectx):			class overlayfilectx(committablefilectx):
	"""Like memfilectx but take an original filectx and optional parameters to			"""Like memfilectx but take an original filectx and optional parameters to
	override parts of it. This is useful when fctx.data() is expensive (i.e.			override parts of it. This is useful when fctx.data() is expensive (i.e.
	flag processor is expensive) and raw data, flags, and filenode could be			flag processor is expensive) and raw data, flags, and filenode could be
	reused (ex. rebase or mode-only amend a REVIDX_EXTSTORED file).			reused (ex. rebase or mode-only amend a REVIDX_EXTSTORED file).
	"""			"""

	def __init__(self, originalfctx, datafunc=None, path=None, flags=None,			def __init__(self, originalfctx, datafunc=None, path=None, flags=None,
	added.append(f)			added.append(f)
	elif f in self:			elif f in self:
	modified.append(f)			modified.append(f)
	else:			else:
	removed.append(f)			removed.append(f)

	return scmutil.status(modified, added, removed, [], [], [], [])			return scmutil.status(modified, added, removed, [], [], [], [])

	class arbitraryfilectx(object):			class arbitraryfilectx(abstractfilectx):
	"""Allows you to use filectx-like functions on a file in an arbitrary			"""Allows you to use filectx-like functions on a file in an arbitrary
	location on disk, possibly not in the working directory.			location on disk, possibly not in the working directory.
	"""			"""
	def __init__(self, path, repo=None):			def __init__(self, path, repo=None):
	# Repo is optional because contrib/simplemerge uses this class.			# Repo is optional because contrib/simplemerge uses this class.
	self._repo = repo			self._repo = repo
	self._path = path			self._path = path

	def cmp(self, fctx):			def cmp(self, fctx):
	# filecmp follows symlinks whereas `cmp` should not, so skip the fast			# filecmp follows symlinks whereas `cmp` should not, so skip the fast
	# path if either side is a symlink.			# path if either side is a symlink.
	symlinks = ('l' in self.flags() or 'l' in fctx.flags())			symlinks = ('l' in self.flags() or 'l' in fctx.flags())
	if not symlinks and isinstance(fctx, workingfilectx) and self._repo:			if not symlinks and isinstance(fctx, workingfilectx) and self._repo:
	# Add a fast-path for merge if both sides are disk-backed.			# Add a fast-path for merge if both sides are disk-backed.
	# Note that filecmp uses the opposite return values (True if same)			# Note that filecmp uses the opposite return values (True if same)
	# from our cmp functions (True if different).			# from our cmp functions (True if different).
	return not filecmp.cmp(self.path(), self._repo.wjoin(fctx.path()))			return not filecmp.cmp(self.path(), self._repo.wjoin(fctx.path()))
	return self.data() != fctx.data()			return self.data() != fctx.data()

				def changectx(self):
				raise error.ProgrammingError("an arbitraryfilectx cannot have a "
				"changectx")

				def isabsent(self):
				return False

				# TODO deduplicate these from ``basefilectx``
				martinvonzUnsubmitted Not Done Can we put them in abstractfilectx if they seems generic enough? We can still created optimized versions in subclasses. martinvonz: Can we put them in abstractfilectx if they seems generic enough? We can still created optimized…
				def isbinary(self):
				try:
				return util.binary(self.data())
				except IOError:
				return False
				def isexec(self):
				return 'x' in self.flags()
				def islink(self):
				return 'l' in self.flags()

				def size(self):
				return os.stat(self._path).st_size

	def path(self):			def path(self):
	return self._path			return self._path

	def flags(self):			def flags(self):
	return ''			return ''

				def setflags(self, l, x):
				raise NotImplementedError

	def data(self):			def data(self):
	return util.readfile(self._path)			return util.readfile(self._path)

	def decodeddata(self):			def decodeddata(self):
	with open(self._path, "rb") as f:			with open(self._path, "rb") as f:
				martinvonzUnsubmitted Not Done Maybe in a separate patch, you could make this just "return self.data()"? martinvonz: Maybe in a separate patch, you could make this just "return self.data()"?
	return f.read()			return f.read()

	def remove(self):			def remove(self):
	util.unlink(self._path)			util.unlink(self._path)

	def write(self, data, flags):			def write(self, data, flags):
	assert not flags			assert not flags
	with open(self._path, "w") as f:			with open(self._path, "w") as f:
	f.write(data)			f.write(data)

mercurial/filemerge.py

	return self._ctx			return self._ctx

	def isbinary(self):			def isbinary(self):
	return False			return False

	def isabsent(self):			def isabsent(self):
	return True			return True

				def isexec(self):
				return False

				def islink(self):
				return False

				def setflags(self, l, x):
				raise error.ProgrammingError("absentfilectx is immutable")

				def write(self, data, flags):
				raise error.ProgrammingError("absentfilectx is immutable")

	def _findtool(ui, tool):			def _findtool(ui, tool):
	if tool in internals:			if tool in internals:
	return tool			return tool
	return findexternaltool(ui, tool)			return findexternaltool(ui, tool)

	def findexternaltool(ui, tool):			def findexternaltool(ui, tool):
	for kn in ("regkey", "regkeyalt"):			for kn in ("regkey", "regkeyalt"):
	k = _toolstr(ui, tool, kn)			k = _toolstr(ui, tool, kn)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	3903		Nov 27 2017, 10:35 PM	★	★
Diff 2	4175		Dec 7 2017, 4:22 PM	★	★