This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9577

upgrade: move `printrequirements()` to UpgradeOperation class
ClosedPublic

Authored by pulkit on Dec 14 2020, 4:55 AM.

Details

Reviewers
mharbison72
Group Reviewers
hg-reviewers
Commits
rHGaba979b1b90b: upgrade: move `printrequirements()` to UpgradeOperation class
Summary

Part of refactor where we make things more arranged and integrated into single
UpgradeOperation class.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Dec 14 2020, 4:55 AM
marmoute added a subscriber: marmoute.Dec 22 2020, 3:34 AM
marmoute added inline comments.
mercurial/upgrade_utils/actions.py
574–585

should we put this into _ attribute to avoid clutting the "public" API of the object ? (and maybe compute them within the print_requirements method?

pulkit added inline comments.Dec 28 2020, 3:36 AM
mercurial/upgrade_utils/actions.py
574–585

should we put this into _ attribute to avoid clutting the "public" API of the object ?

Agreed.

(and maybe compute them within the print_requirements method?

Maybe they can be used at more places, hence leaving them in __init__() for now.

pulkit updated this revision to Diff 24537.Dec 30 2020, 5:30 AM
marmoute added inline comments.Jan 5 2021, 2:52 AM
mercurial/upgrade_utils/actions.py
574–585

I am more leaning toward moving them in init when the "maybe" become a certitude. Or do you have actual code using that down the line ?

mharbison72 accepted this revision.Jan 5 2021, 10:34 AM
This revision is now accepted and ready to land.Jan 5 2021, 10:34 AM

Revision Contents
Changeset List

Diff 24537

mercurial/upgrade.py

if incompatible: if incompatible:
msg = _( msg = _(
b'ignoring revlogs selection flags, format requirements ' b'ignoring revlogs selection flags, format requirements '
b'change: %s\n' b'change: %s\n'
) )
ui.warn(msg % b', '.join(sorted(incompatible))) ui.warn(msg % b', '.join(sorted(incompatible)))
revlogs = upgrade_engine.UPGRADE_ALL_REVLOGS revlogs = upgrade_engine.UPGRADE_ALL_REVLOGS
def write_labeled(l, label):
first = True
for r in sorted(l):
if not first:
ui.write(b', ')
ui.write(r, label=label)
first = False
def printrequirements():
ui.write(_(b'requirements\n'))
ui.write(_(b' preserved: '))
write_labeled(
newreqs & repo.requirements, "upgrade-repo.requirement.preserved"
)
ui.write((b'\n'))
removed = repo.requirements - newreqs
if repo.requirements - newreqs:
ui.write(_(b' removed: '))
write_labeled(removed, "upgrade-repo.requirement.removed")
ui.write((b'\n'))
added = newreqs - repo.requirements
if added:
ui.write(_(b' added: '))
write_labeled(added, "upgrade-repo.requirement.added")
ui.write((b'\n'))
ui.write(b'\n')
upgrade_op = upgrade_actions.UpgradeOperation( upgrade_op = upgrade_actions.UpgradeOperation(
ui, ui,
newreqs, newreqs,
repo.requirements,
actions, actions,
revlogs, revlogs,
) )
if not run: if not run:
fromconfig = [] fromconfig = []
onlydefault = [] onlydefault = []
ui.status( ui.status(
_( _(
b'performing an upgrade with "--run" will make the following ' b'performing an upgrade with "--run" will make the following '
b'changes:\n\n' b'changes:\n\n'
) )
) )
printrequirements() upgrade_op.print_requirements()
upgrade_op.print_optimisations() upgrade_op.print_optimisations()
upgrade_op.print_upgrade_actions() upgrade_op.print_upgrade_actions()
upgrade_op.print_affected_revlogs() upgrade_op.print_affected_revlogs()
unusedoptimize = [i for i in alloptimizations if i not in actions] unusedoptimize = [i for i in alloptimizations if i not in actions]
if unusedoptimize: if unusedoptimize:
ui.status( ui.status(
_( _(
b'additional optimizations are available by specifying ' b'additional optimizations are available by specifying '
b'"--optimize <name>":\n\n' b'"--optimize <name>":\n\n'
) )
) )
for i in unusedoptimize: for i in unusedoptimize:
ui.status(_(b'%s\n %s\n\n') % (i.name, i.description)) ui.status(_(b'%s\n %s\n\n') % (i.name, i.description))
return return
# Else we're in the run=true case. # Else we're in the run=true case.
ui.write(_(b'upgrade will perform the following actions:\n\n')) ui.write(_(b'upgrade will perform the following actions:\n\n'))
printrequirements() upgrade_op.print_requirements()
upgrade_op.print_optimisations() upgrade_op.print_optimisations()
upgrade_op.print_upgrade_actions() upgrade_op.print_upgrade_actions()
upgrade_op.print_affected_revlogs() upgrade_op.print_affected_revlogs()
ui.status(_(b'beginning upgrade...\n')) ui.status(_(b'beginning upgrade...\n'))
with repo.wlock(), repo.lock(): with repo.wlock(), repo.lock():
ui.status(_(b'repository locked and read-only\n')) ui.status(_(b'repository locked and read-only\n'))
# Our strategy for upgrading the repository is to create a new, # Our strategy for upgrading the repository is to create a new,

mercurial/upgrade_utils/actions.py

# e.g. adding generaldelta could schedule parent redeltas. # e.g. adding generaldelta could schedule parent redeltas.
return newactions return newactions
class UpgradeOperation(object): class UpgradeOperation(object):
"""represent the work to be done during an upgrade""" """represent the work to be done during an upgrade"""
def __init__(self, ui, requirements, actions, revlogs_to_process): def __init__(
self,
ui,
new_requirements,
current_requirements,
actions,
revlogs_to_process,
):
self.ui = ui self.ui = ui
self.requirements = requirements self.new_requirements = new_requirements
self.current_requirements = current_requirements
self.actions = actions self.actions = actions
self._actions_names = set([a.name for a in actions]) self._actions_names = set([a.name for a in actions])
self.revlogs_to_process = revlogs_to_process self.revlogs_to_process = revlogs_to_process
# requirements which will be added by the operation
self._added_requirements = (
self.new_requirements - self.current_requirements
)
# requirements which will be removed by the operation
self._removed_requirements = (
self.current_requirements - self.new_requirements
)
# requirements which will be preserved by the operation
self._preserved_requirements = (
self.current_requirements & self.new_requirements
)
marmouteUnsubmitted
Not Done

should we put this into _ attribute to avoid clutting the "public" API of the object ? (and maybe compute them within the print_requirements method?

marmoute: should we put this into `_` attribute to avoid clutting the "public" API of the object ? (and…
pulkitAuthorUnsubmitted
Done

should we put this into _ attribute to avoid clutting the "public" API of the object ?

Agreed.

(and maybe compute them within the print_requirements method?

Maybe they can be used at more places, hence leaving them in __init__() for now.

pulkit: > should we put this into _ attribute to avoid clutting the "public" API of the object ?
marmouteUnsubmitted
Not Done

I am more leaning toward moving them in init when the "maybe" become a certitude. Or do you have actual code using that down the line ?

marmoute: I am more leaning toward moving them in __init__ **when** the "maybe" become a certitude. Or do…
def _write_labeled(self, l, label): def _write_labeled(self, l, label):
""" """
Utility function to aid writing of a list under one label Utility function to aid writing of a list under one label
""" """
first = True first = True
for r in sorted(l): for r in sorted(l):
if not first: if not first:
self.ui.write(b', ') self.ui.write(b', ')
self.ui.write(r, label=label) self.ui.write(r, label=label)
first = False first = False
def print_requirements(self):
self.ui.write(_(b'requirements\n'))
self.ui.write(_(b' preserved: '))
self._write_labeled(
self._preserved_requirements, "upgrade-repo.requirement.preserved"
)
self.ui.write((b'\n'))
if self._removed_requirements:
self.ui.write(_(b' removed: '))
self._write_labeled(
self._removed_requirements, "upgrade-repo.requirement.removed"
)
self.ui.write((b'\n'))
if self._added_requirements:
self.ui.write(_(b' added: '))
self._write_labeled(
self._added_requirements, "upgrade-repo.requirement.added"
)
self.ui.write((b'\n'))
self.ui.write(b'\n')
def print_optimisations(self): def print_optimisations(self):
optimisations = [a for a in self.actions if a.type == OPTIMISATION] optimisations = [a for a in self.actions if a.type == OPTIMISATION]
optimisations.sort(key=lambda a: a.name) optimisations.sort(key=lambda a: a.name)
if optimisations: if optimisations:
self.ui.write(_(b'optimisations: ')) self.ui.write(_(b'optimisations: '))
self._write_labeled( self._write_labeled(
[a.name for a in optimisations], [a.name for a in optimisations],
"upgrade-repo.optimisation.performed", "upgrade-repo.optimisation.performed",

mercurial/upgrade_utils/engine.py

upgrade_op.has_action(b're-delta-multibase'), upgrade_op.has_action(b're-delta-multibase'),
revlogs=upgrade_op.revlogs_to_process, revlogs=upgrade_op.revlogs_to_process,
) )
# Now copy other files in the store directory. # Now copy other files in the store directory.
# The sorted() makes execution deterministic. # The sorted() makes execution deterministic.
for p, kind, st in sorted(srcrepo.store.vfs.readdir(b'', stat=True)): for p, kind, st in sorted(srcrepo.store.vfs.readdir(b'', stat=True)):
if not _filterstorefile( if not _filterstorefile(
srcrepo, dstrepo, upgrade_op.requirements, p, kind, st srcrepo, dstrepo, upgrade_op.new_requirements, p, kind, st
): ):
continue continue
srcrepo.ui.status(_(b'copying %s\n') % p) srcrepo.ui.status(_(b'copying %s\n') % p)
src = srcrepo.store.rawvfs.join(p) src = srcrepo.store.rawvfs.join(p)
dst = dstrepo.store.rawvfs.join(p) dst = dstrepo.store.rawvfs.join(p)
util.copyfile(src, dst, copystat=True) util.copyfile(src, dst, copystat=True)
# We first write the requirements file. Any new requirements will lock # We first write the requirements file. Any new requirements will lock
# out legacy clients. # out legacy clients.
ui.status( ui.status(
_( _(
b'finalizing requirements file and making repository readable ' b'finalizing requirements file and making repository readable '
b'again\n' b'again\n'
) )
) )
scmutil.writereporequirements(srcrepo, upgrade_op.requirements) scmutil.writereporequirements(srcrepo, upgrade_op.new_requirements)
# The lock file from the old store won't be removed because nothing has a # The lock file from the old store won't be removed because nothing has a
# reference to its new location. So clean it up manually. Alternatively, we # reference to its new location. So clean it up manually. Alternatively, we
# could update srcrepo.svfs and other variables to point to the new # could update srcrepo.svfs and other variables to point to the new
# location. This is simpler. # location. This is simpler.
backupvfs.unlink(b'store/lock') backupvfs.unlink(b'store/lock')
return backuppath return backuppath