Patchwork [V2] dirstate: fix filefoldmap incosistency on file delete

login
register
mail settings
Submitter Mateusz Kwapich
Date Nov. 6, 2015, 8:32 p.m.
Message ID <91a3d774f808a3aa14c0.1446841923@mitrandir-mbp1.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/11311/
State Superseded
Headers show

Comments

Mateusz Kwapich - Nov. 6, 2015, 8:32 p.m.
# HG changeset patch
# User Mateusz Kwapich <mitrandir@fb.com>
# Date 1446841908 28800
#      Fri Nov 06 12:31:48 2015 -0800
# Node ID 91a3d774f808a3aa14c0786018da0abc1c5e8055
# Parent  f9984f76fd90e439221425d751e29bae17bec995
dirstate: fix filefoldmap incosistency on file delete

The _filefoldmap is not updated in when files are deleted from dirstate. In the
case where the file with the same but differently cased name is added afterwards
it renders _filefoldmap incorrect.  Those steps must occur to for a problem to
reproduce:
 - call status (with listunknown=True),
 - update working rectory to a commit which does a casefolding change (A -> a)
 - call status again (it will show the file "a" as deleted)

Unfortunately I'm unable to write a test for it because I don't know any
core-mercurial command able to reproduce those steps.

The bug was originally spotted when hgwatchman was enabled. It caused the
changeset contents change during hg rebase (one file unrelarted to changeset
was deleted in it after rebase).

The hgwatchman is able to hit it because when hgignore changes the hgwatchmans
overridestatus is calling original status with listunknown=True.
Yuya Nishihara - Nov. 8, 2015, 12:53 p.m.
On Fri, 6 Nov 2015 12:32:03 -0800, Mateusz Kwapich wrote:
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir@fb.com>
> # Date 1446841908 28800
> #      Fri Nov 06 12:31:48 2015 -0800
> # Node ID 91a3d774f808a3aa14c0786018da0abc1c5e8055
> # Parent  f9984f76fd90e439221425d751e29bae17bec995
> dirstate: fix filefoldmap incosistency on file delete
> 
> The _filefoldmap is not updated in when files are deleted from dirstate. In the
> case where the file with the same but differently cased name is added afterwards
> it renders _filefoldmap incorrect.  Those steps must occur to for a problem to
> reproduce:
>  - call status (with listunknown=True),
>  - update working rectory to a commit which does a casefolding change (A -> a)
>  - call status again (it will show the file "a" as deleted)
> 
> Unfortunately I'm unable to write a test for it because I don't know any
> core-mercurial command able to reproduce those steps.
> 
> The bug was originally spotted when hgwatchman was enabled. It caused the
> changeset contents change during hg rebase (one file unrelarted to changeset
> was deleted in it after rebase).
> 
> The hgwatchman is able to hit it because when hgignore changes the hgwatchmans
> overridestatus is calling original status with listunknown=True.
> 
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -536,6 +536,11 @@
>          if size == 0 and f in self._copymap:
>              del self._copymap[f]
>  
> +        if "_filefoldmap" in self.__dict__:
> +            normed = util.normcase(f)
> +            if normed in self._filefoldmap:
> +                del self._filefoldmap[normed]
> +
>      def merge(self, f):
>          '''Mark a file merged.'''
>          if self._pl[1] == nullid:
> @@ -549,6 +554,11 @@
>              self._droppath(f)
>              del self._map[f]
>  
> +        if "_filefoldmap" in self.__dict__:
> +            normed = util.normcase(f)
> +            if normed in self._filefoldmap:
> +                del self._filefoldmap[normed]

Looks good, but perhaps we can move them to _droppath(f) ?

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -536,6 +536,11 @@ 
         if size == 0 and f in self._copymap:
             del self._copymap[f]
 
+        if "_filefoldmap" in self.__dict__:
+            normed = util.normcase(f)
+            if normed in self._filefoldmap:
+                del self._filefoldmap[normed]
+
     def merge(self, f):
         '''Mark a file merged.'''
         if self._pl[1] == nullid:
@@ -549,6 +554,11 @@ 
             self._droppath(f)
             del self._map[f]
 
+        if "_filefoldmap" in self.__dict__:
+            normed = util.normcase(f)
+            if normed in self._filefoldmap:
+                del self._filefoldmap[normed]
+
     def _discoverpath(self, path, normed, ignoremissing, exists, storemap):
         if exists is None:
             exists = os.path.lexists(os.path.join(self._root, path))