Patchwork D3189: context: extract partial nodeid lookup method to scmutil

login
register
mail settings
Submitter phabricator
Date April 8, 2018, 4:03 p.m.
Message ID <differential-rev-PHID-DREV-owkjc55vmrarao6wl3ku-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30569/
State Superseded
Headers show

Comments

phabricator - April 8, 2018, 4:03 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We will add another caller soon, and there's a non-obvious reason to
  use the unfiltered repo that we don't want to copy across the code
  base.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/context.py
  mercurial/scmutil.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - April 9, 2018, 1:57 p.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> scmutil.py:439
> +    # This matches the "shortest" template function.
> +    return repo.unfiltered().changelog._partialmatch(prefix)
> +

We'll need to verify that the returned node is NOT hidden by e.g.
calling `changelog.rev(node)`.

It's unlikely that callers expect this function may return a hidden
node.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - April 10, 2018, 2 p.m.
martinvonz added inline comments.

INLINE COMMENTS

> yuja wrote in scmutil.py:439
> We'll need to verify that the returned node is NOT hidden by e.g.
> calling `changelog.rev(node)`.
> 
> It's unlikely that callers expect this function may return a hidden
> node.

Makes sense. Okay if I do that in a follow-up so the rest of the stack is not blocked by this (I will be away until Friday)?

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - April 10, 2018, 3:07 p.m.
yuja added inline comments.

INLINE COMMENTS

> martinvonz wrote in scmutil.py:439
> Makes sense. Okay if I do that in a follow-up so the rest of the stack is not blocked by this (I will be away until Friday)?

I could add it if you're happy with the following change.

  node = repo.unfiltered().changelog._partialmatch(prefix)
  repo.changelog.rev(node)  # make sure node isn't filtered
  return node

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - April 10, 2018, 3:35 p.m.
martinvonz added inline comments.

INLINE COMMENTS

> yuja wrote in scmutil.py:439
> I could add it if you're happy with the following change.
> 
>   node = repo.unfiltered().changelog._partialmatch(prefix)
>   repo.changelog.rev(node)  # make sure node isn't filtered
>   return node

LGTM. Thank you!

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - April 10, 2018, 4:03 p.m.
yuja added a comment.


  My bad. Added `if node is None: return` and pushed, thanks.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #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
@@ -433,6 +433,11 @@ 
         hexfunc = short
     return '%d:%s' % (rev, hexfunc(node))
 
+def resolvepartialhexnodeid(repo, prefix):
+    # Uses unfiltered repo because it's faster when then prefix is ambiguous/
+    # This matches the "shortest" template function.
+    return repo.unfiltered().changelog._partialmatch(prefix)
+
 def isrevsymbol(repo, symbol):
     try:
         revsymbol(repo, symbol)
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -450,7 +450,7 @@ 
             except KeyError:
                 pass
 
-            self._node = repo.unfiltered().changelog._partialmatch(changeid)
+            self._node = scmutil.resolvepartialhexnodeid(repo, changeid)
             if self._node is not None:
                 self._rev = repo.changelog.rev(self._node)
                 return