Patchwork [1,of,3,V3] journal: add dirstate tracking

login
register
mail settings
Submitter Martijn Pieters
Date July 7, 2016, 5 p.m.
Message ID <1f282c1e1df71e1a433d.1467910805@mjpieters-mbp.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/15768/
State Superseded
Delegated to: Yuya Nishihara
Headers show

Comments

Martijn Pieters - July 7, 2016, 5 p.m.
# HG changeset patch
# User Martijn Pieters <mjpieters@fb.com>
# Date 1467910735 -3600
#      Thu Jul 07 17:58:55 2016 +0100
# Node ID 1f282c1e1df71e1a433df354000feed5c06ad625
# Parent  6a98f9408a504be455d4382801610daceac429e6
journal: add dirstate tracking

Note that now the default action for `hg journal` is to list the working copy
history, not all bookmarks. In its place is the `--all` switch which lists all
name changes recorded, including the name for which the change was recorded on
each line.
Yuya Nishihara - July 8, 2016, 3:03 p.m.
On Thu, 07 Jul 2016 18:00:05 +0100, Martijn Pieters wrote:
> # HG changeset patch
> # User Martijn Pieters <mjpieters@fb.com>
> # Date 1467910735 -3600
> #      Thu Jul 07 17:58:55 2016 +0100
> # Node ID 1f282c1e1df71e1a433df354000feed5c06ad625
> # Parent  6a98f9408a504be455d4382801610daceac429e6
> journal: add dirstate tracking
> 
> Note that now the default action for `hg journal` is to list the working copy
> history, not all bookmarks. In its place is the `--all` switch which lists all
> name changes recorded, including the name for which the change was recorded on
> each line.

The code looks good to me, but can you update the commit message to mention
why jlock() is introduced? It will help future readers who won't remember the
discussion in the previous thread.

>          fm.condwrite(ui.verbose, 'oldhashes', '%s -> ', oldhashesstr)
>          fm.write('newhashes', '%s', newhashesstr)
>          fm.condwrite(ui.verbose, 'user', ' %s', entry.user.ljust(8))
> +        fm.condwrite(opts.get('all'), 'name', '  %s', entry.name.ljust(8))

It isn't a problem of this patch, but these ljust()s should be replaced by
'%-8s' so that JSON outputs include no padding.
Martijn Pieters - July 11, 2016, 1:04 p.m.
On 8 July 2016 at 16:03, Yuya Nishihara <yuya@tcha.org> wrote:
> The code looks good to me, but can you update the commit message to mention
> why jlock() is introduced? It will help future readers who won't remember the
> discussion in the previous thread.

Done for V4.

>>          fm.condwrite(ui.verbose, 'oldhashes', '%s -> ', oldhashesstr)
>>          fm.write('newhashes', '%s', newhashesstr)
>>          fm.condwrite(ui.verbose, 'user', ' %s', entry.user.ljust(8))
>> +        fm.condwrite(opts.get('all'), 'name', '  %s', entry.name.ljust(8))
>
> It isn't a problem of this patch, but these ljust()s should be replaced by
> '%-8s' so that JSON outputs include no padding.

Good catch, V4 will include a JSON test and no ljust.

Patch

diff --git a/hgext/journal.py b/hgext/journal.py
--- a/hgext/journal.py
+++ b/hgext/journal.py
@@ -15,6 +15,7 @@ 
 
 import collections
 import os
+import weakref
 
 from mercurial.i18n import _
 
@@ -22,9 +23,12 @@ 
     bookmarks,
     cmdutil,
     commands,
+    dirstate,
     dispatch,
     error,
     extensions,
+    localrepo,
+    lock,
     node,
     util,
 )
@@ -43,11 +47,16 @@ 
 
 # namespaces
 bookmarktype = 'bookmark'
+wdirparenttype = 'wdirparent'
 
 # Journal recording, register hooks and storage object
 def extsetup(ui):
     extensions.wrapfunction(dispatch, 'runcommand', runcommand)
     extensions.wrapfunction(bookmarks.bmstore, '_write', recordbookmarks)
+    extensions.wrapfunction(
+        dirstate.dirstate, '_writedirstate', recorddirstateparents)
+    extensions.wrapfunction(
+        localrepo.localrepository.dirstate, 'func', wrapdirstate)
 
 def reposetup(ui, repo):
     if repo.local():
@@ -58,6 +67,42 @@ 
     journalstorage.recordcommand(*fullargs)
     return orig(lui, repo, cmd, fullargs, *args)
 
+# hooks to record dirstate changes
+def wrapdirstate(orig, repo):
+    """Make journal storage available to the dirstate object"""
+    dirstate = orig(repo)
+    if util.safehasattr(repo, 'journal'):
+        dirstate.journalstorage = repo.journal
+    return dirstate
+
+def recorddirstateparents(orig, dirstate, dirstatefp):
+    """Records all dirstate parent changes in the journal."""
+    if util.safehasattr(dirstate, 'journalstorage'):
+        old = [node.nullid, node.nullid]
+        nodesize = len(node.nullid)
+        try:
+            # The only source for the old state is in the dirstate file still
+            # on disk; the in-memory dirstate object only contains the new
+            # state. dirstate._opendirstatefile() switches beteen .hg/dirstate
+            # and .hg/dirstate.pending depending on the transaction state.
+            with dirstate._opendirstatefile() as fp:
+                state = fp.read(2 * nodesize)
+            if len(state) == 2 * nodesize:
+                old = [state[:nodesize], state[nodesize:]]
+        except IOError:
+            pass
+
+        new = dirstate.parents()
+        if old != new:
+            # only record two hashes if there was a merge
+            oldhashes = old[:1] if old[1] == node.nullid else old
+            newhashes = new[:1] if new[1] == node.nullid else new
+            dirstate.journalstorage.record(
+                wdirparenttype, '.', oldhashes, newhashes)
+
+    return orig(dirstate, dirstatefp)
+
+# hooks to record bookmark changes (both local and remote)
 def recordbookmarks(orig, store, fp):
     """Records all bookmark changes in the journal."""
     repo = store._repo
@@ -117,12 +162,17 @@ 
 
     The file format starts with an integer version, delimited by a NUL.
 
+    This storage uses a dedicated lock; this makes it easier to avoid issues
+    with adding entries that added when the regular wlock is unlocked (e.g.
+    the dirstate).
+
     """
     _currentcommand = ()
+    _lockref = None
 
     def __init__(self, repo):
-        self.repo = repo
         self.user = util.getuser()
+        self.ui = repo.ui
         self.vfs = repo.vfs
 
     # track the current command for recording in journal entries
@@ -142,6 +192,24 @@ 
         # with a non-local repo (cloning for example).
         cls._currentcommand = fullargs
 
+    def jlock(self):
+        """Create a lock for the journal file"""
+        if self._lockref and self._lockref():
+            raise error.Abort(_('journal lock does not support nesting'))
+        desc = _('journal of %s') % self.vfs.base
+        try:
+            l = lock.lock(self.vfs, 'journal.lock', 0, desc=desc)
+        except error.LockHeld as inst:
+            self.ui.warn(
+                _("waiting for lock on %s held by %r\n") % (desc, inst.locker))
+            # default to 600 seconds timeout
+            l = lock.lock(
+                self.vfs, 'journal.lock',
+                int(self.ui.config("ui", "timeout", "600")), desc=desc)
+            self.ui.warn(_("got lock after %s seconds\n") % l.delay)
+        self._lockref = weakref.ref(l)
+        return l
+
     def record(self, namespace, name, oldhashes, newhashes):
         """Record a new journal entry
 
@@ -163,7 +231,7 @@ 
             util.makedate(), self.user, self.command, namespace, name,
             oldhashes, newhashes)
 
-        with self.repo.wlock():
+        with self.jlock():
             version = None
             # open file in amend mode to ensure it is created if missing
             with self.vfs('journal', mode='a+b', atomictemp=True) as f:
@@ -176,7 +244,7 @@ 
                     # write anything) if this is not a version we can handle or
                     # the file is corrupt. In future, perhaps rotate the file
                     # instead?
