Patchwork [evolve-ext-V2] directaccess: fix case of shortened hash containing only digits

login
register
mail settings
Submitter Laurent Charignon
Date Nov. 18, 2015, 9:50 p.m.
Message ID <50efee92c2d61e9d40d2.1447883417@lcharignon-mbp.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/11492/
State Accepted
Headers show

Comments

Laurent Charignon - Nov. 18, 2015, 9:50 p.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1447883246 28800
#      Wed Nov 18 13:47:26 2015 -0800
# Node ID 50efee92c2d61e9d40d24c9105b67a73605aee28
# Parent  48547b4c77defdd17c670b1eb0eb94272edf0207
directaccess: fix case of shortened hash containing only digits

For directaccess, there are four cases for what looks like short hashes made
of digits only:
1 | the hash is a revision number and not a short hash for another revision:
we don't change the visibility

2 | the hash is a revision number and a short hash for another revision:
we don't change the visibility

3 | the hash is not a revision number and not a short hash for another revision:
we don't change the visibility

4 | the hash is not a revision number but is a short hash for another revision:
we make it visible

Before the patch we were not lifting visibility in case number 4. This patch
fixes the issue.

'HG:' are removed.
Ryan McElroy - Nov. 18, 2015, 10:17 p.m.
On 11/18/2015 1:50 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1447883246 28800
> #      Wed Nov 18 13:47:26 2015 -0800
> # Node ID 50efee92c2d61e9d40d24c9105b67a73605aee28
> # Parent  48547b4c77defdd17c670b1eb0eb94272edf0207
> directaccess: fix case of shortened hash containing only digits
>
> For directaccess, there are four cases for what looks like short hashes made
> of digits only:
> 1 | the hash is a revision number and not a short hash for another revision:
> we don't change the visibility
>
> 2 | the hash is a revision number and a short hash for another revision:
> we don't change the visibility
>
> 3 | the hash is not a revision number and not a short hash for another revision:
> we don't change the visibility
>
> 4 | the hash is not a revision number but is a short hash for another revision:
> we make it visible
>
> Before the patch we were not lifting visibility in case number 4. This patch
> fixes the issue.
>
> 'HG:' are removed.

Spurious line?

>
> diff --git a/hgext/directaccess.py b/hgext/directaccess.py
> --- a/hgext/directaccess.py
> +++ b/hgext/directaccess.py
> @@ -131,7 +131,7 @@ hashre = util.re.compile('[0-9a-fA-F]{1,
>   
>   _listtuple = ('symbol', '_list')
>   
> -def gethashsymbols(tree):
> +def gethashsymbols(tree, maxrev):
>       # Returns the list of symbols of the tree that look like hashes
>       # for example for the revset 3::abe3ff it will return ('abe3ff')
>       if not tree:
> @@ -139,8 +139,12 @@ def gethashsymbols(tree):
>   
>       if len(tree) == 2 and tree[0] == "symbol":
>           try:
> -            int(tree[1])
> -            return []
> +            n = int(tree[1])
> +            # This isn't necessarily a rev number, could be a hash prefix
> +            if n > maxrev:
> +                return [tree[1]]
> +            else:
> +                return []
>           except ValueError as e:
>               if hashre.match(tree[1]):
>                   return [tree[1]]
> @@ -155,7 +159,7 @@ def gethashsymbols(tree):
>       elif len(tree) >= 3:
>           results = []
>           for subtree in tree[1:]:
> -            results += gethashsymbols(subtree)
> +            results += gethashsymbols(subtree, maxrev)
>           return results
>       else:
>           return []
> @@ -171,8 +175,8 @@ def _posttreebuilthook(orig, tree, repo)
>       if filternm is not None and filternm.startswith('visible-directaccess'):
>           prelength = len(repo._explicitaccess)
>           accessbefore = set(repo._explicitaccess)
> -        repo.symbols = gethashsymbols(tree)
>           cl = repo.unfiltered().changelog
> +        repo.symbols = gethashsymbols(tree, len(cl))
>           for node in repo.symbols:
>               try:
>                   node = cl._partialmatch(node)
> diff --git a/tests/test-inhibit.t b/tests/test-inhibit.t
> --- a/tests/test-inhibit.t
> +++ b/tests/test-inhibit.t
> @@ -525,8 +525,14 @@ Check that rebasing a commit twice makes
>     |/
>     o  14:d66ccb8c5871 add cL
>     |
> -  $ hg strip -r 104eed5354c7
> -  1 changesets pruned
> +  $ hg strip -r 210589181b14
> +  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  working directory now at d66ccb8c5871
> +  2 changesets pruned
> +
> +Using a hash prefix solely made of digits should work
> +  $ hg update 210589181
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>     $ hg rebase -d 18 -r 16 --keep
>     rebasing 16:a438c045eb37 "add cN"
>     $ hg log -r 14:: -G
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Nov. 18, 2015, 10:19 p.m.
On 11/18/2015 01:50 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1447883246 28800
> #      Wed Nov 18 13:47:26 2015 -0800
> # Node ID 50efee92c2d61e9d40d24c9105b67a73605aee28
> # Parent  48547b4c77defdd17c670b1eb0eb94272edf0207
> directaccess: fix case of shortened hash containing only digits

That one is pushed to main, thanks.

> For directaccess, there are four cases for what looks like short hashes made
> of digits only:
> 1 | the hash is a revision number and not a short hash for another revision:
> we don't change the visibility
>
> 2 | the hash is a revision number and a short hash for another revision:
> we don't change the visibility

hopefully, our shortest hash algorithm is not exposing our user to these.


> 'HG:' are removed.

;-)

