This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3765

progress: create helper class for incrementing progress
ClosedPublic

Authored by martinvonz on Jun 17 2018, 12:42 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGbec1212eceaa: progress: create helper class for incrementing progress
Summary

When using ui.progress(), there's a clear pattern that is followed:

  • Pass the same topic and unit
  • Usually pass the same total
  • Call with pos=None to close the progress bar
  • Often keep track of the current position and increment it

This patch creates a simple helper class for this. I'll probably make
it implement the context manager protocol later (calling update(None)
on exit).

Progress is used in low-level modules like changegroup, so I also
exposed it via a method on the ui object. Perhaps the class itself
should also live in ui.py?

This patch also makes merge.oy use it to show that it works.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Jun 17 2018, 12:42 PM
indygreg accepted this revision.Jun 17 2018, 2:01 PM
indygreg added a subscriber: indygreg.

The low-level progress API has always bothered me as well.

Please do follow up and add a context manager to the API!

This revision is now accepted and ready to land.Jun 17 2018, 2:01 PM
Closed by commit rHGbec1212eceaa: progress: create helper class for incrementing progress (authored by martinvonz). · Explain WhyJun 17 2018, 2:07 PM
This revision was automatically updated to reflect the committed changes.
indygreg added a comment.Jun 17 2018, 2:12 PM

Also, it's worth keeping in mind Python function call overhead when working on this code. I'm optimistic that things that need progress bars won't be concerned about this. But it might be worth measuring and keeping in the back of your head.

martinvonz added a comment.Jun 17 2018, 2:54 PM
In D3765#59079, @indygreg wrote:

Also, it's worth keeping in mind Python function call overhead when working on this code. I'm optimistic that things that need progress bars won't be concerned about this. But it might be worth measuring and keeping in the back of your head.

Good point. I also think it's unlikely to be noticeable. Also, we can probably inline the ui.progress() logic if it turns out to be an issue.

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (45 lines)
Mmercurial/scmutil.py (21 lines)
Mmercurial/ui.py (4 lines)

Diff 9128

mercurial/merge.py

