Patchwork [3,of,4] lock message: write lock description to a file

login
register
mail settings
Submitter Tuli Uchitel
Date March 8, 2016, 8:50 p.m.
Message ID <d3da9ce4c07018109275.1457470216@yous-iphone.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/13694/
State Changes Requested
Delegated to: Pierre-Yves David
Headers show

Comments

Tuli Uchitel - March 8, 2016, 8:50 p.m.
# HG changeset patch
# User Tuli Uchitel <tuli@fb.com>
# Date 1457463953 0
#      Tue Mar 08 19:05:53 2016 +0000
# Branch stable
# Node ID d3da9ce4c07018109275c91e41b2c33231449104
# Parent  1ffef024963c56879289caf534575d0c72f4b7e9
lock message: write lock description to a file

implement a method that writes a message to a special file associated with the lock and use it for auditing the case when the user opens an editor session when required
Pierre-Yves David - March 10, 2016, 3:06 p.m.
On 03/08/2016 08:50 PM, Tuli Uchitel wrote:
> # HG changeset patch
> # User Tuli Uchitel <tuli@fb.com>
> # Date 1457463953 0
> #      Tue Mar 08 19:05:53 2016 +0000
> # Branch stable
> # Node ID d3da9ce4c07018109275c91e41b2c33231449104
> # Parent  1ffef024963c56879289caf534575d0c72f4b7e9
> lock message: write lock description to a file
>
> implement a method that writes a message to a special file associated with the lock and use it for auditing the case when the user opens an editor session when required

Please wrap your patch description to 80 char.

> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2717,6 +2717,13 @@
>       repo.dirstate.write(tr)
>       pending = tr and tr.writepending() and repo.root
>
> +    # Trace user requested to open an editor session
> +    currentwlock = repo.currentwlock()
> +    if isinstance(currentwlock, lockmod.lock):

Why the isinstance. currentwlock will either give you a lock or None, 
check for None (if currentwlock is not None:).


> +        locklogmessage = _("user {} requested to open a {} session on host {}").format(
> +            ctx.user(), repo.ui.geteditor(), currentwlock.host)

Don't use format, python 3.5 only support "%" on bytes.

I also second timeless question about translation here. But I'm not sure 
what we should do here.

> +        currentwlock.writelocklogmessage(locklogmessage)
> +
>       editortext = repo.ui.edit(committext, ctx.user(), ctx.extra(),
>                           editform=editform, pending=pending)
>       text = re.sub("(?m)^HG:.*(\n|$)", "", editortext)
> diff --git a/mercurial/lock.py b/mercurial/lock.py
> --- a/mercurial/lock.py
> +++ b/mercurial/lock.py
> @@ -47,6 +47,7 @@
>                    desc=None, inheritchecker=None, parentlock=None):
>           self.vfs = vfs
>           self.f = file
> +        self.lockdescriptionfilename = "%s.description" % self.f

Can we get something a bit more compact as the attribute name?

proposal: descfile

>           self.held = 0
>           self.timeout = timeout
>           self.releasefn = releasefn
> @@ -78,6 +79,10 @@
>           # wrapper around os.getpid() to make testing easier
>           return os.getpid()
>
> +    def writelocklogmessage(self, lockdescriptionmessage):

Can we get a docstring?

Why is this called a "log message" I'm not sure what is loggy there.

> +        fp = self.vfs(self.lockdescriptionfilename, mode='wb')
> +        fp.write(lockdescriptionmessage)
> +
>       def lock(self):
>           timeout = self.timeout
>           while True:
> @@ -226,6 +231,7 @@
>                   if not self._parentheld:
>                       try:
>                           self.vfs.unlink(self.f)
> +                        self.vfs.unlink(self.lockdescriptionfilename)
>                       except OSError:
>                           pass
>               # The postrelease functions typically assume the lock is not held
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2717,6 +2717,13 @@ 
     repo.dirstate.write(tr)
     pending = tr and tr.writepending() and repo.root
 
+    # Trace user requested to open an editor session
+    currentwlock = repo.currentwlock()
+    if isinstance(currentwlock, lockmod.lock):
+        locklogmessage = _("user {} requested to open a {} session on host {}").format(
+            ctx.user(), repo.ui.geteditor(), currentwlock.host)
+        currentwlock.writelocklogmessage(locklogmessage)
+
     editortext = repo.ui.edit(committext, ctx.user(), ctx.extra(),
                         editform=editform, pending=pending)
     text = re.sub("(?m)^HG:.*(\n|$)", "", editortext)
diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -47,6 +47,7 @@ 
                  desc=None, inheritchecker=None, parentlock=None):
         self.vfs = vfs
         self.f = file
+        self.lockdescriptionfilename = "%s.description" % self.f
         self.held = 0
         self.timeout = timeout
         self.releasefn = releasefn
@@ -78,6 +79,10 @@ 
         # wrapper around os.getpid() to make testing easier
         return os.getpid()
 
+    def writelocklogmessage(self, lockdescriptionmessage):
+        fp = self.vfs(self.lockdescriptionfilename, mode='wb')
+        fp.write(lockdescriptionmessage)
+
     def lock(self):
         timeout = self.timeout
         while True:
@@ -226,6 +231,7 @@ 
                 if not self._parentheld:
                     try:
                         self.vfs.unlink(self.f)
+                        self.vfs.unlink(self.lockdescriptionfilename)
                     except OSError:
                         pass
             # The postrelease functions typically assume the lock is not held