This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Details

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.

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.