| Submitter | Pierre-Yves David |
|---|---|
| Date | March 18, 2015, 11:47 p.m. |
| Message ID | <89890be5e4d784cb2645.1426722477@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/8157/ |
| State | Accepted |
| Commit | 026f8af88e49ae11ef4e5af3acb0c6171c50aaf8 |
| Headers | show |
Comments
On Wed, Mar 18, 2015 at 04:47:57PM -0700, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1426046625 25200 > # Tue Mar 10 21:03:45 2015 -0700 > # Node ID 89890be5e4d784cb264583b5366d852a5a50561f > # Parent 81e2aeedbed4a8b18dc7d8d3f9d6f8d7eb6d0d24 > devel: also warn about transaction started without a lock Looks good to me, queued. Note that you should probably coordinate with Brendan to get at least one builder running with this set, or maybe it should just be on for the testsuite. > > Nobody should even start a transaction on an unlocked repository. If developer > warning are enabled this will be reported. This use the same config as the bad > locking order as this is closely related. > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > --- a/mercurial/localrepo.py > +++ b/mercurial/localrepo.py > @@ -917,10 +917,19 @@ class localrepository(object): > if tr and tr.running(): > return tr > return None > > def transaction(self, desc, report=None): > + if (self.ui.configbool('devel', 'all') > + or self.ui.configbool('devel', 'check-locks')): > + l = self._lockref and self._lockref() > + if l is None or not l.held: > + msg = 'transaction with no lock\n' > + if self.ui.tracebackflag: > + util.debugstacktrace(msg, 1) > + else: > + self.ui.write_err(msg) > tr = self.currenttransaction() > if tr is not None: > return tr.nest() > > # abort here if the journal already exists > diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t > --- a/tests/test-devel-warnings.t > +++ b/tests/test-devel-warnings.t > @@ -8,10 +8,11 @@ > > cmdtable = {} > > command = cmdutil.command(cmdtable) > > > > @command('buggylocking', [], '') > > def buggylocking(ui, repo): > + > tr = repo.transaction('buggy') > > lo = repo.lock() > > wl = repo.wlock() > > EOF > > $ cat << EOF >> $HGRCPATH > @@ -22,19 +23,34 @@ > > EOF > > $ hg init lock-checker > $ cd lock-checker > $ hg buggylocking > + transaction with no lock > "lock" taken before "wlock" > $ cat << EOF >> $HGRCPATH > > [devel] > > all=0 > > check-locks=1 > > EOF > $ hg buggylocking > + transaction with no lock > "lock" taken before "wlock" > $ hg buggylocking --traceback > + transaction with no lock > + at: > + */hg:* in <module> (glob) > + */mercurial/dispatch.py:* in run (glob) > + */mercurial/dispatch.py:* in dispatch (glob) > + */mercurial/dispatch.py:* in _runcatch (glob) > + */mercurial/dispatch.py:* in _dispatch (glob) > + */mercurial/dispatch.py:* in runcommand (glob) > + */mercurial/dispatch.py:* in _runcommand (glob) > + */mercurial/dispatch.py:* in checkargs (glob) > + */mercurial/dispatch.py:* in <lambda> (glob) > + */mercurial/util.py:* in check (glob) > + $TESTTMP/buggylocking.py:* in buggylocking (glob) > "lock" taken before "wlock" > at: > */hg:* in <module> (glob) > */mercurial/dispatch.py:* in run (glob) > */mercurial/dispatch.py:* in dispatch (glob) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -917,10 +917,19 @@ class localrepository(object): if tr and tr.running(): return tr return None def transaction(self, desc, report=None): + if (self.ui.configbool('devel', 'all') + or self.ui.configbool('devel', 'check-locks')): + l = self._lockref and self._lockref() + if l is None or not l.held: + msg = 'transaction with no lock\n' + if self.ui.tracebackflag: + util.debugstacktrace(msg, 1) + else: + self.ui.write_err(msg) tr = self.currenttransaction() if tr is not None: return tr.nest() # abort here if the journal already exists diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t --- a/tests/test-devel-warnings.t +++ b/tests/test-devel-warnings.t @@ -8,10 +8,11 @@ > cmdtable = {} > command = cmdutil.command(cmdtable) > > @command('buggylocking', [], '') > def buggylocking(ui, repo): + > tr = repo.transaction('buggy') > lo = repo.lock() > wl = repo.wlock() > EOF $ cat << EOF >> $HGRCPATH @@ -22,19 +23,34 @@ > EOF $ hg init lock-checker $ cd lock-checker $ hg buggylocking + transaction with no lock "lock" taken before "wlock" $ cat << EOF >> $HGRCPATH > [devel] > all=0 > check-locks=1 > EOF $ hg buggylocking + transaction with no lock "lock" taken before "wlock" $ hg buggylocking --traceback + transaction with no lock + at: + */hg:* in <module> (glob) + */mercurial/dispatch.py:* in run (glob) + */mercurial/dispatch.py:* in dispatch (glob) + */mercurial/dispatch.py:* in _runcatch (glob) + */mercurial/dispatch.py:* in _dispatch (glob) + */mercurial/dispatch.py:* in runcommand (glob) + */mercurial/dispatch.py:* in _runcommand (glob) + */mercurial/dispatch.py:* in checkargs (glob) + */mercurial/dispatch.py:* in <lambda> (glob) + */mercurial/util.py:* in check (glob) + $TESTTMP/buggylocking.py:* in buggylocking (glob) "lock" taken before "wlock" at: */hg:* in <module> (glob) */mercurial/dispatch.py:* in run (glob) */mercurial/dispatch.py:* in dispatch (glob)