-                    self.repo.ui.warn(
+                    self.ui.warn(
                         _("unsupported journal file version '%s'\n") % version)
                     return
                 if not version:
@@ -229,15 +297,19 @@ 
 _ignoreopts = ('no-merges', 'graph')
 @command(
     'journal', [
+        ('', 'all', None, 'show history for all names'),
         ('c', 'commits', None, 'show commit metadata'),
     ] + [opt for opt in commands.logopts if opt[1] not in _ignoreopts],
     '[OPTION]... [BOOKMARKNAME]')
 def journal(ui, repo, *args, **opts):
-    """show the previous position of bookmarks
+    """show the previous position of bookmarks and the working copy
 
-    The journal is used to see the previous commits of bookmarks. By default
-    the previous locations for all bookmarks are shown.  Passing a bookmark
-    name will show all the previous positions of that bookmark.
+    The journal is used to see the previous commits that bookmarks and the
+    working copy pointed to. By default the previous locations for the working
+    copy.  Passing a bookmark name will show all the previous positions of
+    that bookmark. Use the --all switch to show previous locations for all
+    bookmarks and the working copy; each line will then include the bookmark
+    name, or '.' for the working copy, as well.
 
     By default hg journal only shows the commit hash and the command that was
     running at that time. -v/--verbose will show the prior hash, the user, and
@@ -250,22 +322,27 @@ 
     `hg journal -T json` can be used to produce machine readable output.
 
     """
-    bookmarkname = None
+    name = '.'
+    if opts.get('all'):
+        if args:
+            raise error.Abort(
+                _("You can't combine --all and filtering on a name"))
+        name = None
     if args:
-        bookmarkname = args[0]
+        name = args[0]
 
     fm = ui.formatter('journal', opts)
 
     if opts.get("template") != "json":
-        if bookmarkname is None:
-            name = _('all bookmarks')
+        if name is None:
+            displayname = _('the working copy and bookmarks')
         else:
-            name = "'%s'" % bookmarkname
-        ui.status(_("previous locations of %s:\n") % name)
+            displayname = "'%s'" % name
+        ui.status(_("previous locations of %s:\n") % displayname)
 
     limit = cmdutil.loglimit(opts)
     entry = None
-    for count, entry in enumerate(repo.journal.filtered(name=bookmarkname)):
+    for count, entry in enumerate(repo.journal.filtered(name=name)):
         if count == limit:
             break
         newhashesstr = ','.join([node.short(hash) for hash in entry.newhashes])
@@ -275,6 +352,7 @@ 
         fm.condwrite(ui.verbose, 'oldhashes', '%s -> ', oldhashesstr)
         fm.write('newhashes', '%s', newhashesstr)
         fm.condwrite(ui.verbose, 'user', ' %s', entry.user.ljust(8))
+        fm.condwrite(opts.get('all'), 'name', '  %s', entry.name.ljust(8))
 
         timestring = util.datestr(entry.timestamp, '%Y-%m-%d %H:%M %1%2')
         fm.condwrite(ui.verbose, 'date', ' %s', timestring)
diff --git a/tests/test-journal.t b/tests/test-journal.t
--- a/tests/test-journal.t
+++ b/tests/test-journal.t
@@ -32,22 +32,37 @@ 
 
   $ hg init repo
   $ cd repo
-  $ echo a > a
-  $ hg commit -Aqm a
-  $ echo b > a
-  $ hg commit -Aqm b
-  $ hg up 0
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
 Test empty journal
 
   $ hg journal
-  previous locations of all bookmarks:
+  previous locations of '.':
   no recorded locations
   $ hg journal foo
   previous locations of 'foo':
   no recorded locations
 
+Test that working copy changes are tracked
+
+  $ echo a > a
+  $ hg commit -Aqm a
+  $ hg journal
+  previous locations of '.':
+  cb9a9f314b8b  commit -Aqm a
+  $ echo b > a
+  $ hg commit -Aqm b
+  $ hg journal
+  previous locations of '.':
+  1e6c11564562  commit -Aqm b
+  cb9a9f314b8b  commit -Aqm a
+  $ hg up 0
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg journal
+  previous locations of '.':
+  cb9a9f314b8b  up 0
+  1e6c11564562  commit -Aqm b
+  cb9a9f314b8b  commit -Aqm a
+
 Test that bookmarks are tracked
 
   $ hg book -r tip bar
@@ -68,22 +83,32 @@ 
   cb9a9f314b8b  book -f bar
   1e6c11564562  book -r tip bar
 
-Test that you can list all bookmarks as well as limit the list or filter on them
+Test that bookmarks and working copy tracking is not mixed
+
+  $ hg journal
+  previous locations of '.':
+  1e6c11564562  up
+  cb9a9f314b8b  up 0
+  1e6c11564562  commit -Aqm b
+  cb9a9f314b8b  commit -Aqm a
+
+Test that you can list all entries as well as limit the list or filter on them
 
   $ hg book -r tip baz
-  $ hg journal
-  previous locations of all bookmarks:
-  1e6c11564562  book -r tip baz
+  $ hg journal --all
+  previous locations of the working copy and bookmarks:
+  1e6c11564562  baz       book -r tip baz
+  1e6c11564562  bar       up
+  1e6c11564562  .         up
+  cb9a9f314b8b  bar       book -f bar
+  1e6c11564562  bar       book -r tip bar
+  cb9a9f314b8b  .         up 0
+  1e6c11564562  .         commit -Aqm b
+  cb9a9f314b8b  .         commit -Aqm a
+  $ hg journal --limit 2
+  previous locations of '.':
   1e6c11564562  up
-  cb9a9f314b8b  book -f bar
-  1e6c11564562  book -r tip bar
-  $ hg journal --limit 2
-  previous locations of all bookmarks:
-  1e6c11564562  book -r tip baz
-  1e6c11564562  up
-  $ hg journal baz
-  previous locations of 'baz':
-  1e6c11564562  book -r tip baz
+  cb9a9f314b8b  up 0
   $ hg journal bar
   previous locations of 'bar':
   1e6c11564562  up
@@ -92,26 +117,27 @@ 
   $ hg journal foo
   previous locations of 'foo':
   no recorded locations
+  $ hg journal .
+  previous locations of '.':
+  1e6c11564562  up
+  cb9a9f314b8b  up 0
+  1e6c11564562  commit -Aqm b
+  cb9a9f314b8b  commit -Aqm a
 
 Test that verbose and commit output work
 
-  $ hg journal --verbose
-  previous locations of all bookmarks:
-  000000000000 -> 1e6c11564562 foobar   1970-01-01 00:00 +0000  book -r tip baz
-  cb9a9f314b8b -> 1e6c11564562 foobar   1970-01-01 00:00 +0000  up
-  1e6c11564562 -> cb9a9f314b8b foobar   1970-01-01 00:00 +0000  book -f bar
-  000000000000 -> 1e6c11564562 foobar   1970-01-01 00:00 +0000  book -r tip bar
+  $ hg journal --verbose --all
+  previous locations of the working copy and bookmarks:
+  000000000000 -> 1e6c11564562 foobar    baz      1970-01-01 00:00 +0000  book -r tip baz
+  cb9a9f314b8b -> 1e6c11564562 foobar    bar      1970-01-01 00:00 +0000  up
+  cb9a9f314b8b -> 1e6c11564562 foobar    .        1970-01-01 00:00 +0000  up
+  1e6c11564562 -> cb9a9f314b8b foobar    bar      1970-01-01 00:00 +0000  book -f bar
+  000000000000 -> 1e6c11564562 foobar    bar      1970-01-01 00:00 +0000  book -r tip bar
+  1e6c11564562 -> cb9a9f314b8b foobar    .        1970-01-01 00:00 +0000  up 0
+  cb9a9f314b8b -> 1e6c11564562 foobar    .        1970-01-01 00:00 +0000  commit -Aqm b
+  000000000000 -> cb9a9f314b8b foobar    .        1970-01-01 00:00 +0000  commit -Aqm a
   $ hg journal --commit
-  previous locations of all bookmarks:
-  1e6c11564562  book -r tip baz
-  changeset:   1:1e6c11564562
-  bookmark:    bar
-  bookmark:    baz
-  tag:         tip
-  user:        test
-  date:        Thu Jan 01 00:00:00 1970 +0000
-  summary:     b
-  
+  previous locations of '.':
   1e6c11564562  up
   changeset:   1:1e6c11564562
   bookmark:    bar
@@ -121,13 +147,13 @@ 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     b
   
-  cb9a9f314b8b  book -f bar
+  cb9a9f314b8b  up 0
   changeset:   0:cb9a9f314b8b
   user:        test
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     a
   
-  1e6c11564562  book -r tip bar
+  1e6c11564562  commit -Aqm b
   changeset:   1:1e6c11564562
   bookmark:    bar
   bookmark:    baz
@@ -136,12 +162,18 @@ 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     b
   
+  cb9a9f314b8b  commit -Aqm a
+  changeset:   0:cb9a9f314b8b
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     a
+  
 
 Test for behaviour on unexpected storage version information
 
   $ printf '42\0' > .hg/journal
   $ hg journal
-  previous locations of all bookmarks:
+  previous locations of '.':
   abort: unknown journal file version '42'
   [255]
   $ hg book -r tip doomed