D1606: phases: drop the list with phase of each rev, always comput phase sets
joerg.sonnenberger (Joerg Sonnenberger)
phabricator at mercurial-scm.org
Thu Dec 7 15:38:40 UTC 2017
joerg.sonnenberger added inline comments.
INLINE COMMENTS
> quark wrote in phases.py:226-237
> Sorry - I made a mistake using `repo.revs('public()')` to test the logic here. Actually that exercises a different code path <https://www.mercurial-scm.org/repo/hg/file/aa905f9cdcda/mercurial/revset.py#l1616>.
>
> There are practically no users exercising this `public in phases` code path. So issues like `fullreposet` requires a `repo` argument is not exposed.
>
> I think we can either:
>
> - Pass an optional `subset` here then replace the revset implementation to use it:
>
> def getrevset(self, repo, phases, subset=None):
> if subset is None:
> subset = smartset.fullreposet(repo)
> if ...
> # fast path
> ...
> return subset & smartset.baseset(revs)
> else:
> ...
>
>
>
> - Remove support for calculating `public()` here by just `raise error.ProgrammingError('public() is unsupported')`
>
> I guess the former is preferred eventually. But if you want to land this faster, the latter is easier. I can help writing a follow-up.
I think this can just follow the logic of the other branch, i.e. short cut if `len(phases) == 1 and repo.changelog.filteredrevs`, otherwise, add `repo.changelog.filteredrevs` to the union as well.
I added the assert to make sure that the check wasn't necessary, missed the implicit dependency on repo here.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1606
To: joerg.sonnenberger, #hg-reviewers, quark
Cc: durin42, quark, mercurial-devel
More information about the Mercurial-devel
mailing list