Patchwork [4,of,4] dirstate: change debugrebuilddirstate --minimal to use dirstate.rebuild

login
register
mail settings
Submitter Christian Delahousse
Date Nov. 12, 2015, 6:17 a.m.
Message ID <abcbc76d870ac9f2a708.1447309078@dev4253.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11365/
State Superseded
Commit 54ace3372f84988786ea4f6e7613fec14bed0660
Headers show

Comments

Christian Delahousse - Nov. 12, 2015, 6:17 a.m.
# HG changeset patch
# User Christian Delahousse <cdelahousse@fb.com>
# Date 1447299615 28800
#      Wed Nov 11 19:40:15 2015 -0800
# Node ID abcbc76d870ac9f2a70816957af4fc6eb227e3b1
# Parent  fc43737c5f8db0b48583bc6553960a9ea25fa3e2
dirstate: change debugrebuilddirstate --minimal to use dirstate.rebuild

When debugrebuilddirstate --minimal is called, rebuilding the dirstate was done
outside of the appropriate rebuild function. This patch makes
debugrebuilddirstate use dirstate.rebuild.

This was done to allow our extension to become aware debugrebuilddirstate
--minimal
Yuya Nishihara - Nov. 13, 2015, 3:12 p.m.
On Wed, 11 Nov 2015 22:17:58 -0800, cdelahousse@fb.com wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1447299615 28800
> #      Wed Nov 11 19:40:15 2015 -0800
> # Node ID abcbc76d870ac9f2a70816957af4fc6eb227e3b1
> # Parent  fc43737c5f8db0b48583bc6553960a9ea25fa3e2
> dirstate: change debugrebuilddirstate --minimal to use dirstate.rebuild
> 
> When debugrebuilddirstate --minimal is called, rebuilding the dirstate was done
> outside of the appropriate rebuild function. This patch makes
> debugrebuilddirstate use dirstate.rebuild.
> 
> This was done to allow our extension to become aware debugrebuilddirstate
> --minimal
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -2881,21 +2881,17 @@
>      wlock = repo.wlock()
>      try:
>          dirstate = repo.dirstate
> -
> +        changedfiles = None
>          # See command doc for what minimal does.
>          if opts.get('minimal'):
> +            manifestfiles = set(ctx.manifest().keys())
>              dirstatefiles = set(dirstate)
> -            ctxfiles = set(ctx.manifest().keys())
> -            for file in (dirstatefiles | ctxfiles):
> -                indirstate = file in dirstatefiles
> -                inctx = file in ctxfiles
> -
> -                if indirstate and not inctx and dirstate[file] != 'a':
> -                    dirstate.drop(file)
> -                elif inctx and not indirstate:
> -                    dirstate.normallookup(file)
> -        else:
> -            dirstate.rebuild(ctx.node(), ctx.manifest())
> +            manifestonly = manifestfiles - dirstatefiles
> +            dsonly = dirstatefiles - manifestfiles
> +            dsnotadded = set(f for f in dsonly if dirstate[f] != 'a')
> +            changedfiles =  manifestonly | dsnotadded
> +
> +        dirstate.rebuild(ctx.node(), ctx.manifest(), changedfiles)

This part looks good to me.

> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -639,17 +639,19 @@
>  
>      def rebuild(self, parent, allfiles, changedfiles=None):
>          if changedfiles is None:
> +            self.clear()
>              changedfiles = allfiles
> -        oldmap = self._map
> -        self.clear()

Can we skip the whole steps of clear(), such as _lastnormaltime = 0 ?

> -        for f in allfiles:
> -            if f not in changedfiles:
> -                self._map[f] = oldmap[f]
> +
> +        for f in changedfiles:
> +            mode = 0o666
> +            if f in allfiles and 'x' in allfiles.flags(f):
> +                mode = 0o777
> +
> +            if f in allfiles:
> +                self._map[f] = dirstatetuple('n', mode, -1, 0)
>              else:
> -                if 'x' in allfiles.flags(f):
> -                    self._map[f] = dirstatetuple('n', 0o777, -1, 0)
> -                else:
> -                    self._map[f] = dirstatetuple('n', 0o666, -1, 0)
> +                del self._map[f]

The change looks logically correct to me, but I got the following error:

  % hg init test
  % cd test
  % touch foo && hg ci -Am 'add foo'
  % hg rm foo && hg ci -m 'remove foo'
  % hg strip --keep 0
  ...
    File "hgext/strip.py", line 207, in stripcmd
      repo.dirstate.rebuild(urev, uctx.manifest(), changedfiles)
    File "mercurial/dirstate.py", line 653, in rebuild
      del self._map[f]
  KeyError: 'foo'

Regards,

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2881,21 +2881,17 @@ 
     wlock = repo.wlock()
     try:
         dirstate = repo.dirstate
-
+        changedfiles = None
         # See command doc for what minimal does.
         if opts.get('minimal'):
+            manifestfiles = set(ctx.manifest().keys())
             dirstatefiles = set(dirstate)
-            ctxfiles = set(ctx.manifest().keys())
-            for file in (dirstatefiles | ctxfiles):
-                indirstate = file in dirstatefiles
-                inctx = file in ctxfiles
-
-                if indirstate and not inctx and dirstate[file] != 'a':
-                    dirstate.drop(file)
-                elif inctx and not indirstate:
-                    dirstate.normallookup(file)
-        else:
-            dirstate.rebuild(ctx.node(), ctx.manifest())
+            manifestonly = manifestfiles - dirstatefiles
+            dsonly = dirstatefiles - manifestfiles
+            dsnotadded = set(f for f in dsonly if dirstate[f] != 'a')
+            changedfiles =  manifestonly | dsnotadded
+
+        dirstate.rebuild(ctx.node(), ctx.manifest(), changedfiles)
     finally:
         wlock.release()
 
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -639,17 +639,19 @@ 
 
     def rebuild(self, parent, allfiles, changedfiles=None):
         if changedfiles is None:
+            self.clear()
             changedfiles = allfiles
-        oldmap = self._map
-        self.clear()
-        for f in allfiles:
-            if f not in changedfiles:
-                self._map[f] = oldmap[f]
+
+        for f in changedfiles:
+            mode = 0o666
+            if f in allfiles and 'x' in allfiles.flags(f):
+                mode = 0o777
+
+            if f in allfiles:
+                self._map[f] = dirstatetuple('n', mode, -1, 0)
             else:
-                if 'x' in allfiles.flags(f):
-                    self._map[f] = dirstatetuple('n', 0o777, -1, 0)
-                else:
-                    self._map[f] = dirstatetuple('n', 0o666, -1, 0)
+                del self._map[f]
+
         self._pl = (parent, nullid)
         self._dirty = True
 
diff --git a/tests/test-rebuildstate.t b/tests/test-rebuildstate.t
--- a/tests/test-rebuildstate.t
+++ b/tests/test-rebuildstate.t
@@ -84,7 +84,7 @@ 
   $ hg debugrebuilddirstate --minimal
   $ hg debugdirstate --nodates
   r   0          0 * bar (glob)
-  n   0         -1 * foo (glob)
+  n 644         -1 * foo (glob)
   a   0         -1 * qux (glob)
   $ hg status -A
   A qux