This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10768

upgrade: Use `improvement` subclasses everywhere, not instances
ClosedPublic

Authored by SimonSapin on May 25 2021, 2:11 AM.

Download Raw Diff

Details

Reviewers

marmoute

Group Reviewers

hg-reviewers

Commits

rHGe985a36c2aa3: upgrade: Use `improvement` subclasses everywhere, not instances
rHGb3e2ae9e6d42: upgrade: Use `improvement` subclasses everywhere, not instances

Summary

This changes the source definition of optimizations to match that of formats:
a subclass with a decorator, instead of an instance passed to a function call.
Not having any instance removes the confusion between class attributes and
instance attributes, which were used interchangeably.

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

SimonSapin created this revision.May 25 2021, 2:11 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMay 25 2021, 2:11 AM

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

SimonSapin added a child revision: D10769: dirstate-v2: Add `hg debugupgraderepo` command support.May 25 2021, 2:11 AM

marmoute requested changes to this revision.May 25 2021, 4:35 AM

marmoute added a subscriber: marmoute.

marmoute added inline comments.

mercurial/upgrade_utils/actions.py
49–90	Why are dropping these comment? it seems useful.

This revision now requires changes to proceed.May 25 2021, 4:35 AM

SimonSapin added inline comments.May 26 2021, 5:14 AM

mercurial/upgrade_utils/actions.py
49–90	This docstring used to document instance attributes, but instances attributes aren’t used anymore. Class attributes already have individual comments in the base class definition with mostly the same content. I had missed that `touches_*` class attributes were not documented individually, I’ll restore that (even though these comments are not doing much besides paraphrasing what the attribute names are already saying).

marmoute added inline comments.May 26 2021, 5:15 AM

mercurial/upgrade_utils/actions.py
49–90	okay, thanks for the explanation.

SimonSapin updated this revision to Diff 28204.May 26 2021, 5:16 AM

SimonSapin added a commit: rHGb3e2ae9e6d42: upgrade: Use `improvement` subclasses everywhere, not instances.May 27 2021, 2:59 PM

This revision was not accepted when it landed; it landed in state Needs Review.

Closed by commit rHGb3e2ae9e6d42: upgrade: Use `improvement` subclasses everywhere, not instances (authored by SimonSapin). · Explain Why

This revision was automatically updated to reflect the committed changes.

SimonSapin added a commit: rHGe985a36c2aa3: upgrade: Use `improvement` subclasses everywhere, not instances.May 28 2021, 5:39 PM

Revision Contents
Changeset List

			Path	Packages
