Patchwork [2,of,3] dirstate: change added/modified placeholder hash length to 20 bytes

login
register
mail settings
Submitter Durham Goode
Date Nov. 10, 2016, 11:01 a.m.
Message ID <9b2f77c847a4dfc187b7.1478775665@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/17433/
State Accepted
Headers show

Comments

Durham Goode - Nov. 10, 2016, 11:01 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1478773156 28800
#      Thu Nov 10 02:19:16 2016 -0800
# Node ID 9b2f77c847a4dfc187b70c84caadebf41e8b248d
# Parent  84d7edbc0712c8c71b5e32c9cc1ee04032b9eb2d
dirstate: change added/modified placeholder hash length to 20 bytes

Previously the added/modified placeholder hash for manifests generated from the
dirstate was a 21byte long string consisting of the p1 file hash plus a single
character to indicate an add or a modify. Normal hashes are only 20 bytes long.
This makes it complicated to implement more efficient manifest implementations
which rely on the hashes being fixed length.

Let's change this hash to just be 20 bytes long, and rely on the astronomical
improbability of an actual hash being these 20 bytes (just like we rely on no
hash every being the nullid).

This changes the possible behavior slightly in that the hash for all
added/modified entries in the dirstate manifest will now be the same (so simple
node comparisons would say they are equal), but we should never be doing simple
node comparisons on these nodes even with the old hashes, because they did not
accurately represent the content (i.e. two files based off the same p1 file
node, with different working copy contents would have the same hash (even with
the appended character) in the old scheme too, so we couldn't depend on the
hashes period).

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -14,8 +14,10 @@  import stat
 
 from .i18n import _
 from .node import (
+    addednodeid,
     bin,
     hex,
+    modifiednodeid,
     newnodeid,
     nullid,
     nullrev,
@@ -1225,23 +1227,13 @@  class committablectx(basectx):
         """
         parents = self.parents()
 
-        man1 = parents[0].manifest()
-        man = man1.copy()
-        if len(parents) > 1:
-            man2 = self.p2().manifest()
-            def getman(f):
-                if f in man1:
-                    return man1
-                return man2
-        else:
-            getman = lambda f: man1
+        man = parents[0].manifest().copy()
 
-        copied = self._repo.dirstate.copies()
         ff = self._flagfunc
-        for i, l in (("a", self._status.added), ("m", self._status.modified)):
+        for i, l in ((addednodeid, self._status.added),
+                     (modifiednodeid, self._status.modified)):
             for f in l:
-                orig = copied.get(f, f)
-                man[f] = getman(orig).get(orig, nullid) + i
+                man[f] = i
                 try:
                     man.setflag(f, ff(f))
                 except OSError:
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -278,7 +278,7 @@  def _makegetfctx(ctx):
         ac = repo.changelog.ancestors(revs, inclusive=True)
         ctx._ancestrycontext = ac
     def makectx(f, n):
-        if len(n) != 20 or n in node.wdirnodes:  # in a working context?
+        if n in node.wdirnodes:  # in a working context?
             if ctx.rev() is None:
                 return ctx.filectx(f)
             return repo[None][f]
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -15,6 +15,7 @@  import struct
 
 from .i18n import _
 from .node import (
+    addednodeid,
     bin,
     hex,
     nullhex,
@@ -873,7 +874,7 @@  def manifestmerge(repo, wctx, p2, pa, br
                     else:
                         actions[f] = ('cd', (f, None, f, False, pa.node()),
                                       "prompt changed/deleted")
-                elif n1[20:] == 'a':
+                elif n1 == addednodeid:
                     # This extra 'a' is added by working copy manifest to mark
                     # the file as locally added. We should forget it instead of
                     # deleting it.
diff --git a/mercurial/node.py b/mercurial/node.py
--- a/mercurial/node.py
+++ b/mercurial/node.py
@@ -20,8 +20,10 @@  nullhex = hex(nullid)
 # Phony node value to stand-in for new files in some uses of
 # manifests.
 newnodeid = '!' * 20
+addednodeid = ('0' * 15) + 'added'
+modifiednodeid = ('0' * 12) + 'modified'
 
-wdirnodes = set((newnodeid,))
+wdirnodes = set((newnodeid, addednodeid, modifiednodeid))
 
 # pseudo identifiers for working directory
 # (they are experimental, so don't add too many dependencies on them)