Patchwork [1,of,5,mergedriver] mergestate: raise exception if otherctx is accessed but _other isn't set

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 30, 2015, 6:34 p.m.
Message ID <6ea95357d149d5e1b243.1448908491@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11673/
State Accepted
Delegated to: Martin von Zweigbergk
Headers show

Comments

Siddharth Agarwal - Nov. 30, 2015, 6:34 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1448906709 28800
#      Mon Nov 30 10:05:09 2015 -0800
# Node ID 6ea95357d149d5e1b243bc57abd629962dfea072
# Parent  95c37d37acaeb0885a3220939d035649398e81c2
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r 6ea95357d149
mergestate: raise exception if otherctx is accessed but _other isn't set

We don't want to inadvertently return the workingctx (self._repo[None]).
Martin von Zweigbergk - Nov. 30, 2015, 6:45 p.m.
On Mon, Nov 30, 2015 at 10:38 AM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1448906709 28800
> #      Mon Nov 30 10:05:09 2015 -0800
> # Node ID 6ea95357d149d5e1b243bc57abd629962dfea072
> # Parent  95c37d37acaeb0885a3220939d035649398e81c2
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r 6ea95357d149
> mergestate: raise exception if otherctx is accessed but _other isn't set
>

When can that happen?
Siddharth Agarwal - Nov. 30, 2015, 7:29 p.m.
On 11/30/15 10:45, Martin von Zweigbergk wrote:
>
>
> On Mon, Nov 30, 2015 at 10:38 AM Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1448906709 28800
>     #      Mon Nov 30 10:05:09 2015 -0800
>     # Node ID 6ea95357d149d5e1b243bc57abd629962dfea072
>     # Parent  95c37d37acaeb0885a3220939d035649398e81c2
>     # Available At http://42.netv6.net/sid0-wip/hg/
>     #              hg pull http://42.netv6.net/sid0-wip/hg/ -r
>     6ea95357d149
>     mergestate: raise exception if otherctx is accessed but _other
>     isn't set
>
>
> When can that happen?

Not in normal use -- only due to a programming error.

>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Martin von Zweigbergk - Nov. 30, 2015, 7:34 p.m.
On Mon, Nov 30, 2015 at 11:29 AM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/30/15 10:45, Martin von Zweigbergk wrote:
> >
> >
> > On Mon, Nov 30, 2015 at 10:38 AM Siddharth Agarwal <sid0@fb.com
> > <mailto:sid0@fb.com>> wrote:
> >
> >     # HG changeset patch
> >     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
> >     # Date 1448906709 28800
> >     #      Mon Nov 30 10:05:09 2015 -0800
> >     # Node ID 6ea95357d149d5e1b243bc57abd629962dfea072
> >     # Parent  95c37d37acaeb0885a3220939d035649398e81c2
> >     # Available At http://42.netv6.net/sid0-wip/hg/
> >     #              hg pull http://42.netv6.net/sid0-wip/hg/ -r
> >     6ea95357d149
> >     mergestate: raise exception if otherctx is accessed but _other
> >     isn't set
> >
> >
> > When can that happen?
>
> Not in normal use -- only due to a programming error.
>

Wouldn't we normally use an assertion in such cases?
Siddharth Agarwal - Nov. 30, 2015, 9:13 p.m.
On 11/30/15 11:34, Martin von Zweigbergk wrote:
>
>
> On Mon, Nov 30, 2015 at 11:29 AM Siddharth Agarwal 
> <sid@less-broken.com <mailto:sid@less-broken.com>> wrote:
>
>     On 11/30/15 10:45, Martin von Zweigbergk wrote:
>     >
>     >
>     > On Mon, Nov 30, 2015 at 10:38 AM Siddharth Agarwal <sid0@fb.com
>     <mailto:sid0@fb.com>
>     > <mailto:sid0@fb.com <mailto:sid0@fb.com>>> wrote:
>     >
>     >     # HG changeset patch
>     >     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>
>     <mailto:sid0@fb.com <mailto:sid0@fb.com>>>
>     >     # Date 1448906709 28800
>     >     #      Mon Nov 30 10:05:09 2015 -0800
>     >     # Node ID 6ea95357d149d5e1b243bc57abd629962dfea072
>     >     # Parent  95c37d37acaeb0885a3220939d035649398e81c2
>     >     # Available At http://42.netv6.net/sid0-wip/hg/
>     >     #              hg pull http://42.netv6.net/sid0-wip/hg/ -r
>     >     6ea95357d149
>     >     mergestate: raise exception if otherctx is accessed but _other
>     >     isn't set
>     >
>     >
>     > When can that happen?
>
>     Not in normal use -- only due to a programming error.
>
>
> Wouldn't we normally use an assertion in such cases?

Python 'assert' statements don't work in optimized mode, so we tend to 
prefer RuntimeErrors instead. But yes, the idea is similar.
Martin von Zweigbergk - Nov. 30, 2015, 10:09 p.m.
On Mon, Nov 30, 2015 at 1:13 PM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/30/15 11:34, Martin von Zweigbergk wrote:
> >
> >
> > On Mon, Nov 30, 2015 at 11:29 AM Siddharth Agarwal
> > <sid@less-broken.com <mailto:sid@less-broken.com>> wrote:
> >
> >     On 11/30/15 10:45, Martin von Zweigbergk wrote:
> >     >
> >     >
> >     > On Mon, Nov 30, 2015 at 10:38 AM Siddharth Agarwal <sid0@fb.com
> >     <mailto:sid0@fb.com>
> >     > <mailto:sid0@fb.com <mailto:sid0@fb.com>>> wrote:
> >     >
> >     >     # HG changeset patch
> >     >     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>
> >     <mailto:sid0@fb.com <mailto:sid0@fb.com>>>
> >     >     # Date 1448906709 28800
> >     >     #      Mon Nov 30 10:05:09 2015 -0800
> >     >     # Node ID 6ea95357d149d5e1b243bc57abd629962dfea072
> >     >     # Parent  95c37d37acaeb0885a3220939d035649398e81c2
> >     >     # Available At http://42.netv6.net/sid0-wip/hg/
> >     >     #              hg pull http://42.netv6.net/sid0-wip/hg/ -r
> >     >     6ea95357d149
> >     >     mergestate: raise exception if otherctx is accessed but _other
> >     >     isn't set
> >     >
> >     >
> >     > When can that happen?
> >
> >     Not in normal use -- only due to a programming error.
> >
> >
> > Wouldn't we normally use an assertion in such cases?
>
> Python 'assert' statements don't work in optimized mode, so we tend to
> prefer RuntimeErrors instead. But yes, the idea is similar.
>
>
Okay, pushed to the clowncopter!

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -288,6 +288,8 @@  class mergestate(object):
 
     @util.propertycache
     def otherctx(self):
+        if self._other is None:
+            raise RuntimeError("localctx accessed but self._local isn't set")
         return self._repo[self._other]
 
     def active(self):