Patchwork D1612: fsmonitor: remove watchman transaction and working copy change notifications

login
register
mail settings
Submitter phabricator
Date Dec. 7, 2017, 2:54 a.m.
Message ID <differential-rev-PHID-DREV-zqew6dfsw3dpdr5x4b6e-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25985/
State Superseded
Headers show

Comments

phabricator - Dec. 7, 2017, 2:54 a.m.
ekent created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Remove working copy change and transaction notifications. We were relying
  upon callbacks on transaction function. This caused issues with lock ordering.
  A different approach will be adopted in a subsequent commit.

TEST PLAN
  Tested checkout, update and working copy changes with extension enabled.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1612

AFFECTED FILES
  hgext/fsmonitor/__init__.py

CHANGE DETAILS




To: ekent, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 7, 2017, 7:21 p.m.
durin42 added a comment.


  This looks fine to me, assuming I'm right that https://phab.mercurial-scm.org/D1611 and https://phab.mercurial-scm.org/D1612 have swapped commit messages...

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1612

To: ekent, #hg-reviewers
Cc: durin42, mercurial-devel

Patch

diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py
--- a/hgext/fsmonitor/__init__.py
+++ b/hgext/fsmonitor/__init__.py
@@ -161,6 +161,9 @@ 
 configitem('fsmonitor', 'blacklistusers',
     default=list,
 )
+configitem('experimental', 'fsmonitor.transaction_notify',
+    default=False,
+)
 
 # This extension is incompatible with the following blacklisted extensions
 # and will disable itself when encountering one of these:
@@ -656,14 +659,18 @@ 
         self.enter()
 
     def enter(self):
-        # We explicitly need to take a lock here, before we proceed to update
-        # watchman about the update operation, so that we don't race with
-        # some other actor.  merge.update is going to take the wlock almost
-        # immediately anyway, so this is effectively extending the lock
-        # around a couple of short sanity checks.
+        # Make sure we have a wlock prior to sending notifications to watchman.
+        # We don't want to race with other actors. In the update case,
+        # merge.update is going to take the wlock almost immediately. We are
+        # effectively extending the lock around several short sanity checks.
         if self.oldnode is None:
             self.oldnode = self.repo['.'].node()
-        self._lock = self.repo.wlock()
+
+        if self.repo.currentwlock() is None:
+            if util.safehasattr(self.repo, 'wlocknostateupdate'):
+                self._lock = self.repo.wlocknostateupdate()
+            else:
+                self._lock = self.repo.wlock()
         self.need_leave = self._state(
             'state-enter',
             hex(self.oldnode))
@@ -784,4 +791,34 @@ 
                 orig = super(fsmonitorrepo, self).status
                 return overridestatus(orig, self, *args, **kwargs)
 
+            def wlocknostateupdate(self, *args, **kwargs):
+                return super(fsmonitorrepo, self).wlock(*args, **kwargs)
+
+            def wlock(self, *args, **kwargs):
+                l = super(fsmonitorrepo, self).wlock(*args, **kwargs)
+                if not ui.configbool(
+                    "experimental", "fsmonitor.transaction_notify"):
+                    return l
+                if l.held != 1:
+                    return l
+                origrelease = l.releasefn
+
+                def staterelease():
+                    if origrelease:
+                        origrelease()
+                    if l.stateupdate:
+                        l.stateupdate.exit()
+                        l.stateupdate = None
+
+                try:
+                    l.stateupdate = None
+                    l.stateupdate = state_update(self, name="hg.transaction")
+                    l.stateupdate.enter()
+                    l.releasefn = staterelease
+                except Exception as e:
+                    # Swallow any errors; fire and forget
+                    self.ui.log(
+                        'watchman', 'Exception in state update %s\n', e)
+                return l
+
         repo.__class__ = fsmonitorrepo