Patchwork [STABLE] convert: handle LookupError in mercurial_source.lookuprev()

login
register
mail settings
Submitter Matt Harbison
Date Jan. 19, 2015, 3:38 a.m.
Message ID <fc03807767639e86d7b3.1421638730@Envy>
Download mbox | patch
Permalink /patch/7518/
State Accepted
Commit fea3416f2440928921320df11c4cb51ed753197a
Headers show

Comments

Matt Harbison - Jan. 19, 2015, 3:38 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1421637713 18000
#      Sun Jan 18 22:21:53 2015 -0500
# Branch stable
# Node ID fc03807767639e86d7b3b82b9857f8922e2c6d8f
# Parent  74f3ae15cb0e5554633d82ed95262688a7cc774f
convert: handle LookupError in mercurial_source.lookuprev()

This is in line with the documentation on the base class method, and is related
to issue4496 (but doesn't fix the reporter's problem of not mangling other data
that matches a revision pattern).  Now instead of aborting when there is an
ambiguous source rev, it simply won't update the commit comment.  A warning
message might be nice, but a None return masks whether the problem was no
matching revision, or more than one.

The only other caller of this is the logic that converts tags, but those are
never ambiguous since they are always 40 characters.

A test isn't feasible because there simply aren't enough commits in the test
suite repos to have an ambiguous identifier that is at least 6 characters long,
and it would be too easy for the ambiguity to disappear when unrelated changes
are made.  Instead, I simply ran 'hg --traceback log -r c' on the hg repo, and
handled the error it threw.
Yuya Nishihara - Jan. 19, 2015, 11:14 a.m.
On Sun, 18 Jan 2015 22:38:50 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1421637713 18000
> #      Sun Jan 18 22:21:53 2015 -0500
> # Branch stable
> # Node ID fc03807767639e86d7b3b82b9857f8922e2c6d8f
> # Parent  74f3ae15cb0e5554633d82ed95262688a7cc774f
> convert: handle LookupError in mercurial_source.lookuprev()
> 
> This is in line with the documentation on the base class method, and is related
> to issue4496 (but doesn't fix the reporter's problem of not mangling other data
> that matches a revision pattern).  Now instead of aborting when there is an
> ambiguous source rev, it simply won't update the commit comment.  A warning
> message might be nice, but a None return masks whether the problem was no
> matching revision, or more than one.
> 
> The only other caller of this is the logic that converts tags, but those are
> never ambiguous since they are always 40 characters.
> 
> A test isn't feasible because there simply aren't enough commits in the test
> suite repos to have an ambiguous identifier that is at least 6 characters long,
> and it would be too easy for the ambiguity to disappear when unrelated changes
> are made.  Instead, I simply ran 'hg --traceback log -r c' on the hg repo, and
> handled the error it threw.
> 
> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
> --- a/hgext/convert/hg.py
> +++ b/hgext/convert/hg.py
> @@ -467,7 +467,7 @@
>      def lookuprev(self, rev):
>          try:
>              return hex(self.repo.lookup(rev))
> -        except error.RepoError:
> +        except error.RepoError, error.LookupError:

It should be (error.RepoError, error.LookupError).
Matt Harbison - Jan. 20, 2015, 1:24 a.m.
On Mon, 19 Jan 2015 06:14:23 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sun, 18 Jan 2015 22:38:50 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1421637713 18000
>> #      Sun Jan 18 22:21:53 2015 -0500
>> # Branch stable
>> # Node ID fc03807767639e86d7b3b82b9857f8922e2c6d8f
>> # Parent  74f3ae15cb0e5554633d82ed95262688a7cc774f
>> convert: handle LookupError in mercurial_source.lookuprev()
>>
>> This is in line with the documentation on the base class method, and is  
>> related
>> to issue4496 (but doesn't fix the reporter's problem of not mangling  
>> other data
>> that matches a revision pattern).  Now instead of aborting when there  
>> is an
>> ambiguous source rev, it simply won't update the commit comment.  A  
>> warning
>> message might be nice, but a None return masks whether the problem was  
>> no
>> matching revision, or more than one.
>>
>> The only other caller of this is the logic that converts tags, but  
>> those are
>> never ambiguous since they are always 40 characters.
>>
>> A test isn't feasible because there simply aren't enough commits in the  
>> test
>> suite repos to have an ambiguous identifier that is at least 6  
>> characters long,
>> and it would be too easy for the ambiguity to disappear when unrelated  
>> changes
>> are made.  Instead, I simply ran 'hg --traceback log -r c' on the hg  
>> repo, and
>> handled the error it threw.
>>
>> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
>> --- a/hgext/convert/hg.py
>> +++ b/hgext/convert/hg.py
>> @@ -467,7 +467,7 @@
>>      def lookuprev(self, rev):
>>          try:
>>              return hex(self.repo.lookup(rev))
>> -        except error.RepoError:
>> +        except error.RepoError, error.LookupError:
>
> It should be (error.RepoError, error.LookupError).

You're right, thanks.  I'll let whoever queues it fix it in flight.

--Matt
Matt Mackall - Jan. 20, 2015, 10:32 p.m.
On Sun, 2015-01-18 at 22:38 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1421637713 18000
> #      Sun Jan 18 22:21:53 2015 -0500
> # Branch stable
> # Node ID fc03807767639e86d7b3b82b9857f8922e2c6d8f
> # Parent  74f3ae15cb0e5554633d82ed95262688a7cc774f
> convert: handle LookupError in mercurial_source.lookuprev()

Queued for stable with paren fix, thanks.

Patch

diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
--- a/hgext/convert/hg.py
+++ b/hgext/convert/hg.py
@@ -467,7 +467,7 @@ 
     def lookuprev(self, rev):
         try:
             return hex(self.repo.lookup(rev))
-        except error.RepoError:
+        except error.RepoError, error.LookupError:
             return None
 
     def getbookmarks(self):