Patchwork [V2] dirstate: move pure python dirstate packing to pure/parsers.py

login
register
mail settings
Submitter Siddharth Agarwal
Date Feb. 5, 2013, 10:12 p.m.
Message ID <e975281179d27bd46ab6.1360102332@sid0x220>
Download mbox | patch
Permalink /patch/813/
State Accepted
Commit 194e63c1ccb92b3a3afe7ab90b395a0887c87100
Headers show

Comments

Siddharth Agarwal - Feb. 5, 2013, 10:12 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1358495168 28800
# Node ID e975281179d27bd46ab6ebece8bf3c738b481ed5
# Parent  fe95dbd2ce7e4c6794be0500d31cf524b1de2fa0
dirstate: move pure python dirstate packing to pure/parsers.py
Matt Mackall - Feb. 5, 2013, 10:39 p.m.
On Tue, 2013-02-05 at 14:12 -0800, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1358495168 28800
> # Node ID e975281179d27bd46ab6ebece8bf3c738b481ed5
> # Parent  fe95dbd2ce7e4c6794be0500d31cf524b1de2fa0
> dirstate: move pure python dirstate packing to pure/parsers.py

This was intentionally not done when the C code was introduced to avoid
a hard dependence on having the latest set of compiled modules.
Apparently some "operating systems" ship without C compilers and some
even want you to pay extra money for them. Also, it makes bisecting in
the vicinity of the new C code a huge nuisance.

But I guess the C code's been in long enough that it won't be a hassle.
Siddharth Agarwal - Feb. 6, 2013, 12:31 a.m.
On 02/05/2013 02:39 PM, Matt Mackall wrote:
> This was intentionally not done when the C code was introduced to avoid
> a hard dependence on having the latest set of compiled modules.
> Apparently some "operating systems" ship without C compilers and some
> even want you to pay extra money for them. Also, it makes bisecting in
> the vicinity of the new C code a huge nuisance.
>
> But I guess the C code's been in long enough that it won't be a hassle.

Interesting. Why was parse_dirstate moved out then?
Matt Mackall - Feb. 6, 2013, 5:02 p.m.
On Tue, 2013-02-05 at 16:31 -0800, Siddharth Agarwal wrote:
> On 02/05/2013 02:39 PM, Matt Mackall wrote:
> > This was intentionally not done when the C code was introduced to avoid
> > a hard dependence on having the latest set of compiled modules.
> > Apparently some "operating systems" ship without C compilers and some
> > even want you to pay extra money for them. Also, it makes bisecting in
> > the vicinity of the new C code a huge nuisance.
> >
> > But I guess the C code's been in long enough that it won't be a hassle.
> 
> Interesting. Why was parse_dirstate moved out then?

It happened years earlier.
Bryan O'Sullivan - Feb. 8, 2013, 11:34 a.m.
Matt wrote:

> But I guess the C code's been in long enough that it won't be a hassle.
>>
>
Pushed to crew, then.

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -9,10 +9,8 @@  import errno
 from node import nullid
 from i18n import _
 import scmutil, util, ignore, osutil, parsers, encoding
-import struct, os, stat, errno
-import cStringIO
+import os, stat, errno
 
-_format = ">cllll"
 propertycache = util.propertycache
 filecache = scmutil.filecache
 _rangemask = 0x7fffffff
@@ -508,38 +506,7 @@  class dirstate(object):
         # use the modification time of the newly created temporary file as the
         # filesystem's notion of 'now'
         now = util.fstat(st).st_mtime
-        copymap = self._copymap
-        try:
-            finish(parsers.pack_dirstate(self._map, copymap, self._pl, now))
-            return
-        except AttributeError:
-            pass
-
-        now = int(now)
-        cs = cStringIO.StringIO()
-        pack = struct.pack
-        write = cs.write
-        write("".join(self._pl))
-        for f, e in self._map.iteritems():
-            if e[0] == 'n' and e[3] == now:
-                # The file was last modified "simultaneously" with the current
-                # write to dirstate (i.e. within the same second for file-
-                # systems with a granularity of 1 sec). This commonly happens
-                # for at least a couple of files on 'update'.
-                # The user could change the file without changing its size
-                # within the same second. Invalidate the file's stat data in
-                # dirstate, forcing future 'status' calls to compare the
-                # contents of the file. This prevents mistakenly treating such
-                # files as clean.
-                e = (e[0], 0, -1, -1)   # mark entry as 'unset'
-                self._map[f] = e
-
-            if f in copymap:
-                f = "%s\0%s" % (f, copymap[f])
-            e = pack(_format, e[0], e[1], e[2], e[3], len(f))
-            write(e)
-            write(f)
-        finish(cs.getvalue())
+        finish(parsers.pack_dirstate(self._map, self._copymap, self._pl, now))
 
     def _dirignore(self, f):
         if f == '.':
diff --git a/mercurial/parsers.c b/mercurial/parsers.c
--- a/mercurial/parsers.c
+++ b/mercurial/parsers.c
@@ -326,7 +326,8 @@  static PyObject *pack_dirstate(PyObject 
 		if (getintat(v, 3, &mtime) == -1)
 			goto bail;
 		if (*s == 'n' && mtime == (uint32_t)now) {
-			/* See dirstate.py:write for why we do this. */
+			/* See pure/parsers.py:pack_dirstate for why we do
+			 * this. */
 			if (PyDict_SetItem(map, k, dirstate_unset) == -1)
 				goto bail;
 			mode = 0, size = -1, mtime = -1;
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -7,7 +7,7 @@ 
 
 from mercurial.node import bin, nullid
 from mercurial import util
-import struct, zlib
+import struct, zlib, cStringIO
 
 _pack = struct.pack
 _unpack = struct.unpack
@@ -87,3 +87,29 @@  def parse_dirstate(dmap, copymap, st):
             copymap[f] = c
         dmap[f] = e[:4]
     return parents
+
+def pack_dirstate(dmap, copymap, pl, now):
+    now = int(now)
+    cs = cStringIO.StringIO()
+    write = cs.write
+    write("".join(pl))
+    for f, e in dmap.iteritems():
+        if e[0] == 'n' and e[3] == now:
+            # The file was last modified "simultaneously" with the current
+            # write to dirstate (i.e. within the same second for file-
+            # systems with a granularity of 1 sec). This commonly happens
+            # for at least a couple of files on 'update'.
+            # The user could change the file without changing its size
+            # within the same second. Invalidate the file's stat data in
+            # dirstate, forcing future 'status' calls to compare the
+            # contents of the file. This prevents mistakenly treating such
+            # files as clean.
+            e = (e[0], 0, -1, -1)   # mark entry as 'unset'
+            dmap[f] = e
+
+        if f in copymap:
+            f = "%s\0%s" % (f, copymap[f])
+        e = _pack(">cllll", e[0], e[1], e[2], e[3], len(f))
+        write(e)
+        write(f)
+    return cs.getvalue()