fca = actx[fa] fca = actx[fa]
else: else:
# TODO: move to absentfilectx # TODO: move to absentfilectx
fca = repo.filectx(f1, fileid=nullrev) fca = repo.filectx(f1, fileid=nullrev)
ms.add(fcl, fco, fca, f) ms.add(fcl, fco, fca, f)
if f1 != f and move: if f1 != f and move:
moves.append(f1) moves.append(f1)
_updating = _('updating')
_files = _('files')
progress = repo.ui.progress
# remove renamed files after safely stored # remove renamed files after safely stored
for f in moves: for f in moves:
if wctx[f].lexists(): if wctx[f].lexists():
repo.ui.debug("removing %s\n" % f) repo.ui.debug("removing %s\n" % f)
wctx[f].audit() wctx[f].audit()
wctx[f].remove() wctx[f].remove()
numupdates = sum(len(l) for m, l in actions.items() numupdates = sum(len(l) for m, l in actions.items()
if m != ACTION_KEEP) if m != ACTION_KEEP)
z = 0 progress = repo.ui.makeprogress(_('updating'), unit=_('files'),
total=numupdates)
if [a for a in actions[ACTION_REMOVE] if a[0] == '.hgsubstate']: if [a for a in actions[ACTION_REMOVE] if a[0] == '.hgsubstate']:
subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels) subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels)
# record path conflicts # record path conflicts
for f, args, msg in actions[ACTION_PATH_CONFLICT]: for f, args, msg in actions[ACTION_PATH_CONFLICT]:
f1, fo = args f1, fo = args
s = repo.ui.status s = repo.ui.status
s(_("%s: path conflict - a file or link has the same name as a " s(_("%s: path conflict - a file or link has the same name as a "
"directory\n") % f) "directory\n") % f)
if fo == 'l': if fo == 'l':
s(_("the local file has been renamed to %s\n") % f1) s(_("the local file has been renamed to %s\n") % f1)
else: else:
s(_("the remote file has been renamed to %s\n") % f1) s(_("the remote file has been renamed to %s\n") % f1)
s(_("resolve manually then use 'hg resolve --mark %s'\n") % f) s(_("resolve manually then use 'hg resolve --mark %s'\n") % f)
ms.addpath(f, f1, fo) ms.addpath(f, f1, fo)
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
# When merging in-memory, we can't support worker processes, so set the # When merging in-memory, we can't support worker processes, so set the
# per-item cost at 0 in that case. # per-item cost at 0 in that case.
cost = 0 if wctx.isinmemory() else 0.001 cost = 0 if wctx.isinmemory() else 0.001
# remove in parallel (must come before resolving path conflicts and getting) # remove in parallel (must come before resolving path conflicts and getting)
prog = worker.worker(repo.ui, cost, batchremove, (repo, wctx), prog = worker.worker(repo.ui, cost, batchremove, (repo, wctx),
actions[ACTION_REMOVE]) actions[ACTION_REMOVE])
for i, item in prog: for i, item in prog:
z += i progress.increment(step=i, item=item)
progress(_updating, z, item=item, total=numupdates, unit=_files)
removed = len(actions[ACTION_REMOVE]) removed = len(actions[ACTION_REMOVE])
# resolve path conflicts (must come before getting) # resolve path conflicts (must come before getting)
for f, args, msg in actions[ACTION_PATH_CONFLICT_RESOLVE]: for f, args, msg in actions[ACTION_PATH_CONFLICT_RESOLVE]:
repo.ui.debug(" %s: %s -> pr\n" % (f, msg)) repo.ui.debug(" %s: %s -> pr\n" % (f, msg))
f0, = args f0, = args
if wctx[f0].lexists(): if wctx[f0].lexists():
repo.ui.note(_("moving %s to %s\n") % (f0, f)) repo.ui.note(_("moving %s to %s\n") % (f0, f))
wctx[f].audit() wctx[f].audit()
wctx[f].write(wctx.filectx(f0).data(), wctx.filectx(f0).flags()) wctx[f].write(wctx.filectx(f0).data(), wctx.filectx(f0).flags())
wctx[f0].remove() wctx[f0].remove()
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
# get in parallel # get in parallel
prog = worker.worker(repo.ui, cost, batchget, (repo, mctx, wctx), prog = worker.worker(repo.ui, cost, batchget, (repo, mctx, wctx),
actions[ACTION_GET]) actions[ACTION_GET])
for i, item in prog: for i, item in prog:
z += i progress.increment(step=i, item=item)
progress(_updating, z, item=item, total=numupdates, unit=_files)
updated = len(actions[ACTION_GET]) updated = len(actions[ACTION_GET])
if [a for a in actions[ACTION_GET] if a[0] == '.hgsubstate']: if [a for a in actions[ACTION_GET] if a[0] == '.hgsubstate']:
subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels) subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels)
# forget (manifest only, just log it) (must come first) # forget (manifest only, just log it) (must come first)
for f, args, msg in actions[ACTION_FORGET]: for f, args, msg in actions[ACTION_FORGET]:
repo.ui.debug(" %s: %s -> f\n" % (f, msg)) repo.ui.debug(" %s: %s -> f\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
# re-add (manifest only, just log it) # re-add (manifest only, just log it)
for f, args, msg in actions[ACTION_ADD]: for f, args, msg in actions[ACTION_ADD]:
repo.ui.debug(" %s: %s -> a\n" % (f, msg)) repo.ui.debug(" %s: %s -> a\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
# re-add/mark as modified (manifest only, just log it) # re-add/mark as modified (manifest only, just log it)
for f, args, msg in actions[ACTION_ADD_MODIFIED]: for f, args, msg in actions[ACTION_ADD_MODIFIED]:
repo.ui.debug(" %s: %s -> am\n" % (f, msg)) repo.ui.debug(" %s: %s -> am\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
# keep (noop, just log it) # keep (noop, just log it)
for f, args, msg in actions[ACTION_KEEP]: for f, args, msg in actions[ACTION_KEEP]:
repo.ui.debug(" %s: %s -> k\n" % (f, msg)) repo.ui.debug(" %s: %s -> k\n" % (f, msg))
# no progress # no progress
# directory rename, move local # directory rename, move local
for f, args, msg in actions[ACTION_DIR_RENAME_MOVE_LOCAL]: for f, args, msg in actions[ACTION_DIR_RENAME_MOVE_LOCAL]:
repo.ui.debug(" %s: %s -> dm\n" % (f, msg)) repo.ui.debug(" %s: %s -> dm\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
f0, flags = args f0, flags = args
repo.ui.note(_("moving %s to %s\n") % (f0, f)) repo.ui.note(_("moving %s to %s\n") % (f0, f))
wctx[f].audit() wctx[f].audit()
wctx[f].write(wctx.filectx(f0).data(), flags) wctx[f].write(wctx.filectx(f0).data(), flags)
wctx[f0].remove() wctx[f0].remove()
updated += 1 updated += 1
# local directory rename, get # local directory rename, get
for f, args, msg in actions[ACTION_LOCAL_DIR_RENAME_GET]: for f, args, msg in actions[ACTION_LOCAL_DIR_RENAME_GET]:
repo.ui.debug(" %s: %s -> dg\n" % (f, msg)) repo.ui.debug(" %s: %s -> dg\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
f0, flags = args f0, flags = args
repo.ui.note(_("getting %s to %s\n") % (f0, f)) repo.ui.note(_("getting %s to %s\n") % (f0, f))
wctx[f].write(mctx.filectx(f0).data(), flags) wctx[f].write(mctx.filectx(f0).data(), flags)
updated += 1 updated += 1
# exec # exec
for f, args, msg in actions[ACTION_EXEC]: for f, args, msg in actions[ACTION_EXEC]:
repo.ui.debug(" %s: %s -> e\n" % (f, msg)) repo.ui.debug(" %s: %s -> e\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
flags, = args flags, = args
wctx[f].audit() wctx[f].audit()
wctx[f].setflags('l' in flags, 'x' in flags) wctx[f].setflags('l' in flags, 'x' in flags)
updated += 1 updated += 1
# the ordering is important here -- ms.mergedriver will raise if the merge # the ordering is important here -- ms.mergedriver will raise if the merge
# driver has changed, and we want to be able to bypass it when overwrite is # driver has changed, and we want to be able to bypass it when overwrite is
# True # True
newactions.append((f, args, msg)) newactions.append((f, args, msg))
mergeactions = newactions mergeactions = newactions
try: try:
# premerge # premerge
tocomplete = [] tocomplete = []
for f, args, msg in mergeactions: for f, args, msg in mergeactions:
repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg)) repo.ui.debug(" %s: %s -> m (premerge)\n" % (f, msg))
z += 1 progress.increment(item=f)
progress(_updating, z, item=f, total=numupdates, unit=_files)
if f == '.hgsubstate': # subrepo states need updating if f == '.hgsubstate': # subrepo states need updating
subrepoutil.submerge(repo, wctx, mctx, wctx.ancestor(mctx), subrepoutil.submerge(repo, wctx, mctx, wctx.ancestor(mctx),
overwrite, labels) overwrite, labels)
continue continue
wctx[f].audit() wctx[f].audit()
complete, r = ms.preresolve(f, wctx) complete, r = ms.preresolve(f, wctx)
if not complete: if not complete:
numupdates += 1 numupdates += 1
tocomplete.append((f, args, msg)) tocomplete.append((f, args, msg))
# merge # merge
for f, args, msg in tocomplete: for f, args, msg in tocomplete:
repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg)) repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
z += 1 progress.increment(item=f, total=numupdates)
progress(_updating, z, item=f, total=numupdates, unit=_files)
ms.resolve(f, wctx) ms.resolve(f, wctx)
finally: finally:
ms.commit() ms.commit()
unresolved = ms.unresolvedcount() unresolved = ms.unresolvedcount()
if (usemergedriver and not unresolved if (usemergedriver and not unresolved
# #
# We don't need to do the same operation for 'dc' and 'cd' because # We don't need to do the same operation for 'dc' and 'cd' because
# those lists aren't consulted again. # those lists aren't consulted again.
mfiles.difference_update(a[0] for a in acts) mfiles.difference_update(a[0] for a in acts)
actions[ACTION_MERGE] = [a for a in actions[ACTION_MERGE] actions[ACTION_MERGE] = [a for a in actions[ACTION_MERGE]
if a[0] in mfiles] if a[0] in mfiles]
progress(_updating, None, total=numupdates, unit=_files) progress.update(None)
return updateresult(updated, merged, removed, unresolved) return updateresult(updated, merged, removed, unresolved)
def recordupdates(repo, actions, branchmerge): def recordupdates(repo, actions, branchmerge):
"record merge actions to the dirstate" "record merge actions to the dirstate"
# remove (must come first) # remove (must come first)
for f, args, msg in actions.get(ACTION_REMOVE, []): for f, args, msg in actions.get(ACTION_REMOVE, []):
if branchmerge: if branchmerge:
repo.dirstate.remove(f) repo.dirstate.remove(f)

mercurial/scmutil.py

"""run cmd as a subprocess that allows inheriting repo's wlock """run cmd as a subprocess that allows inheriting repo's wlock
This can only be called while the wlock is held. This takes all the This can only be called while the wlock is held. This takes all the
arguments that ui.system does, and returns the exit code of the arguments that ui.system does, and returns the exit code of the
subprocess.""" subprocess."""
return _locksub(repo, repo.currentwlock(), 'HG_WLOCK_LOCKER', cmd, *args, return _locksub(repo, repo.currentwlock(), 'HG_WLOCK_LOCKER', cmd, *args,
**kwargs) **kwargs)
class progress(object):
def __init__(self, ui, topic, unit="", total=None):
self.ui = ui
self.pos = 0
self.topic = topic
self.unit = unit
self.total = total
def update(self, pos, item="", total=None):
if total:
self.total = total
self.pos = pos
self._print(item)
def increment(self, step=1, item="", total=None):
self.update(self.pos + step, item, total)
def _print(self, item):
self.ui.progress(self.topic, self.pos, item, self.unit,
self.total)
def gdinitconfig(ui): def gdinitconfig(ui):
"""helper function to know if a repo should be created as general delta """helper function to know if a repo should be created as general delta
""" """
# experimental config: format.generaldelta # experimental config: format.generaldelta
return (ui.configbool('format', 'generaldelta') return (ui.configbool('format', 'generaldelta')
or ui.configbool('format', 'usegeneraldelta')) or ui.configbool('format', 'usegeneraldelta'))
def gddeltaconfig(ui): def gddeltaconfig(ui):

mercurial/ui.py

if total: if total:
pct = 100.0 * pos / total pct = 100.0 * pos / total
self.debug('%s:%s %d/%d%s (%4.2f%%)\n' self.debug('%s:%s %d/%d%s (%4.2f%%)\n'
% (topic, item, pos, total, unit, pct)) % (topic, item, pos, total, unit, pct))
else: else:
self.debug('%s:%s %d%s\n' % (topic, item, pos, unit)) self.debug('%s:%s %d%s\n' % (topic, item, pos, unit))
def makeprogress(self, topic, unit="", total=None):
'''exists only so low-level modules won't need to import scmutil'''
return scmutil.progress(self, topic, unit, total)
def log(self, service, *msg, **opts): def log(self, service, *msg, **opts):
'''hook for logging facility extensions '''hook for logging facility extensions
service should be a readily-identifiable subsystem, which will service should be a readily-identifiable subsystem, which will
allow filtering. allow filtering.
*msg should be a newline-terminated format string to log, and *msg should be a newline-terminated format string to log, and
then any values to %-format into that format string. then any values to %-format into that format string.