Patchwork [1,of,4] manifest: repurpose flagsdiff() into (node-and-flag)diff()

login
register
mail settings
Submitter Martin von Zweigbergk
Date Oct. 15, 2014, 7:02 a.m.
Message ID <4d6a49d3f8d681b606da.1413356566@handduk2.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/6274/
State Accepted
Headers show

Comments

Martin von Zweigbergk - Oct. 15, 2014, 7:02 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@gmail.com>
# Date 1413331756 25200
#      Tue Oct 14 17:09:16 2014 -0700
# Node ID 4d6a49d3f8d681b606dace0fec7ad6003490c650
# Parent  48c0b101a9de1fdbd638daa858da845cd05a6be7
manifest: repurpose flagsdiff() into (node-and-flag)diff()

The manifestdict class already has a method for diff flags between two
manifests (presumably because there is no full access to the private
_flags field). The only caller is merge.manifestmerge(), which also
wants a diff of files between the same manifests. Let's combine the
code for diffing files and flags into a single method on
manifestdict. This puts all the manifest diffing in one place and will
allow for further simplification. It might also be useful for it to be
encapsulated in manifestdict if we later decide to to shard
manifests. The docstring is intentionally unclear about missing
entries for now.
Augie Fackler - Oct. 15, 2014, 6:21 p.m.
On Wed, Oct 15, 2014 at 12:02:46AM -0700, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@gmail.com>
> # Date 1413331756 25200
> #      Tue Oct 14 17:09:16 2014 -0700
> # Node ID 4d6a49d3f8d681b606dace0fec7ad6003490c650
> # Parent  48c0b101a9de1fdbd638daa858da845cd05a6be7
> manifest: repurpose flagsdiff() into (node-and-flag)diff()

+1. This will make life easier for my manifest refactoring.

>
> The manifestdict class already has a method for diff flags between two
> manifests (presumably because there is no full access to the private
> _flags field). The only caller is merge.manifestmerge(), which also
> wants a diff of files between the same manifests. Let's combine the
> code for diffing files and flags into a single method on
> manifestdict. This puts all the manifest diffing in one place and will
> allow for further simplification. It might also be useful for it to be
> encapsulated in manifestdict if we later decide to to shard
> manifests. The docstring is intentionally unclear about missing
> entries for now.
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -37,8 +37,15 @@
>                  if flags:
>                      ret._flags[fn] = flags
>          return ret
> -    def flagsdiff(self, d2):
> -        return dicthelpers.diff(self._flags, d2._flags, "")
> +
> +    def diff(self, m2):
> +        '''Finds changes between the current manifest and m2. The result is
> +        returned as a dict with filename as key and values of the form
> +        ((n1,n2),(fl1,fl2)), where n1/n2 is the nodeid in the current/other
> +        manifest and fl1/fl2 is the flag in the current/other manifest.'''
> +        flagsdiff = dicthelpers.diff(self._flags, m2._flags, "")
> +        fdiff = dicthelpers.diff(self, m2)
> +        return dicthelpers.join(fdiff, flagsdiff)
>
>      def text(self):
>          fl = sorted(self)
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -10,7 +10,7 @@
>  from node import nullid, nullrev, hex, bin
>  from i18n import _
>  from mercurial import obsolete
> -import error as errormod, util, filemerge, copies, subrepo, worker, dicthelpers
> +import error as errormod, util, filemerge, copies, subrepo, worker
>  import errno, os, shutil
>
>  _pack = struct.pack
> @@ -422,11 +422,9 @@
>
>      aborts = []
>      # Compare manifests
> -    fdiff = dicthelpers.diff(m1, m2)
> -    flagsdiff = m1.flagsdiff(m2)
> -    diff12 = dicthelpers.join(fdiff, flagsdiff)
> +    diff = m1.diff(m2)
>
> -    for f, (n12, fl12) in diff12.iteritems():
> +    for f, (n12, fl12) in diff.iteritems():
>          if n12:
>              n1, n2 = n12
>          else: # file contents didn't change, but flags did

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -37,8 +37,15 @@ 
                 if flags:
                     ret._flags[fn] = flags
         return ret
-    def flagsdiff(self, d2):
-        return dicthelpers.diff(self._flags, d2._flags, "")
+
+    def diff(self, m2):
+        '''Finds changes between the current manifest and m2. The result is
+        returned as a dict with filename as key and values of the form
+        ((n1,n2),(fl1,fl2)), where n1/n2 is the nodeid in the current/other
+        manifest and fl1/fl2 is the flag in the current/other manifest.'''
+        flagsdiff = dicthelpers.diff(self._flags, m2._flags, "")
+        fdiff = dicthelpers.diff(self, m2)
+        return dicthelpers.join(fdiff, flagsdiff)
 
     def text(self):
         fl = sorted(self)
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -10,7 +10,7 @@ 
 from node import nullid, nullrev, hex, bin
 from i18n import _
 from mercurial import obsolete
-import error as errormod, util, filemerge, copies, subrepo, worker, dicthelpers
+import error as errormod, util, filemerge, copies, subrepo, worker
 import errno, os, shutil
 
 _pack = struct.pack
@@ -422,11 +422,9 @@ 
 
     aborts = []
     # Compare manifests
-    fdiff = dicthelpers.diff(m1, m2)
-    flagsdiff = m1.flagsdiff(m2)
-    diff12 = dicthelpers.join(fdiff, flagsdiff)
+    diff = m1.diff(m2)
 
-    for f, (n12, fl12) in diff12.iteritems():
+    for f, (n12, fl12) in diff.iteritems():
         if n12:
             n1, n2 = n12
         else: # file contents didn't change, but flags did