This is an archive of the discontinued Mercurial Phabricator instance.

Differential D200

undo: cover lock error cases and add timeout
ClosedPublic

Authored by felixmerk on Jul 31 2017, 5:50 PM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
stash
Group Reviewers
Restricted Project
Commits
rFBHGXcaf5efb61c75: undo: cover lock error cases and add timeout
Summary

Lock error handling reduces likelyhood undo hangs or breaks other commands.
Fixes known bug of undo in read-only repo. Undo is not neccesary for mercurial
to work and handles missing data (decently) well. Failing silently (potentially
logging to scuba for timeouts) is acceptable, since if the issue is real we will
fail not silently when trying to undo.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

felixmerk created this revision.Jul 31 2017, 5:50 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 31 2017, 5:50 PM
quark requested changes to this revision.Jul 31 2017, 6:21 PM
quark added a subscriber: quark.
quark added inline comments.
hgext3rd/undo.py
87

This could raise OSError that is not handled.

100

nit: need \n.

104

\n

This revision now requires changes to proceed.Jul 31 2017, 6:21 PM
felixmerk edited edge metadata.Jul 31 2017, 6:49 PM
felixmerk updated this revision to Diff 468.
stash accepted this revision.Aug 1 2017, 10:32 AM
quark accepted this revision.Aug 1 2017, 2:38 PM

Please add the missing \n before landing this.

hgext3rd/undo.py
90

\n

This revision is now accepted and ready to land.Aug 1 2017, 2:38 PM
Closed by commit rFBHGXcaf5efb61c75: undo: cover lock error cases and add timeout (authored by felixmerk). · Explain WhyAug 3 2017, 5:45 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext3rd/undo.py (34 lines)

Diff 518

hgext3rd/undo.py

True if changes have been recorded, False otherwise True if changes have been recorded, False otherwise
''' '''
changes = False changes = False
if repo is not None:# some hg commands don't require repo if repo is not None:# some hg commands don't require repo
# undolog specific lock # undolog specific lock
# allows running command during other commands when # allows running command during other commands when
# otherwise legal. Could cause weird undolog states, # otherwise legal. Could cause weird undolog states,
# which gap handling generally covers. # which gap handling generally covers.
try:
try:
quarkUnsubmitted
Not Done

This could raise OSError that is not handled.

quark: This could raise `OSError` that is not handled.
repo.vfs.makedirs('undolog') repo.vfs.makedirs('undolog')
with lockmod.lock(repo.vfs, "undolog/lock", desc="undolog"): except OSError:
repo.ui.debug("can't make undolog folder in .hg")
quarkUnsubmitted
Not Done

\n

quark: `\n`
return changes
with lockmod.lock(repo.vfs, "undolog/lock", desc="undolog",
timeout=2):
# developer config: undo._duringundologlock # developer config: undo._duringundologlock
if repo.ui.configbool('undo', '_duringundologlock'): if repo.ui.configbool('undo', '_duringundologlock'):
repo.hook("duringundologlock") repo.hook("duringundologlock")
tr = lighttransaction(repo) tr = lighttransaction(repo)
with tr: with tr:
changes = log(repo.filtered('visible'), command, tr) changes = log(repo.filtered('visible'), command, tr)
if changes and not ("undo" == command[0] or "redo" == if changes and not ("undo" == command[0] or "redo" ==
quarkUnsubmitted
Not Done

nit: need \n.

quark: nit: need `\n`.
command[0]): command[0]):
_delundoredo(repo) _delundoredo(repo)
except error.LockUnavailable:# no write permissions
repo.ui.debug("undolog lacks write permission\n")
quarkUnsubmitted
Not Done

\n

quark: `\n`
except error.LockHeld:# timeout, not fatal: don't abort actual command
# TODO: log to Scuba. This shouldn't happen too often as it will
# create gaps in the undo log
repo.ui.debug("undolog lock timeout\n")
return changes return changes
def lighttransaction(repo): def lighttransaction(repo):
# full fledged transactions have two serious issues: # full fledged transactions have two serious issues:
# 1. they may cause infite loops through hooks # 1. they may cause infite loops through hooks
# that run commands # that run commands
# 2. they are really expensive performance wise # 2. they are really expensive performance wise
# #