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

[Pierre-Yves David]

On 03/08/2016 08:50 PM, Tuli Uchitel wrote:
> # HG changeset patch
> # User Tuli Uchitel <tuli at 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 at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David