( )⚙ D1143 revset: update visibility exceptions with hidden commits from the tree

This is an archive of the discontinued Mercurial Phabricator instance.

revset: update visibility exceptions with hidden commits from the tree
AbandonedPublic

Authored by pulkit on Oct 17 2017, 8:19 AM.

Details

Reviewers
indygreg
yuja
Group Reviewers
hg-reviewers
Summary

This patch adds functionality to update the visibility exceptions set with the
hidden commits whose hashes are passed if the command allow accessing them and
showing warning depending on repo.filtername which is set by the type of
command.

The logic to check whether a symbol is hash and logic related to showing
warning to user is imported from directacess extension from fb-hgext.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Oct 17 2017, 8:19 AM
dlax added a subscriber: dlax.Oct 17 2017, 3:09 PM
indygreg requested changes to this revision.Oct 18 2017, 12:31 PM
indygreg added a subscriber: indygreg.

I'm not going to queue the remainder of this series because I'm unsure what the decisions around this feature are. I will add some nits on the code though.

mercurial/revset.py
2207

Nit: this should be a tuple, not a list. Tuples are immutable and I believe Python will compile it once instead of allocating a new list object every invocation.

2283

I'm not sure if the number of items in symbols is expected to be large, but if it is, aliasing cl._partialmatch to avoid an attribute lookup is a justifiable micro-optimization.

2288

repo.changelog in a loop is bad for perf. Please alias the changelog to a local variable.

2291–2292

Nit: these 2 lines can be combined to avoid extra indentation.

2293–2296

Nit: I think it is better to have the list of hidden changesets after the message, not in the middle of it.

Nit: I /think/ "," should be localized.

Nit: str should probably be replaced with something from pycompat for Python 3 compatibility?

This revision now requires changes to proceed.Oct 18 2017, 12:31 PM
pulkit edited the summary of this revision. (Show Details)Nov 2 2017, 2:05 PM
pulkit retitled this revision from revset: update repo.pinnedrevs with hidden commits from the tree to revset: update visibility exceptions with hidden commits from the tree.
pulkit updated this revision to Diff 3205.
pulkit planned changes to this revision.Nov 22 2017, 8:24 PM
pulkit updated this revision to Diff 4097.Dec 4 2017, 9:17 AM
yuja requested changes to this revision.Dec 6 2017, 9:15 AM
yuja added a subscriber: yuja.

Can't we apply _updateexceptions() out of the revset.match*()?
It's horrible that a revset query does mutate the repo.

# somewhere near scmutil.revrange()
tree = revsetlang.parse(spec)
symbols = revsetlang.hashlikesymbols(tree)
... continue _updateexceptions()
mercurial/revset.py
2246

Perhaps this can be moved to _updateexceptions() phase, so
that we can simply collect hash-like strings without the knowledge
of the tip revision.

2253

Please move this to revsetlang module.

2263

No need to test the length.

2265

We can get rid of it by processing a pre-optimized tree.

2271

Seems tricky. Instead, test the _ishashsymbol() before adding
to the results list?

This revision now requires changes to proceed.Dec 6 2017, 9:15 AM
pulkit abandoned this revision.Feb 14 2018, 11:26 AM