Overall, I like the functionality this series adds, but I'm less
convinced on the specific implementation. Adding more stuff to the
junkyard that is the "ui" class doesn't feel awesome, and I'm concerned
with how it will interact with chg and a future direction of ui
immutability.
I have an alternate suggestion for how to achieve this visibility into
what is going on in a repo:
Instead of having this information inside of a lock, what if every
process that started up in a repo tried to create a file in
.hg/processes/ (or something) of the form "info.<pid>". Then you
wouldn't need to pass this data through to the lock command, wouldn't
need to plumb it through the ui class, and combining the lock info with
the processes info would still be straightforward.
This would allow you to touch only the dispatch logic and achieve
something even cooler: you'd be able to see all of the processes running
in a repo (even read processes, as long as the reader has write access
to the repo's .hg/).
Thoughts on this alternate approach?
Some nitpicks inline as well.
On 3/20/17 7:49 AM, Phil Cohen wrote:
# HG changeset patch
# User Phil Cohen <phil...@fb.com>
# Date 1489996027 25200
# Mon Mar 20 00:47:07 2017 -0700
# Node ID f37121209a2bbcde572e986f2b038bf2da7f954a
# Parent 1ca023fb02cbe4747e2b5b625866cfa538cbebd3
localrepo: pass args and command running as store/write lock metadata
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -10,6 +10,7 @@
import errno
import hashlib
import inspect
+import json
import os
import random
import time
@@ -1355,10 +1356,15 @@
if parentenvvar is not None:
parentlock = encoding.environ.get(parentenvvar)
try:
+ metadata = json.dumps({
+ 'cmd': self.ui.commandname,
+ 'args': self.ui.args
+ })
l = lockmod.lock(vfs, lockname, 0, releasefn=releasefn,
acquirefn=acquirefn, desc=desc,
inheritchecker=inheritchecker,
- parentlock=parentlock)
+ parentlock=parentlock,
+ metadata=metadata)
It wasn't clear to me that metadata should be a string and not arbitrary
data that would be "somehow" serialized. It might be good to name (in
the previous patch) the variable "infostring" rather than "metadata" so
that this is more self-documenting.
Alternatively, have the lock code do the JSON serialization, and allow
metadata to be a list of dict of data that the lock will do the
serialization for, if present. That help's guarantee that lock.info will
always contain JSON serialized data.
except error.LockHeld as inst:
if not wait:
raise
diff --git a/tests/test-repo-lock-writes-metadata.t
b/tests/test-repo-lock-writes-metadata.t
new file mode 100644
--- /dev/null
+++ b/tests/test-repo-lock-writes-metadata.t
@@ -0,0 +1,38 @@
+Make a repo
+
+ $ hg init test
+ $ cd test
+ $ echo file > file
+ $ hg commit -Aqm initial
Nit-pick: I'd love some whitespace after the previous test instead of
before the next test.
+Make an extension
+
+ $ cat << EOT > lockinfo.py
+ > from contextlib import nested
+ > from mercurial import (cmdutil, extensions)
+ > 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/wlock.info"), readf(".hg/store/lock.info"))
+ > status("No lock, shouldn't have either file:")
+ > with repo.wlock():
+ > status("Have the write lock:")
+ > with repo.lock():
+ > status("Also have the store lock:")
+ > status("Just the write lock:")
+ > status("Neither:")
+ > EOT
+Test store and write lock
+ $ hg locktest -v --config extensions.x=lockinfo.py
+ ("No lock, shouldn't have either file:", '<not found>', '<not found>')
+ ('Have the write lock:', '{"cmd": "locktest", "args": ["locktest", "-v"]}',
'<not found>')
+ ('Also have the store lock:', '{"cmd": "locktest", "args": ["locktest", "-v"]}', '{"cmd": "locktest",
"args": ["locktest", "-v"]}')
+ ('Just the write lock:', '{"cmd": "locktest", "args": ["locktest", "-v"]}',
'<not found>')
+ ('Neither:', '<not found>', '<not found>')
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
