| Submitter | Pierre-Yves David |
|---|---|
| Date | March 11, 2015, 4:56 a.m. |
| Message ID | <831c69fcda436381cc27.1426049808@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/8000/ |
| State | Superseded |
| Commit | d6ac30f4edefcf205bf27a3ddf37626922fa8018 |
| Headers | show |
Comments
On 3/10/2015 9:56 PM, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1421405470 28800 > # Fri Jan 16 02:51:10 2015 -0800 > # Node ID 831c69fcda436381cc27a0679c05b71d69184376 > # Parent e9e42adc21a5c0a9bd68079bdd209b7a086ad046 > devel: move the lock-checking code into core > > If the developer warning are enabled, bad locking order will be reported without > the need for the contrib extensions. > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > --- a/mercurial/localrepo.py > +++ b/mercurial/localrepo.py > @@ -1144,10 +1144,18 @@ 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.''' > + if self.ui.develflag('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, 2) > + else: > + self.ui.write_err(msg) Same feedback as on #4: 2 is a magic number, msg is constructed before it's needed and not always used. > l = self._wlockref and self._wlockref() > if l is not None and l.held: > l.lock() > return l > > diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t > new file mode 100644 > --- /dev/null > +++ b/tests/test-devel-warnings.t > @@ -0,0 +1,35 @@ > + > + $ cat << EOF > buggylocking.py > + > """A small extension to acquires lock in the wrong order > + > """ > + > > + > from mercurial import cmdutil > + > > + > cmdtable = {} > + > command = cmdutil.command(cmdtable) > + > > + > @command('buggylocking', [], '') > + > def buggylocking(ui, repo): > + > lo = repo.lock() > + > wl = repo.wlock() > + > EOF > + > + $ cat << EOF >> $HGRCPATH > + > [extensions] > + > buggylocking=$TESTTMP/buggylocking.py > + > [devel] > + > all=1 > + > EOF > + > + $ hg init lock-checker > + $ cd lock-checker > + $ hg buggylocking > + "lock" taken before "wlock" > + $ cat << EOF >> $HGRCPATH > + > [devel] > + > all=0 > + > check-locks=1 > + > EOF > + $ hg buggylocking > + "lock" taken before "wlock" > + $ cd .. Overall, I like this patchset; prefer it with Yuya's and my suggestions.
Patch
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -1144,10 +1144,18 @@ 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.''' + if self.ui.develflag('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, 2) + else: + self.ui.write_err(msg) l = self._wlockref and self._wlockref() if l is not None and l.held: l.lock() return l diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t new file mode 100644 --- /dev/null +++ b/tests/test-devel-warnings.t @@ -0,0 +1,35 @@ + + $ cat << EOF > buggylocking.py + > """A small extension to acquires lock in the wrong order + > """ + > + > from mercurial import cmdutil + > + > cmdtable = {} + > command = cmdutil.command(cmdtable) + > + > @command('buggylocking', [], '') + > def buggylocking(ui, repo): + > lo = repo.lock() + > wl = repo.wlock() + > EOF + + $ cat << EOF >> $HGRCPATH + > [extensions] + > buggylocking=$TESTTMP/buggylocking.py + > [devel] + > all=1 + > EOF + + $ hg init lock-checker + $ cd lock-checker + $ hg buggylocking + "lock" taken before "wlock" + $ cat << EOF >> $HGRCPATH + > [devel] + > all=0 + > check-locks=1 + > EOF + $ hg buggylocking + "lock" taken before "wlock" + $ cd ..