Patchwork [STABLE] revert: apply normallookup on reverted file if size isn't changed (issue4583)

# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1429884991 -32400
#      Fri Apr 24 23:16:31 2015 +0900
# Branch stable
# Node ID b483fc5e67961924de9ccda7cd7133a24845050a
# Parent  ca1ad8ef38be225caec42564502aafe43cae173d
revert: apply normallookup on reverted file if size isn't changed (issue4583)

Before this patch, reverting a file to the revision other than the
parent doesn't update dirstate. This seems to expect that timestamp
and/or size will be changed by reverting.

But if (1) dirstate of file "f" is filled with timestamp before
reverting and (2) size and timestamp of file "f" isn't changed at
reverting, file "f" is recognized as CLEAN unexpectedly.

This patch applies "dirstate.normallookup()" on reverted file, if size
isn't changed.

Making "localrepository.wwrite()" return length of written data is
needed to avoid additional (and redundant) "lstat(2)" on the reverted
file. "filectx.size()" can't be used to know it, because data may be
decoded at being written out.

BTW, interactive reverting may cause similar problem, too. But this
patch doesn't focus on fixing it, because (1) interactive (maybe slow)
reverting changes one (or both) of size/timestamp of reverted files in
many usecases, and (2) changes to fix it seems not suitable for stable
branch.

Please ignore this. I'll soon post revised one, which examines also
file size to avoid silently breaking test condition.

At Fri, 24 Apr 2015 23:20:43 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1429884991 -32400
> #      Fri Apr 24 23:16:31 2015 +0900
> # Branch stable
> # Node ID b483fc5e67961924de9ccda7cd7133a24845050a
> # Parent  ca1ad8ef38be225caec42564502aafe43cae173d
> revert: apply normallookup on reverted file if size isn't changed (issue4583)
> 
> Before this patch, reverting a file to the revision other than the
> parent doesn't update dirstate. This seems to expect that timestamp
> and/or size will be changed by reverting.
> 
> But if (1) dirstate of file "f" is filled with timestamp before
> reverting and (2) size and timestamp of file "f" isn't changed at
> reverting, file "f" is recognized as CLEAN unexpectedly.
> 
> This patch applies "dirstate.normallookup()" on reverted file, if size
> isn't changed.
> 
> Making "localrepository.wwrite()" return length of written data is
> needed to avoid additional (and redundant) "lstat(2)" on the reverted
> file. "filectx.size()" can't be used to know it, because data may be
> decoded at being written out.
> 
> BTW, interactive reverting may cause similar problem, too. But this
> patch doesn't focus on fixing it, because (1) interactive (maybe slow)
> reverting changes one (or both) of size/timestamp of reverted files in
> many usecases, and (2) changes to fix it seems not suitable for stable
> branch.
> 
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -3066,7 +3066,7 @@ def _performrevert(repo, parents, ctx, a
>      node = ctx.node()
>      def checkout(f):
>          fc = ctx[f]
> -        repo.wwrite(f, fc.data(), fc.flags())
> +        return repo.wwrite(f, fc.data(), fc.flags())
>  
>      audit_path = pathutil.pathauditor(repo.root)
>      for f in actions['forget'][0]:
> @@ -3114,9 +3114,13 @@ def _performrevert(repo, parents, ctx, a
>          del fp
>      else:
>          for f in actions['revert'][0]:
> -            checkout(f)
> +            wsize = checkout(f)
>              if normal:
>                  normal(f)
> +            elif wsize == repo.dirstate._map[f][2]:
> +                # changes may be overlooked without normallookup,
> +                # if size isn't changed at reverting
> +                repo.dirstate.normallookup(f)
>  
>      for f in actions['add'][0]:
>          checkout(f)
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -917,6 +917,10 @@ class localrepository(object):
>          return self._filter(self._encodefilterpats, filename, data)
>  
>      def wwrite(self, filename, data, flags):
> +        """write ``data`` into ``filename`` in the working directory
> +
> +        This returns length of written (maybe decoded) data.
> +        """
>          data = self._filter(self._decodefilterpats, filename, data)
>          if 'l' in flags:
>              self.wvfs.symlink(data, filename)
> @@ -924,6 +928,7 @@ class localrepository(object):
>              self.wvfs.write(filename, data)
>              if 'x' in flags:
>                  self.wvfs.setflags(filename, False, True)
> +        return len(data)
>  
>      def wwritedata(self, filename, data):
>          return self._filter(self._decodefilterpats, filename, data)
> diff --git a/tests/test-merge-tools.t b/tests/test-merge-tools.t
> --- a/tests/test-merge-tools.t
> +++ b/tests/test-merge-tools.t
> @@ -608,7 +608,12 @@ update is a merge ...
>    true.executable=cat
>    # hg update -C 1
>    $ hg update -q 0
> +  $ touch -t 200001010000 f
> +  $ hg status f
>    $ hg revert -q -r 1 .
> +  $ touch -t 200001010000 f
> +  $ hg status f
> +  M f
>    $ hg update -r 2
>    merging f
>    revision 1
> @@ -634,7 +639,12 @@ update should also have --tool
>    true.executable=cat
>    # hg update -C 1
>    $ hg update -q 0
> +  $ touch -t 200001010000 f
> +  $ hg status f
>    $ hg revert -q -r 1 .
> +  $ touch -t 200001010000 f
> +  $ hg status f
> +  M f
>    $ hg update -r 2 --tool false
>    merging f
>    merging f failed!
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp