[PATCH 1 of 5] dirstate: introduce unsureifambig to detect change of a file correctly

[Martin von Zweigbergk]

On Wed, May 27, 2015 at 10:04 AM FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
wrote:

> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1432745859 -32400
> #      Thu May 28 01:57:39 2015 +0900
> # Node ID ab9f120295b59933d1acd72771f01b5fac8d221d
> # Parent  6ac860f700b5cfeda232d5305963047696b869ca
> dirstate: introduce unsureifambig to detect change of a file correctly
>
> To detect change of a file without redundant comparison of file
> content, dirstate recognizes a file as certainly clean, if:
>
>   (1) it is already known as "normal",
>   (2) dirstate entry for it has valid (= not "-1") timestamp, and
>   (3) mode, size and timestamp of it on the filesystem are as same as
>       ones expected in dirstate
>
> This works as expected in many cases, but doesn't in the corner case
> that changing a file keeps mode, size and timestamp of it on the
> filesystem.
>
> Occasional test failure for unexpected file status is typical example
> of this corner case: batch execution with small working directory
> causes that `parsers.pack_dirstate()` replaces timestamp of dirstate
> entries with "-1" in many cases, and condition (2) above is rarely
> satisfied.
>
> This patch adds utility function `unsureifambig()`, which invokes
> `normallookup()` if (A) the file is already known as "normal" and (B)
> size of it is equal to expected one.
>
> Otherwise, `unsureifambig()` does nothing. Keeping expected mode and
> size of the file in dirstate is still useful to detect change of the
> file without comparison of file content, in subsequent processing.
>
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -438,6 +438,20 @@
>          if f in self._copymap:
>              del self._copymap[f]
>
> +    def unsureifambig(self, f, size):
>

I'm still trying to understand this patch, but for now, I just have some
 comments on this name. I spent a few seconds trying to make sense of the
name "unsure if am big", thinking you meant "unsure if is big" and that it
would check for possibly large files. Can we spell out "ambiguous"? Also, I
think the preferred name is "lookup", not "unsure". It does call
"normallookup" after all.


> +        '''Mark a file "possibly dirty", if file size isn't changed
> +
> +        A file ``f`` may be mis-recognized as clean, even if it is
> +        already modified, when mode, ``size`` and timestamp of it on
> +        the filesystem are as same as ones expected in dirstate.
> +
> +        To avoid such ambiguous situation, this function should be
> +        called just after writing data into the working directory.
> +        '''
> +        st = self._map.get(f)
> +        if st and st[0] == 'n' and st[2] == size:
> +            self.normallookup(f)
> +
>      def otherparent(self, f):
>          '''Mark as coming from the other parent, always dirty.'''
>          if self._pl[1] == nullid:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150528/06b9d69e/attachment.html>