[PATCH 5 of 6 misc] transplant: assert instead of UnboundLocalError on parentrev in .applied()

Augie Fackler raf at durin42.com
Sun Apr 13 21:36:11 CDT 2014


On Apr 13, 2014, at 1:13 PM, Mads Kiilerich <mads at kiilerich.com> wrote:

> # HG changeset patch
> # User Mads Kiilerich <madski at unity3d.com>
> # Date 1366133616 -7200
> #      Tue Apr 16 19:33:36 2013 +0200
> # Node ID 52bdb665e6938025c57a2d745de1306aa9439a9f
> # Parent  8674a2e7d16fb130b0af9c541c663462a54bc454
> transplant: assert instead of UnboundLocalError on parentrev in .applied()
> 
> There might be implicit reasons it can't happen, but better be more explicit
> and make it fail with a more useful assertion ... if it ever should happen.
> 
> diff --git a/hgext/transplant.py b/hgext/transplant.py
> --- a/hgext/transplant.py
> +++ b/hgext/transplant.py
> @@ -91,10 +91,12 @@ class transplanter(object):
>     def applied(self, repo, node, parent):
>         '''returns True if a node is already an ancestor of parent
>         or is parent or has already been transplanted'''
> +        parentrev = None
>         if hasnode(repo, parent):
>             parentrev = repo.changelog.rev(parent)
>         if hasnode(repo, node):
>             rev = repo.changelog.rev(node)
> +            assert parentrev is not None, (node, parent)

Can you put a more descriptive message in the assertion? Eg "missing parent revision for %r"  %(node, parent) or something?

The message as stated won't help anyone figure out what the invariant we're enforcing is.

>             reachable = repo.changelog.ancestors([parentrev], rev,
>                                                  inclusive=True)
>             if rev in reachable:
> @@ -105,6 +107,7 @@ class transplanter(object):
>                 self.transplants.remove(t)
>                 return False
>             lnoderev = repo.changelog.rev(t.lnode)
> +            assert parentrev is not None, (node, parent)
>             if lnoderev in repo.changelog.ancestors([parentrev], lnoderev,
>                                                     inclusive=True):
>                 return True
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20140413/709caad0/attachment.pgp>


More information about the Mercurial-devel mailing list