Patchwork [1,of,3,manifest-cleanup] manifest: move manifestdict-to-text encoding to manifest class

login
register
mail settings
Submitter Augie Fackler
Date Oct. 9, 2014, 1:08 p.m.
Message ID <0255f395b1cfd4889711.1412860108@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/6164/
State Accepted
Commit bf69cb09a6c96af9319ef175a1c28ea2f04ea306
Headers show

Comments

Augie Fackler - Oct. 9, 2014, 1:08 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1412794050 14400
#      Wed Oct 08 14:47:30 2014 -0400
# Node ID 0255f395b1cfd4889711a727b0ce6fa214eef6b2
# Parent  379c09b0052abc93ebe549d8d97d422f1e1076cf
manifest: move manifestdict-to-text encoding to manifest class

A future patch will introduce a new format, with a new class.
Augie Fackler - Oct. 9, 2014, 1:11 p.m.
I *think* this series is close to being able to start the actual work
on a new manifest format as (loosely) discussed in
https://titanpad.com/mercurial32-manifest-sharding, for anyone curious
where this is going (I wanted feedback on this interface relatively
early, since I'm hoping it needs only minor tweaks and I'd like to
avoid extra churn.)

On Thu, Oct 9, 2014 at 9:08 AM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1412794050 14400
> #      Wed Oct 08 14:47:30 2014 -0400
> # Node ID 0255f395b1cfd4889711a727b0ce6fa214eef6b2
> # Parent  379c09b0052abc93ebe549d8d97d422f1e1076cf
> manifest: move manifestdict-to-text encoding to manifest class
>
> A future patch will introduce a new format, with a new class.
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -40,6 +40,14 @@
>      def flagsdiff(self, d2):
>          return dicthelpers.diff(self._flags, d2._flags, "")
>
> +    def text(self):
> +        fl = sorted(self)
> +        _checkforbidden(fl)
> +
> +        hex, flags = revlog.hex, self.flags
> +        # if this is changed to support newlines in filenames,
> +        # be sure to check the templates/ dir again (especially *-raw.tmpl)
> +        return ''.join("%s\0%s%s\n" % (f, hex(self[f]), flags(f)) for f in fl)
>
>  def _checkforbidden(l):
>      """Check filenames for illegal characters."""
> @@ -220,14 +228,7 @@
>              # just encode a fulltext of the manifest and pass that
>              # through to the revlog layer, and let it handle the delta
>              # process.
> -            files = sorted(map)
> -            _checkforbidden(files)
> -
> -            # if this is changed to support newlines in filenames,
> -            # be sure to check the templates/ dir again (especially *-raw.tmpl)
> -            hex, flags = revlog.hex, map.flags
> -            text = ''.join("%s\0%s%s\n" % (f, hex(map[f]), flags(f))
> -                           for f in files)
> +            text = map.text()
>              arraytext = array.array('c', text)
>              cachedelta = None
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Siddharth Agarwal - Oct. 9, 2014, 5:23 p.m.
On 10/09/2014 06:08 AM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1412794050 14400
> #      Wed Oct 08 14:47:30 2014 -0400
> # Node ID 0255f395b1cfd4889711a727b0ce6fa214eef6b2
> # Parent  379c09b0052abc93ebe549d8d97d422f1e1076cf
> manifest: move manifestdict-to-text encoding to manifest class

The series LGTM.

>
> A future patch will introduce a new format, with a new class.
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -40,6 +40,14 @@
>       def flagsdiff(self, d2):
>           return dicthelpers.diff(self._flags, d2._flags, "")
>   
> +    def text(self):
> +        fl = sorted(self)
> +        _checkforbidden(fl)
> +
> +        hex, flags = revlog.hex, self.flags
> +        # if this is changed to support newlines in filenames,
> +        # be sure to check the templates/ dir again (especially *-raw.tmpl)
> +        return ''.join("%s\0%s%s\n" % (f, hex(self[f]), flags(f)) for f in fl)
>   
>   def _checkforbidden(l):
>       """Check filenames for illegal characters."""
> @@ -220,14 +228,7 @@
>               # just encode a fulltext of the manifest and pass that
>               # through to the revlog layer, and let it handle the delta
>               # process.
> -            files = sorted(map)
> -            _checkforbidden(files)
> -
> -            # if this is changed to support newlines in filenames,
> -            # be sure to check the templates/ dir again (especially *-raw.tmpl)
> -            hex, flags = revlog.hex, map.flags
> -            text = ''.join("%s\0%s%s\n" % (f, hex(map[f]), flags(f))
> -                           for f in files)
> +            text = map.text()
>               arraytext = array.array('c', text)
>               cachedelta = None
>   
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Durham Goode - Oct. 9, 2014, 5:59 p.m.
On 10/9/14, 6:11 AM, "Augie Fackler" <raf@durin42.com> wrote:

>I *think* this series is close to being able to start the actual work
>on a new manifest format as (loosely) discussed in
>https://urldefense.proofpoint.com/v1/url?u=https://titanpad.com/mercurial3
>2-manifest-sharding&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=pHOG6Hz51SkYmYr%2F
>xoTFzw%3D%3D%0A&m=xYookmAlsQsDHc4RJrSTqfyhNrdU2S9sghmbIrJr9Cc%3D%0A&s=36c3
>bd9ed753df24dd916aec5ebbe698a77a340f072a9be88820d1c2e4918037, for anyone
>curious
>where this is going (I wanted feedback on this interface relatively
>early, since I'm hoping it needs only minor tweaks and I'd like to
>avoid extra churn.)

Do you have any document (or list in your head) with what the final
interface might look like after everything is moved?  It¹s hard to get a
feel for the interface when seeing individual functions getting moved
piece by piece.

>
>On Thu, Oct 9, 2014 at 9:08 AM, Augie Fackler <raf@durin42.com> wrote:
>> # HG changeset patch
>> # User Augie Fackler <raf@durin42.com>
>> # Date 1412794050 14400
>> #      Wed Oct 08 14:47:30 2014 -0400
>> # Node ID 0255f395b1cfd4889711a727b0ce6fa214eef6b2
>> # Parent  379c09b0052abc93ebe549d8d97d422f1e1076cf
>> manifest: move manifestdict-to-text encoding to manifest class
>>
>> A future patch will introduce a new format, with a new class.
Augie Fackler - Oct. 9, 2014, 6:12 p.m.
On Thu, Oct 9, 2014 at 1:59 PM, Durham Goode <durham@fb.com> wrote:
>>where this is going (I wanted feedback on this interface relatively
>>early, since I'm hoping it needs only minor tweaks and I'd like to
>>avoid extra churn.)
>
> Do you have any document (or list in your head) with what the final
> interface might look like after everything is moved?  It¹s hard to get a
> feel for the interface when seeing individual functions getting moved
> piece by piece.

So, as far as I can tell, the interface is going to remain the same
from the perspective of most commands. The only tweak I expect in what
I have now is that the .text() and .fastdelta() methods on manifest
classes will need some way to return the expected object hash, so that
can be passed on to the revlog layer, and we might need to do some
clever hash-function-injection into the lower layer as well.

Does that answer your question?
Augie Fackler - Oct. 9, 2014, 6:15 p.m.
On Thu, Oct 9, 2014 at 1:59 PM, Durham Goode <durham@fb.com> wrote:
>>where this is going (I wanted feedback on this interface relatively
>>early, since I'm hoping it needs only minor tweaks and I'd like to
>>avoid extra churn.)
>
> Do you have any document (or list in your head) with what the final
> interface might look like after everything is moved?  It¹s hard to get a
> feel for the interface when seeing individual functions getting moved
> piece by piece.

So, as far as I can tell, the interface is going to remain the same
from the perspective of most commands. The only tweak I expect in what
I have now is that the .text() and .fastdelta() methods on manifest
classes will need some way to return the expected object hash, so that
can be passed on to the revlog layer, and we might need to do some
clever hash-function-injection into the lower layer as well.

Does that answer your question?

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -40,6 +40,14 @@ 
     def flagsdiff(self, d2):
         return dicthelpers.diff(self._flags, d2._flags, "")
 
+    def text(self):
+        fl = sorted(self)
+        _checkforbidden(fl)
+
+        hex, flags = revlog.hex, self.flags
+        # if this is changed to support newlines in filenames,
+        # be sure to check the templates/ dir again (especially *-raw.tmpl)
+        return ''.join("%s\0%s%s\n" % (f, hex(self[f]), flags(f)) for f in fl)
 
 def _checkforbidden(l):
     """Check filenames for illegal characters."""
@@ -220,14 +228,7 @@ 
             # just encode a fulltext of the manifest and pass that
             # through to the revlog layer, and let it handle the delta
             # process.
-            files = sorted(map)
-            _checkforbidden(files)
-
-            # if this is changed to support newlines in filenames,
-            # be sure to check the templates/ dir again (especially *-raw.tmpl)
-            hex, flags = revlog.hex, map.flags
-            text = ''.join("%s\0%s%s\n" % (f, hex(map[f]), flags(f))
-                           for f in files)
+            text = map.text()
             arraytext = array.array('c', text)
             cachedelta = None