This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4314

setdiscovery: use a revset instead of dagutil.descendantset()
ClosedPublic

Authored by indygreg on Aug 17 2018, 5:30 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG484c9fe570a7: setdiscovery: use a revset instead of dagutil.descendantset()
Summary

This is the only use of descendantset() in the repo.

Strictly speaking, the revset behaves slightly differently than
dagutil. The reason is that dagutil is using revlog.index for
DAG traversal and this data structure isn't aware of visibility /
filtering. So it can operate on revisions it shouldn't operate on.

But our test coverage of this code is pretty comprehensive and
this change causes no tests to fail. So I think we are good.

Also, the revset parser failed to parse %ld:: - %ld::, hence
the use of descendants(). I'm not sure if that is a feature or
a bug.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Aug 17 2018, 5:30 PM
indygreg added a subscriber: yuja.Aug 17 2018, 5:33 PM

@yuja: I thought I'd draw your attention to the potential revset parser bug detailed in the commit message.

Closed by commit rHG484c9fe570a7: setdiscovery: use a revset instead of dagutil.descendantset() (authored by indygreg). · Explain WhyAug 18 2018, 4:06 AM
This revision was automatically updated to reflect the committed changes.
yuja added a comment.Aug 18 2018, 4:33 AM
@yuja: I thought I'd draw your attention to the potential revset parser bug detailed in the commit message

x :: - y :: is ambiguous because :: can be infix/postfix op, and - can
be prefix/infix op. The infix :: just wins as the parser only looks for the
next token (1 look-ahead), and the expression is parsed as (x :: (- y)) ::,
which is unfortunately valid.

Revision Contents
Changeset List

PathPackages
Mmercurial/setdiscovery.py (7 lines)

Diff 10408

mercurial/setdiscovery.py

missing = set() missing = set()
full = False full = False
progress = ui.makeprogress(_('searching'), unit=_('queries')) progress = ui.makeprogress(_('searching'), unit=_('queries'))
while undecided: while undecided:
if sample: if sample:
missinginsample = [n for i, n in enumerate(sample) if not yesno[i]] missinginsample = [n for i, n in enumerate(sample) if not yesno[i]]
missing.update(dag.descendantset(missinginsample, missing))
if missing:
missing.update(local.revs('descendants(%ld) - descendants(%ld)',
missinginsample, missing))
else:
missing.update(local.revs('descendants(%ld)', missinginsample))
undecided.difference_update(missing) undecided.difference_update(missing)
if not undecided: if not undecided:
break break
if full or common.hasbases(): if full or common.hasbases():
if full: if full: