Patchwork [6,of,6,V2] dirstate: read from pending file under HG_PENDING mode if it exists

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Oct. 13, 2015, 5:54 p.m.
Message ID <68fdb7150daad2ea8747.1444758895@feefifofum>
Download mbox | patch
Permalink /patch/11017/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Oct. 13, 2015, 5:54 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1444758557 -32400
#      Wed Oct 14 02:49:17 2015 +0900
# Node ID 68fdb7150daad2ea8747b65145d00acab9be01de
# Parent  f452ff0cefe2755caff781f95bd135038e3d4776
dirstate: read from pending file under HG_PENDING mode if it exists

True/False value of '_pendingmode' means whether 'dirstate.pending' is
used to initialize own '_map' and so on. When it is None, neither
'dirstate' nor 'dirstate.pending' is read in yet.

This is used to keep consistent view between '_pl()' and '_read()'.

Once '_pendingmode' is determined by reading one of 'dirstate' or
'dirstate.pending' in, '_pendingmode' is kept even if 'invalidate()'
is invoked. This should be reasonable, because:

  - effective 'invalidate()' invocation should occur only in wlock scope, and
  - wlock can't be gotten under HG_PENDING mode

'_trypending()' is defined as a normal function to factor similar code
path (in bookmarks and phases) out in the future easily.
Pierre-Yves David - Oct. 13, 2015, 7:30 p.m.
On 10/13/2015 10:54 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1444758557 -32400
> #      Wed Oct 14 02:49:17 2015 +0900
> # Node ID 68fdb7150daad2ea8747b65145d00acab9be01de
> # Parent  f452ff0cefe2755caff781f95bd135038e3d4776
> dirstate: read from pending file under HG_PENDING mode if it exists

This series is pushed to the clowcopter. Does this conclude your 
dirstate run?

Do we have any mechanism to clean up a stalled .pending file? this 
should like something we should guard ourself against.
Katsunori FUJIWARA - Oct. 14, 2015, 8:38 a.m.
At Tue, 13 Oct 2015 12:30:44 -0700,
Pierre-Yves David wrote:
> 
> On 10/13/2015 10:54 AM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1444758557 -32400
> > #      Wed Oct 14 02:49:17 2015 +0900
> > # Node ID 68fdb7150daad2ea8747b65145d00acab9be01de
> > # Parent  f452ff0cefe2755caff781f95bd135038e3d4776
> > dirstate: read from pending file under HG_PENDING mode if it exists
> 
> This series is pushed to the clowcopter. Does this conclude your 
> dirstate run?

Even after this series, there are some situations that in-memory
changes (not only for dirstate, but also for changelog and so on)
aren't visible to external hook/commit log editor process.

I have some patches to make in-memory changes visible to all related
external process (even though they don't change dirstate itself).


> Do we have any mechanism to clean up a stalled .pending file? this 
> should like something we should guard ourself against.

Oh, I overlooked the case that temporary files ('.pending' or so) are
accidentally left.

How about this ?

  (1) check existence of 'journal.backupfiles' before starting
      transaction (maybe in 'transaction.__init__' ?)

  (2) unlink files registered in it, if it exists (= meaning failure
      of cleanup at preceding transaction)

These functions can be easily factored out from 'rollback' and
'_playback' in transaction.py.

BTW, this method requires some additional preparations, because
current unlinking 'journal.backupfiles' logic in transaction.py
prevents this file from being used as "failure of cleanup at preceding
transaction" mark, AFAIK.


> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -36,6 +36,22 @@ 
         os.close(tmpfd)
         vfs.unlink(tmpname)
 
+def _trypending(root, vfs, filename):
+    '''Open  file to be read according to HG_PENDING environment variable
+
+    This opens '.pending' of specified 'filename' only when HG_PENDING
+    is equal to 'root'.
+
+    This returns '(fp, is_pending_opened)' tuple.
+    '''
+    if root == os.environ.get('HG_PENDING'):
+        try:
+            return (vfs('%s.pending' % filename), True)
+        except IOError as inst:
+            if inst.errno != errno.ENOENT:
+                raise
+    return (vfs(filename), False)
+
 class dirstate(object):
 
     def __init__(self, opener, ui, root, validate):
@@ -64,6 +80,9 @@ 
         self._filename = 'dirstate'
         self._pendingfilename = '%s.pending' % self._filename
 
+        # for consitent view between _pl() and _read() invocations
+        self._pendingmode = None
+
     def beginparentchange(self):
         '''Marks the beginning of a set of changes that involve changing
         the dirstate parents. If there is an exception during this time,
@@ -137,7 +156,7 @@ 
     @propertycache
     def _pl(self):
         try:
-            fp = self._opener(self._filename)
+            fp = self._opendirstatefile()
             st = fp.read(40)
             fp.close()
             l = len(st)
@@ -342,11 +361,20 @@ 
             f.discard()
             raise
 
+    def _opendirstatefile(self):
+        fp, mode = _trypending(self._root, self._opener, self._filename)
+        if self._pendingmode is not None and self._pendingmode != mode:
+            fp.close()
+            raise error.Abort(_('working directory state may be '
+                                'changed parallelly'))
+        self._pendingmode = mode
+        return fp
+
     def _read(self):
         self._map = {}
         self._copymap = {}
         try:
-            fp = self._opener.open(self._filename)
+            fp = self._opendirstatefile()
             try:
                 st = fp.read()
             finally: