Re: [PATCH 2 of 3] lock: add ability to store additional metadata on filesytem
On Mon, Mar 20, 2017 at 12:49 AM, Phil Cohen wrote: > @@ -248,6 +253,7 @@ > if not self._parentheld: > try: > self.vfs.unlink(self.f) > +self.vfs.unlink(self.f + '.info') > except OSError: > pass > This unlink ordering introduces a race condition, where one process can delete a .info file owned by another. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 3] lock: add ability to store additional metadata on filesytem
On 3/20/17 7:49 AM, Phil Cohen wrote:
# HG changeset patch
# User Phil Cohen
# 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 .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 .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 ""
+ > 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', '')
+ ('Should have an info file.', '/arbitrary/data')
+ ('Should be deleted.', '')
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 3] lock: add ability to store additional metadata on filesytem
# HG changeset patch
# User Phil Cohen
# 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 .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.
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 .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):
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)
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
# 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 ""
+ > 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', '')
+ ('Should have an info file.', '/arbitrary/data')
+ ('Should be deleted.', '')
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