M			mercurial/upgrade_utils/actions.py (277 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	28198		May 25 2021, 2:11 AM	★	★
Diff 2	28204		May 26 2021, 5:16 AM	★	★
Diff 3	28215	rHGb3e2ae9e6d4289f9500f2ef1ab9213fa4ee24af1	May 21 2021, 11:12 AM	★	★

	Status	Author	Revision
	Closed	SimonSapin	D10769 dirstate-v2: Add `hg debugupgraderepo` command support
	Closed	SimonSapin	D10768 upgrade: Use `improvement` subclasses everywhere, not instances

Diff 28215

mercurial/upgrade_utils/actions.py

	return set()			return set()


	FORMAT_VARIANT = b'deficiency'			FORMAT_VARIANT = b'deficiency'
	OPTIMISATION = b'optimization'			OPTIMISATION = b'optimization'


	class improvement(object):			class improvement(object):
	"""Represents an improvement that can be made as part of an upgrade.			"""Represents an improvement that can be made as part of an upgrade."""

	The following attributes are defined on each instance:			### The following attributes should be defined for each subclass:

	name			# Either ``FORMAT_VARIANT`` or ``OPTIMISATION``.
	Machine-readable string uniquely identifying this improvement. It			# A format variant is where we change the storage format. Not all format
	will be mapped to an action later in the upgrade process.			# variant changes are an obvious problem.
				# An optimization is an action (sometimes optional) that
	type			# can be taken to further improve the state of the repository.
	Either ``FORMAT_VARIANT`` or ``OPTIMISATION``.			type = None
	A format variant is where we change the storage format. Not all format
	variant changes are an obvious problem.
	An optimization is an action (sometimes optional) that
	can be taken to further improve the state of the repository.

	description
	Message intended for humans explaining the improvement in more detail,
	including the implications of it. For ``FORMAT_VARIANT`` types, should be
	worded in the present tense. For ``OPTIMISATION`` types, should be
	worded in the future tense.

	upgrademessage
	Message intended for humans explaining what an upgrade addressing this
	issue will do. Should be worded in the future tense.

	postupgrademessage
	Message intended for humans which will be shown post an upgrade
	operation when the improvement will be added

	postdowngrademessage
	Message intended for humans which will be shown post an upgrade
	operation in which this improvement was removed

	touches_filelogs (bool)
	Whether this improvement touches filelogs

	touches_manifests (bool)
	Whether this improvement touches manifests

	touches_changelog (bool)
	Whether this improvement touches changelog

	touches_requirements (bool)
	Whether this improvement changes repository requirements
	marmouteUnsubmitted Not Done Why are dropping these comment? it seems useful. marmoute: Why are dropping these comment? it seems useful.
	SimonSapinAuthorUnsubmitted Done This docstring used to document instance attributes, but instances attributes aren’t used anymore. Class attributes already have individual comments in the base class definition with mostly the same content. I had missed that `touches_` class attributes were not documented individually, I’ll restore that (even though these comments are not doing much besides paraphrasing what the attribute names are already saying). SimonSapin:* This docstring used to document instance attributes, but instances attributes aren’t used…
	marmouteUnsubmitted Not Done okay, thanks for the explanation. marmoute: okay, thanks for the explanation.
	"""

	def __init__(self, name, type, description, upgrademessage):
	self.name = name
	self.type = type
	self.description = description
	self.upgrademessage = upgrademessage
	self.postupgrademessage = None
	self.postdowngrademessage = None
	# By default for now, we assume every improvement touches
	# all the things
	self.touches_filelogs = True
	self.touches_manifests = True
	self.touches_changelog = True
	self.touches_requirements = True

	def __eq__(self, other):
	if not isinstance(other, improvement):
	# This is what python tell use to do
	return NotImplemented
	return self.name == other.name

	def __ne__(self, other):
	return not (self == other)

	def __hash__(self):
	return hash(self.name)


	allformatvariant = [] # type: List[Type['formatvariant']]


	def registerformatvariant(cls):
	allformatvariant.append(cls)
	return cls


	class formatvariant(improvement):
	"""an improvement subclass dedicated to repository format"""

	type = FORMAT_VARIANT
	### The following attributes should be defined for each class:

	# machine-readable string uniquely identifying this improvement. it will be			# machine-readable string uniquely identifying this improvement. it will be
	# mapped to an action later in the upgrade process.			# mapped to an action later in the upgrade process.
	name = None			name = None

	# message intended for humans explaining the improvement in more detail,			# message intended for humans explaining the improvement in more detail,
	# including the implications of it ``FORMAT_VARIANT`` types, should be			# including the implications of it ``FORMAT_VARIANT`` types, should be
	# worded			# worded
	# Message intended for humans which will be shown post an upgrade			# Message intended for humans which will be shown post an upgrade
	# operation when the improvement will be added			# operation when the improvement will be added
	postupgrademessage = None			postupgrademessage = None

	# Message intended for humans which will be shown post an upgrade			# Message intended for humans which will be shown post an upgrade
	# operation in which this improvement was removed			# operation in which this improvement was removed
	postdowngrademessage = None			postdowngrademessage = None


	# By default for now, we assume every improvement touches all the things			# By default for now, we assume every improvement touches all the things

				# Whether this improvement touches filelogs
	touches_filelogs = True			touches_filelogs = True

				# Whether this improvement touches manifests
	touches_manifests = True			touches_manifests = True

				# Whether this improvement touches changelog
	touches_changelog = True			touches_changelog = True

				# Whether this improvement changes repository requirements
	touches_requirements = True			touches_requirements = True

	def __init__(self):
	raise NotImplementedError()			allformatvariant = [] # type: List[Type['formatvariant']]


				def registerformatvariant(cls):
				allformatvariant.append(cls)
				return cls


				class formatvariant(improvement):
				"""an improvement subclass dedicated to repository format"""

				type = FORMAT_VARIANT

	@staticmethod			@staticmethod
	def fromrepo(repo):			def fromrepo(repo):
	"""current value of the variant in the repository"""			"""current value of the variant in the repository"""
	raise NotImplementedError()			raise NotImplementedError()

	@staticmethod			@staticmethod
	def fromconfig(repo):			def fromconfig(repo):
	ALL_OPTIMISATIONS = []			ALL_OPTIMISATIONS = []


	def register_optimization(obj):			def register_optimization(obj):
	ALL_OPTIMISATIONS.append(obj)			ALL_OPTIMISATIONS.append(obj)
	return obj			return obj


	register_optimization(			class optimization(improvement):
	improvement(			"""an improvement subclass dedicated to optimizations"""
	name=b're-delta-parent',
	type=OPTIMISATION,			type = OPTIMISATION


				@register_optimization
				class redeltaparents(optimization):
				name = b're-delta-parent'

				type = OPTIMISATION

	description=_(			description = _(
	b'deltas within internal storage will be recalculated to '			b'deltas within internal storage will be recalculated to '
	b'choose an optimal base revision where this was not '			b'choose an optimal base revision where this was not '
	b'already done; the size of the repository may shrink and '			b'already done; the size of the repository may shrink and '
	b'various operations may become faster; the first time '			b'various operations may become faster; the first time '
	b'this optimization is performed could slow down upgrade '			b'this optimization is performed could slow down upgrade '
	b'execution considerably; subsequent invocations should '			b'execution considerably; subsequent invocations should '
	b'not run noticeably slower'			b'not run noticeably slower'
	),			)

	upgrademessage=_(			upgrademessage = _(
	b'deltas within internal storage will choose a new '			b'deltas within internal storage will choose a new '
	b'base revision if needed'			b'base revision if needed'
	),
	)
	)			)

	register_optimization(
	improvement(			@register_optimization
	name=b're-delta-multibase',			class redeltamultibase(optimization):
	type=OPTIMISATION,			name = b're-delta-multibase'

				type = OPTIMISATION

	description=_(			description = _(
	b'deltas within internal storage will be recalculated '			b'deltas within internal storage will be recalculated '
	b'against multiple base revision and the smallest '			b'against multiple base revision and the smallest '
	b'difference will be used; the size of the repository may '			b'difference will be used; the size of the repository may '
	b'shrink significantly when there are many merges; this '			b'shrink significantly when there are many merges; this '
	b'optimization will slow down execution in proportion to '			b'optimization will slow down execution in proportion to '
	b'the number of merges in the repository and the amount '			b'the number of merges in the repository and the amount '
	b'of files in the repository; this slow down should not '			b'of files in the repository; this slow down should not '
	b'be significant unless there are tens of thousands of '			b'be significant unless there are tens of thousands of '
	b'files and thousands of merges'			b'files and thousands of merges'
	),			)

	upgrademessage=_(			upgrademessage = _(
	b'deltas within internal storage will choose an '			b'deltas within internal storage will choose an '
	b'optimal delta by computing deltas against multiple '			b'optimal delta by computing deltas against multiple '
	b'parents; may slow down execution time '			b'parents; may slow down execution time '
	b'significantly'			b'significantly'
	),
	)
	)			)

	register_optimization(
	improvement(			@register_optimization
	name=b're-delta-all',			class redeltaall(optimization):
	type=OPTIMISATION,			name = b're-delta-all'

				type = OPTIMISATION

	description=_(			description = _(
	b'deltas within internal storage will always be '			b'deltas within internal storage will always be '
	b'recalculated without reusing prior deltas; this will '			b'recalculated without reusing prior deltas; this will '
	b'likely make execution run several times slower; this '			b'likely make execution run several times slower; this '
	b'optimization is typically not needed'			b'optimization is typically not needed'
	),			)

	upgrademessage=_(			upgrademessage = _(
	b'deltas within internal storage will be fully '			b'deltas within internal storage will be fully '
	b'recomputed; this will likely drastically slow down '			b'recomputed; this will likely drastically slow down '
	b'execution time'			b'execution time'
	),
	)
	)			)

	register_optimization(
	improvement(			@register_optimization
	name=b're-delta-fulladd',			class redeltafulladd(optimization):
	type=OPTIMISATION,			name = b're-delta-fulladd'

				type = OPTIMISATION

	description=_(			description = _(
	b'every revision will be re-added as if it was new '			b'every revision will be re-added as if it was new '
	b'content. It will go through the full storage '			b'content. It will go through the full storage '
	b'mechanism giving extensions a chance to process it '			b'mechanism giving extensions a chance to process it '
	b'(eg. lfs). This is similar to "re-delta-all" but even '			b'(eg. lfs). This is similar to "re-delta-all" but even '
	b'slower since more logic is involved.'			b'slower since more logic is involved.'
	),			)

	upgrademessage=_(			upgrademessage = _(
	b'each revision will be added as new content to the '			b'each revision will be added as new content to the '
	b'internal storage; this will likely drastically slow '			b'internal storage; this will likely drastically slow '
	b'down execution time, but some extensions might need '			b'down execution time, but some extensions might need '
	b'it'			b'it'
	),
	)
	)			)


	def findoptimizations(repo):			def findoptimizations(repo):
	"""Determine optimisation that could be used during upgrade"""			"""Determine optimisation that could be used during upgrade"""
	# These are unconditionally added. There is logic later that figures out			# These are unconditionally added. There is logic later that figures out
	# which ones to apply.			# which ones to apply.
	return list(ALL_OPTIMISATIONS)			return list(ALL_OPTIMISATIONS)