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

[Augie Fackler]

On Tue, Mar 10, 2015 at 09:56:50PM -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 5cab671ca5e7346e25b7d1a98cd9bdb1f4f25894
> # Parent  fb5fc276e7112b1d2b849d42d3502103e95a6d6a
> devel: also warn about transaction started without a lock

I don't disagree with Ryan's comments, but wanted to say that I
support this series. I've been meaning to get a buildbot running with
the lock checker enabled for a while, and this is probably a better
solution.

>
> 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
> @@ -896,10 +896,18 @@ class localrepository(object):
>          if tr and tr.running():
>              return tr
>          return None
>
>      def transaction(self, desc, report=None):
> +        if self.ui.develflag('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, 2)
> +                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,14 +23,16 @@
>    > 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"
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel