This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1506

dagop: handle IndexError when using wdir() in dag range
Needs RevisionPublic

Authored by swhitaker on Nov 24 2017, 7:06 AM.

Details

Reviewers
mbthomas
yuja
Group Reviewers
hg-reviewers
Summary

Using wdir() in a dag range revset can crash Mercurial. For example:

hg status --rev '.^::wdir()

revlog.c reports an IndexError in this instance, but it isn't caught
by the calling code. This change adds IndexError to the set of exception
types the calling code catches. When an IndexError is caught, the code
falls back to calling the pure Python implementation of reachableroots,
which fails gracefully.

Test Plan

$ hg status --rev '.::wdir()'
abort: working directory revision cannot be specified

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

swhitaker created this revision.Nov 24 2017, 7:06 AM
mbthomas accepted this revision.Nov 24 2017, 11:31 AM
yuja requested changes to this revision.Nov 24 2017, 10:54 PM
yuja added a subscriber: yuja.

I think it's better to fail fast instead of trying the slow path which is supposed
to raise WdirUnsupported exception.

$ hg status --rev '.::wdir()'
abort: working directory revision cannot be specified

Can you add test?

This revision now requires changes to proceed.Nov 24 2017, 10:54 PM
ryanmce added a subscriber: ryanmce.Nov 26 2017, 4:42 AM

I understand that fixing the crash is a good first step, but ideally, wouldn't this be supported? Is there a reason it's particularly hard to support?

yuja added a comment.Nov 26 2017, 9:01 AM
In D1506#25509, @ryanmce wrote:

but ideally, wouldn't this be supported?

Yeah, this case, includepath=True, won't be difficult to handle. If reachableroots() raised
WdirUnsupportedError, drop wdir revision from y of x::y, add parents instead, rerun
x::(y - wdir() + parents(wdir())), and append wdir to the result.

Revision Contents
Changeset List

PathPackages
Mmercurial/dagop.py (2 lines)

Diff 3813

mercurial/dagop.py

If includepath is True, return (<roots>::<heads>).""" If includepath is True, return (<roots>::<heads>)."""
if not roots: if not roots:
return baseset() return baseset()
minroot = roots.min() minroot = roots.min()
roots = list(roots) roots = list(roots)
heads = list(heads) heads = list(heads)
try: try:
revs = repo.changelog.reachableroots(minroot, heads, roots, includepath) revs = repo.changelog.reachableroots(minroot, heads, roots, includepath)
except AttributeError: except (AttributeError, IndexError):
revs = _reachablerootspure(repo, minroot, roots, heads, includepath) revs = _reachablerootspure(repo, minroot, roots, heads, includepath)
revs = baseset(revs) revs = baseset(revs)
revs.sort() revs.sort()
return revs return revs
def _changesrange(fctx1, fctx2, linerange2, diffopts): def _changesrange(fctx1, fctx2, linerange2, diffopts):
"""Return `(diffinrange, linerange1)` where `diffinrange` is True """Return `(diffinrange, linerange1)` where `diffinrange` is True
if diff from fctx2 to fctx1 has changes in linerange2 and if diff from fctx2 to fctx1 has changes in linerange2 and