| Submitter | Pierre-Yves David |
|---|---|
| Date | April 12, 2015, 5:33 p.m. |
| Message ID | <66b327c773f63f0b9f2b.1428860033@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/8622/ |
| State | Superseded |
| Commit | bedefc611f2564aa2421b99dabb2a9bd7c4c01cc |
| Headers | show |
Comments
On Sun, Apr 12, 2015 at 10:34 AM Pierre-Yves David < pierre-yves.david@ens-lyon.org> wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1428847308 14400 > # Sun Apr 12 10:01:48 2015 -0400 > # Node ID 66b327c773f63f0b9f2b00b2126adbf113f1ca36 > # Parent 52ff737c63d2b2cb41185549aa9c35bc47317032 > wlock: only issue devel warning when actually acquiring the lock > > Before this change, any call to 'wlock' after we acquired a 'lock' was > issuing a > warning. This is wrong is the 'wlock' have been properly acquired before > the > 'lock' in a previous call. > > We move the warning code to only issue such warnings when we are acquiring > the > 'wlock' -after- acquiring 'lock'. This is the expected behavior of this > warning > from the start. > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > --- a/mercurial/localrepo.py > +++ b/mercurial/localrepo.py > @@ -1199,23 +1199,24 @@ class localrepository(object): > > def wlock(self, wait=True): > '''Lock the non-store parts of the repository (everything under > .hg except .hg/store) and return a weak reference to the lock. > Use this before modifying files in .hg.''' > + l = self._wlockref and self._wlockref() > + if l is not None and l.held: > + l.lock() > + return l > + > if (self.ui.configbool('devel', 'all') > or self.ui.configbool('devel', 'check-locks')): > l = self._lockref and self._lockref() > if l is not None and l.held: > msg = '"lock" taken before "wlock"\n' > if self.ui.tracebackflag: > util.debugstacktrace(msg, 1) > else: > self.ui.write_err(msg) > - l = self._wlockref and self._wlockref() > - if l is not None and l.held: > - l.lock() > - return l > > def unlock(): > if self.dirstate.pendingparentchange(): > self.dirstate.invalidate() > else: > 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 > @@ -13,10 +13,26 @@ > > tr = repo.transaction('buggy') > > lo = repo.lock() > > wl = repo.wlock() > > wl.release() > > lo.release() > + > > + > @command('properlocking', [], '') > + > def buggylocking(ui, repo): > Is the name a copy&paste error? > + > """check that reentrance is fine""" > + > wl = repo.wlock() > + > lo = repo.lock() > + > tr = repo.transaction('buggy') > + > tr2 = repo.transaction('buggy') > Here too? We're *not* testing buggy locking, are we? > + > lo2 = repo.lock() > + > wl2 = repo.wlock() > + > wl2.release() > + > lo2.release() > + > tr2.close() > + > tr.close() > + > lo.release() > + > wl.release() > > EOF > > $ cat << EOF >> $HGRCPATH > > [extensions] > > buggylocking=$TESTTMP/buggylocking.py > @@ -62,6 +78,7 @@ > */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) > + $ hg properlocking > $ cd .. > _______________________________________________ > 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 @@ -1199,23 +1199,24 @@ class localrepository(object): def wlock(self, wait=True): '''Lock the non-store parts of the repository (everything under .hg except .hg/store) and return a weak reference to the lock. Use this before modifying files in .hg.''' + l = self._wlockref and self._wlockref() + if l is not None and l.held: + l.lock() + return l + if (self.ui.configbool('devel', 'all') or self.ui.configbool('devel', 'check-locks')): l = self._lockref and self._lockref() if l is not None and l.held: msg = '"lock" taken before "wlock"\n' if self.ui.tracebackflag: util.debugstacktrace(msg, 1) else: self.ui.write_err(msg) - l = self._wlockref and self._wlockref() - if l is not None and l.held: - l.lock() - return l def unlock(): if self.dirstate.pendingparentchange(): self.dirstate.invalidate() else: 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 @@ -13,10 +13,26 @@ > tr = repo.transaction('buggy') > lo = repo.lock() > wl = repo.wlock() > wl.release() > lo.release() + > + > @command('properlocking', [], '') + > def buggylocking(ui, repo): + > """check that reentrance is fine""" + > wl = repo.wlock() + > lo = repo.lock() + > tr = repo.transaction('buggy') + > tr2 = repo.transaction('buggy') + > lo2 = repo.lock() + > wl2 = repo.wlock() + > wl2.release() + > lo2.release() + > tr2.close() + > tr.close() + > lo.release() + > wl.release() > EOF $ cat << EOF >> $HGRCPATH > [extensions] > buggylocking=$TESTTMP/buggylocking.py @@ -62,6 +78,7 @@ */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) + $ hg properlocking $ cd ..