Patch

diff --git a/hgext/directaccess.py b/hgext/directaccess.py
--- a/hgext/directaccess.py
+++ b/hgext/directaccess.py
@@ -131,7 +131,7 @@  hashre = util.re.compile('[0-9a-fA-F]{1,
 
 _listtuple = ('symbol', '_list')
 
-def gethashsymbols(tree):
+def gethashsymbols(tree, maxrev):
     # Returns the list of symbols of the tree that look like hashes
     # for example for the revset 3::abe3ff it will return ('abe3ff')
     if not tree:
@@ -139,8 +139,12 @@  def gethashsymbols(tree):
 
     if len(tree) == 2 and tree[0] == "symbol":
         try:
-            int(tree[1])
-            return []
+            n = int(tree[1])
+            # This isn't necessarily a rev number, could be a hash prefix
+            if n > maxrev:
+                return [tree[1]]
+            else:
+                return []
         except ValueError as e:
             if hashre.match(tree[1]):
                 return [tree[1]]
@@ -155,7 +159,7 @@  def gethashsymbols(tree):
     elif len(tree) >= 3:
         results = []
         for subtree in tree[1:]:
-            results += gethashsymbols(subtree)
+            results += gethashsymbols(subtree, maxrev)
         return results
     else:
         return []
@@ -171,8 +175,8 @@  def _posttreebuilthook(orig, tree, repo)
     if filternm is not None and filternm.startswith('visible-directaccess'):
         prelength = len(repo._explicitaccess)
         accessbefore = set(repo._explicitaccess)
-        repo.symbols = gethashsymbols(tree)
         cl = repo.unfiltered().changelog
+        repo.symbols = gethashsymbols(tree, len(cl))
         for node in repo.symbols:
             try:
                 node = cl._partialmatch(node)
diff --git a/tests/test-inhibit.t b/tests/test-inhibit.t
--- a/tests/test-inhibit.t
+++ b/tests/test-inhibit.t
@@ -525,8 +525,14 @@  Check that rebasing a commit twice makes
   |/
   o  14:d66ccb8c5871 add cL
   |
-  $ hg strip -r 104eed5354c7
-  1 changesets pruned
+  $ hg strip -r 210589181b14
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  working directory now at d66ccb8c5871
+  2 changesets pruned
+
+Using a hash prefix solely made of digits should work
+  $ hg update 210589181
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg rebase -d 18 -r 16 --keep
   rebasing 16:a438c045eb37 "add cN"
   $ hg log -r 14:: -G