| Submitter | Durham Goode |
|---|---|
| Date | Sept. 9, 2014, 8:21 p.m. |
| Message ID | <2328d9edda009ab9ae9d.1410294090@dev2000.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/5758/ |
| State | Accepted |
| Headers | show |
Comments
On Tue, Sep 09, 2014 at 01:21:30PM -0700, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1409942264 25200 > # Fri Sep 05 11:37:44 2014 -0700 > # Node ID 2328d9edda009ab9ae9da8156d96763ff1ef76fc > # Parent af3352972f8fddfb0c5b755e6ceb2470a31e6f7c > dirstate: add exception when calling setparent without begin/end > > Adds an exception when calling dirstate.setparent without having first called > dirstate.beginparentchange. This will prevent people from writing code that > modifies the dirstate parent without considering the transactionality of their > change. > > This will break third party extensions that call setparents. > > diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py > --- a/mercurial/dirstate.py > +++ b/mercurial/dirstate.py > @@ -256,6 +256,10 @@ > > See localrepo.setparents() > """ > + if self._parentwriters == 0: > + raise util.Abort(_("cannot set dirstate parent without " + > + " calling dirstate.begingparentchange")) Hm. Can we make this some non-Abort exception? This is really something that should never happen, and a stacktrace seems warranted (the message here strikes me as something that'd only confuse users anyway). Maybe ValueError for now? Do you mind if I make that change? > + > self._dirty = self._dirtypl = True > oldp2 = self._pl[1] > self._pl = p1, p2 > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 9/10/14, 7:15 PM, "Augie Fackler" <raf@durin42.com> wrote: >On Tue, Sep 09, 2014 at 01:21:30PM -0700, Durham Goode wrote: >> # HG changeset patch >> # User Durham Goode <durham@fb.com> >> # Date 1409942264 25200 >> # Fri Sep 05 11:37:44 2014 -0700 >> # Node ID 2328d9edda009ab9ae9da8156d96763ff1ef76fc >> # Parent af3352972f8fddfb0c5b755e6ceb2470a31e6f7c >> dirstate: add exception when calling setparent without begin/end >> >> Adds an exception when calling dirstate.setparent without having first >>called >> dirstate.beginparentchange. This will prevent people from writing code >>that >> modifies the dirstate parent without considering the transactionality >>of their >> change. >> >> This will break third party extensions that call setparents. >> >> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py >> --- a/mercurial/dirstate.py >> +++ b/mercurial/dirstate.py >> @@ -256,6 +256,10 @@ >> >> See localrepo.setparents() >> """ >> + if self._parentwriters == 0: >> + raise util.Abort(_("cannot set dirstate parent without " + >> + " calling dirstate.begingparentchange")) > >Hm. Can we make this some non-Abort exception? This is really >something that should never happen, and a stacktrace seems warranted >(the message here strikes me as something that'd only confuse users >anyway). Maybe ValueError for now? Do you mind if I make that change? Yea, it’s definitely only there for developers. You can change it. > >> + >> self._dirty = self._dirtypl = True >> oldp2 = self._pl[1] >> self._pl = p1, p2 >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@selenic.com >> http://selenic.com/mailman/listinfo/mercurial-devel >
On Wed, Sep 10, 2014 at 07:23:18PM +0000, Durham Goode wrote: > On 9/10/14, 7:15 PM, "Augie Fackler" <raf@durin42.com> wrote: > > >On Tue, Sep 09, 2014 at 01:21:30PM -0700, Durham Goode wrote: > >> # HG changeset patch > >> # User Durham Goode <durham@fb.com> > >> # Date 1409942264 25200 > >> # Fri Sep 05 11:37:44 2014 -0700 > >> # Node ID 2328d9edda009ab9ae9da8156d96763ff1ef76fc > >> # Parent af3352972f8fddfb0c5b755e6ceb2470a31e6f7c > >> dirstate: add exception when calling setparent without begin/end > >> > >> Adds an exception when calling dirstate.setparent without having first > >>called > >> dirstate.beginparentchange. This will prevent people from writing code > >>that > >> modifies the dirstate parent without considering the transactionality > >>of their > >> change. > >> > >> This will break third party extensions that call setparents. > >> > >> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py > >> --- a/mercurial/dirstate.py > >> +++ b/mercurial/dirstate.py > >> @@ -256,6 +256,10 @@ > >> > >> See localrepo.setparents() > >> """ > >> + if self._parentwriters == 0: > >> + raise util.Abort(_("cannot set dirstate parent without " + > >> + " calling dirstate.begingparentchange")) > > > >Hm. Can we make this some non-Abort exception? This is really > >something that should never happen, and a stacktrace seems warranted > >(the message here strikes me as something that'd only confuse users > >anyway). Maybe ValueError for now? Do you mind if I make that change? > > Yea, it’s definitely only there for developers. You can change it. Excellent. Queued with ValueError instead of Abort. Thanks! > > > > >> + > >> self._dirty = self._dirtypl = True > >> oldp2 = self._pl[1] > >> self._pl = p1, p2 > >> _______________________________________________ > >> Mercurial-devel mailing list > >> Mercurial-devel@selenic.com > >> http://selenic.com/mailman/listinfo/mercurial-devel > > > > >
Patch
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py --- a/mercurial/dirstate.py +++ b/mercurial/dirstate.py @@ -256,6 +256,10 @@ See localrepo.setparents() """ + if self._parentwriters == 0: + raise util.Abort(_("cannot set dirstate parent without " + + " calling dirstate.begingparentchange")) + self._dirty = self._dirtypl = True oldp2 = self._pl[1] self._pl = p1, p2