Patchwork D1733: scmutil: add utility fn to return repo object with user passed revs unhidden

login
register
mail settings
Submitter phabricator
Date Dec. 19, 2017, 12:10 p.m.
Message ID <differential-rev-PHID-DREV-fv7ib3rbzfrhhr62bg2g-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26359/
State Superseded
Headers show

Comments

phabricator - Dec. 19, 2017, 12:10 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  There has been a need for accessing hidden changesets by default without passing
  --hidden. This is currently done using the directaccess extension but is bit
  hacky.
  
  This patch adds a utility function to return a repo object having user passed
  revisions unhidden. This functionality will live behind a
  config option and won't be the default behaviour. There is also a config option
  added by this patch which tells whether we want to unhide only those revisions
  whose hashes are passed or should we consider revisions numbers also.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1733

AFFECTED FILES
  mercurial/configitems.py
  mercurial/scmutil.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 19, 2017, 2:18 p.m.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  Other than that, this one looks good.

INLINE COMMENTS

> scmutil.py:1345
> +
> +    repo = repo.filtered('visible-hidden')
> +    repo.addvisibilityexceptions(revs)

If `visible-` is required, we should probably check the filtername
of the source repo.

> scmutil.py:1346
> +    repo = repo.filtered('visible-hidden')
> +    repo.addvisibilityexceptions(revs)
> +    return repo

And I slightly prefer a single function call that creates new filtered
repository with the given "exception" revs.

Otherwise, we might have to be careful to not keep a stale cache
that gets invalid on `addvisibilityexceptions(revs)`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1733

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 22, 2017, 1:03 p.m.
pulkit added inline comments.

INLINE COMMENTS

> yuja wrote in scmutil.py:1345
> If `visible-` is required, we should probably check the filtername
> of the source repo.

Sorry I don't get you. Check the filtername of the source repo for what?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1733

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 22, 2017, 1:52 p.m.
yuja added inline comments.

INLINE COMMENTS

> pulkit wrote in scmutil.py:1345
> Sorry I don't get you. Check the filtername of the source repo for what?

repo = repo.filtered('served')
  repo = unhidehashlikerevs(repo, ...)
  repo.filtername == 'visible-hidden'

Does it make sense?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1733

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 26, 2017, 1:51 p.m.
yuja added inline comments.

INLINE COMMENTS

> scmutil.py:1322
> +
> +    if not repo.filtername.startswith('visible'):
> +        return repo

It seems better to use an explicit list of filternames,
i.e. `('visible', 'visible-hidden')`. I don't think we should introduce
new naming scheme, 'visible<x>'.

Can you send a followup?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1733

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -1305,3 +1305,78 @@ 
     before it is used, whether or not the convert extension was formally loaded.
     """
     return sink
+
+def unhidehashlikerevs(repo, specs, hiddentype):
+    """parse the user specs and unhide changesets whose hash or revision number
+    is passed.
+
+    hiddentype can be: 1) 'warn': warn while unhiding changesets
+                       2) 'nowarn': don't warn while unhiding changesets
+
+    returns a repo object with the required changesets unhidden
+    """
+    if not repo.filtername or not repo.ui.configbool('experimental',
+                                                     'directaccess'):
+        return repo
+
+    symbols = set()
+    for spec in specs:
+        try:
+            tree = revsetlang.parse(spec)
+        except error.ParseError: # will be reported by scmutil.revrange()
+            continue
+
+        symbols.update(revsetlang.gethashlikesymbols(tree))
+
+    if not symbols:
+        return repo
+
+    revs = _getrevsfromsymbols(repo, symbols)
+
+    if not revs:
+        return repo
+
+    if hiddentype == 'warn':
+        unfi = repo.unfiltered()
+        revstr = _(",").join([pycompat.bytestr(unfi[l]) for l in revs])
+        repo.ui.warn(_("warning: accessing hidden changesets for write "
+                       "operation: %s\n") % revstr)
+
+    repo = repo.filtered('visible-hidden')
+    repo.addvisibilityexceptions(revs)
+    return repo
+
+def _getrevsfromsymbols(repo, symbols):
+    """parse the list of symbols and returns a set of revision numbers of hidden
+    changesets present in symbols"""
+    revs = set()
+    unfi = repo.unfiltered()
+    unficl = unfi.changelog
+    cl = repo.changelog
+    tiprev = len(unficl)
+    pmatch = unficl._partialmatch
+    allowrevnums = repo.ui.configbool('experimental', 'directaccess.revnums')
+    for s in symbols:
+        try:
+            n = int(s)
+            if n <= tiprev:
+                if not allowrevnums:
+                    continue
+                else:
+                    if n not in cl:
+                        revs.add(n)
+                    continue
+        except ValueError:
+            pass
+
+        try:
+            s = pmatch(s)
+        except error.LookupError:
+            s = None
+
+        if s is not None:
+            rev = unficl.rev(s)
+            if rev not in cl:
+                revs.add(rev)
+
+    return revs
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -452,6 +452,12 @@ 
 coreconfigitem('experimental', 'crecordtest',
     default=None,
 )
+coreconfigitem('experimental', 'directaccess',
+    default=False,
+)
+coreconfigitem('experimental', 'directaccess.revnums',
+    default=False,
+)
 coreconfigitem('experimental', 'editortmpinhg',
     default=False,
 )