[PATCH 3 of 3 V2] devel: also warn about transaction started without a lock

[Augie Fackler]

On Wed, Mar 18, 2015 at 04:47:57PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at 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 at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel