On 3/20/17 7:49 AM, Phil Cohen wrote:
# HG changeset patch
# User Phil Cohen <phil...@fb.com>
# Date 1489995201 25200
#      Mon Mar 20 00:33:21 2017 -0700
# Node ID 1ca023fb02cbe4747e2b5b625866cfa538cbebd3
# Parent  2c1d5a02ec533055f182b0cc1280107fbfb76206
lock: add ability to store additional metadata on filesytem

The metadata is written to <lock>.info while the lock is held. It can be
inspected by outside processes to better understand what the lock-holding
process is doing in cases when the PID alone is not enough.

Some examples might include:

- Getting the original command line `hg` is running when the lockholding process
   was a originally `chg` process.
- Finding out what command an `hg` process is running without having to parse
   its command line yourself.

Excellent summary!


diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -59,8 +59,10 @@
_host = None + # `metadata` is written to a <lockname>.info file while the lock is held
      def __init__(self, vfs, file, timeout=-1, releasefn=None, acquirefn=None,
-                 desc=None, inheritchecker=None, parentlock=None):
+                 desc=None, inheritchecker=None, parentlock=None,
+                 metadata=None):

Case study in function parameter lists slowly growing over time :-)

          self.vfs = vfs
          self.f = file
          self.held = 0
@@ -70,6 +72,7 @@
          self.desc = desc
          self._inheritchecker = inheritchecker
          self.parentlock = parentlock
+        self.metadata = metadata
          self._parentheld = False
          self._inherited = False
          self.postrelease  = []
@@ -128,6 +131,8 @@
              try:
                  self.vfs.makelock(lockname, self.f)
                  self.held = 1
+                if self.metadata is not None:
+                    self.vfs.write(self.f + '.info', self.metadata)

I'd strongly suggest using a constant for the '.info' suffix here.

hg doesn't have too much use of constants yet, but the community seems receptive to adding more, and it would definitely help the codebase maintainability and readability. Others in community, please confirm that I'm remembering this correctly.

I'd suggest a class-level variable that's UPPERCASE.

              except (OSError, IOError) as why:
                  if why.errno == errno.EEXIST:
                      locker = self._readlock()
@@ -248,6 +253,7 @@
                  if not self._parentheld:
                      try:
                          self.vfs.unlink(self.f)
+                        self.vfs.unlink(self.f + '.info')
                      except OSError:
                          pass

Technically, each of these should be wrapped in it's own try/catch, because you don't want to have someone delete the lock manually and then have something look for lock.info and report bad info back.

I have a patch series I'm about to send that introduces "tryunlink" which would be useful for you here.

              # The postrelease functions typically assume the lock is not held
diff --git a/tests/test-lock-meta.t b/tests/test-lock-meta.t
new file mode 100644
--- /dev/null
+++ b/tests/test-lock-meta.t
@@ -0,0 +1,36 @@
+Make a repo
+
+  $ hg init test
+  $ cd test
+  $ echo file > file
+  $ hg commit -Aqm initial
+
+Make an extension
+
+  $ cat << EOT > lockinfo.py
+  > from contextlib import nested
+  > from mercurial import (cmdutil, extensions, lock)
+  > cmdtable = {}
+  > command = cmdutil.command(cmdtable)
+  > @command('locktest')
+  > def locktest(ui, repo, *args, **opts):
+  >   def readf(path):
+  >      try:
+  >        with open(path, "r") as f:
+  >           return f.read()
+  >      except IOError as e:
+  >         return "<not found>"
+  >   def status(str):
+  >     print(str, readf(".hg/customlock.info"))
+  >   with lock.lock(repo.vfs, "customlock"):
+  >       status("No metadata passed; file should not exist")
+  >   with lock.lock(repo.vfs, "customlock", metadata="/arbitrary/data"):
+  >       status("Should have an info file.")
+  >   status("Should be deleted.")
+  > EOT
+
+Test store and write lock
+  $ hg locktest --config extensions.x=lockinfo.py
+  ('No metadata passed; file should not exist', '<not found>')
+  ('Should have an info file.', '/arbitrary/data')
+  ('Should be deleted.', '<not found>')


_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to