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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 24605

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