Patchwork [5,of,5] util: make copyfile avoid ambiguity of file stat if needed

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 18, 2016, 3:26 p.m.
Message ID <2767843f27d7a68a8b19.1463585191@feefifofum>
Download mbox | patch
Permalink /patch/15168/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - May 18, 2016, 3:26 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1463584838 -32400
#      Thu May 19 00:20:38 2016 +0900
# Node ID 2767843f27d7a68a8b19bef9f8321c25215242b0
# Parent  a74b90602bdeb11bab9cef36283f5119c96c6656
util: make copyfile avoid ambiguity of file stat if needed

In some cases below, copying from backup is used to restore original
contents of a file. If copying keeps ctime, mtime and size of a file,
restoring is overlooked, and old contents cached before restoring
isn't invalidated as expected.

  - failure of transaction before closing (from '.hg/journal.backup.*')
  - rollback of previous transaction (from '.hg/undo.backup.*')

To avoid such problem, this patch makes copyfile() avoid ambiguity of
file stat, if needed.

Ambiguity check is executed, only if:

  - checkambig=True is specified (not all copying needs ambiguity check), and
  - destination file exists before copying

This patch also adds 'not (copystat and checkambig)' assertion,
because combination of copystat and checkambig is meaningless.

This patch is a part of preparation for "Exact Cache Validation Plan":

    https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan
Augie Fackler - May 23, 2016, 7:40 p.m.
On Thu, May 19, 2016 at 12:26:31AM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1463584838 -32400
> #      Thu May 19 00:20:38 2016 +0900
> # Node ID 2767843f27d7a68a8b19bef9f8321c25215242b0
> # Parent  a74b90602bdeb11bab9cef36283f5119c96c6656
> util: make copyfile avoid ambiguity of file stat if needed

These are queued, thanks

>
> In some cases below, copying from backup is used to restore original
> contents of a file. If copying keeps ctime, mtime and size of a file,
> restoring is overlooked, and old contents cached before restoring
> isn't invalidated as expected.
>
>   - failure of transaction before closing (from '.hg/journal.backup.*')
>   - rollback of previous transaction (from '.hg/undo.backup.*')
>
> To avoid such problem, this patch makes copyfile() avoid ambiguity of
> file stat, if needed.
>
> Ambiguity check is executed, only if:
>
>   - checkambig=True is specified (not all copying needs ambiguity check), and
>   - destination file exists before copying
>
> This patch also adds 'not (copystat and checkambig)' assertion,
> because combination of copystat and checkambig is meaningless.
>
> This patch is a part of preparation for "Exact Cache Validation Plan":
>
>     https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1010,10 +1010,14 @@ def checksignature(func):
>
>      return check
>
> -def copyfile(src, dest, hardlink=False, copystat=False):
> +def copyfile(src, dest, hardlink=False, copystat=False, checkambig=False):
>      '''copy a file, preserving mode and optionally other stat info like
>      atime/mtime'''
> +    assert not (copystat and checkambig)
> +    oldstat = None
>      if os.path.lexists(dest):
> +        if checkambig:
> +            oldstat = checkambig and filestat(dest)
>          unlink(dest)
>      # hardlinks are problematic on CIFS, quietly ignore this flag
>      # until we find a way to work around it cleanly (issue4546)
> @@ -1035,6 +1039,12 @@ def copyfile(src, dest, hardlink=False,
>                  shutil.copystat(src, dest)
>              else:
>                  shutil.copymode(src, dest)
> +                if oldstat and oldstat.stat:
> +                    newstat = filestat(dest)
> +                    if newstat.isambig(oldstat):
> +                        # stat of copied file is ambiguous to original one
> +                        advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
> +                        os.utime(dest, (advanced, advanced))
>          except shutil.Error as inst:
>              raise Abort(str(inst))
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - May 23, 2016, 7:44 p.m.
On Thu, May 19, 2016 at 12:26:31AM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1463584838 -32400
> #      Thu May 19 00:20:38 2016 +0900
> # Node ID 2767843f27d7a68a8b19bef9f8321c25215242b0
> # Parent  a74b90602bdeb11bab9cef36283f5119c96c6656
> util: make copyfile avoid ambiguity of file stat if needed

Queued these, very nice.

>
> In some cases below, copying from backup is used to restore original
> contents of a file. If copying keeps ctime, mtime and size of a file,
> restoring is overlooked, and old contents cached before restoring
> isn't invalidated as expected.
>
>   - failure of transaction before closing (from '.hg/journal.backup.*')
>   - rollback of previous transaction (from '.hg/undo.backup.*')
>
> To avoid such problem, this patch makes copyfile() avoid ambiguity of
> file stat, if needed.
>
> Ambiguity check is executed, only if:
>
>   - checkambig=True is specified (not all copying needs ambiguity check), and
>   - destination file exists before copying
>
> This patch also adds 'not (copystat and checkambig)' assertion,
> because combination of copystat and checkambig is meaningless.
>
> This patch is a part of preparation for "Exact Cache Validation Plan":
>
>     https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1010,10 +1010,14 @@ def checksignature(func):
>
>      return check
>
> -def copyfile(src, dest, hardlink=False, copystat=False):
> +def copyfile(src, dest, hardlink=False, copystat=False, checkambig=False):
>      '''copy a file, preserving mode and optionally other stat info like
>      atime/mtime'''
> +    assert not (copystat and checkambig)
> +    oldstat = None
>      if os.path.lexists(dest):
> +        if checkambig:
> +            oldstat = checkambig and filestat(dest)
>          unlink(dest)
>      # hardlinks are problematic on CIFS, quietly ignore this flag
>      # until we find a way to work around it cleanly (issue4546)
> @@ -1035,6 +1039,12 @@ def copyfile(src, dest, hardlink=False,
>                  shutil.copystat(src, dest)
>              else:
>                  shutil.copymode(src, dest)
> +                if oldstat and oldstat.stat:
> +                    newstat = filestat(dest)
> +                    if newstat.isambig(oldstat):
> +                        # stat of copied file is ambiguous to original one
> +                        advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
> +                        os.utime(dest, (advanced, advanced))
>          except shutil.Error as inst:
>              raise Abort(str(inst))
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1010,10 +1010,14 @@  def checksignature(func):
 
     return check
 
-def copyfile(src, dest, hardlink=False, copystat=False):
+def copyfile(src, dest, hardlink=False, copystat=False, checkambig=False):
     '''copy a file, preserving mode and optionally other stat info like
     atime/mtime'''
+    assert not (copystat and checkambig)
+    oldstat = None
     if os.path.lexists(dest):
+        if checkambig:
+            oldstat = checkambig and filestat(dest)
         unlink(dest)
     # hardlinks are problematic on CIFS, quietly ignore this flag
     # until we find a way to work around it cleanly (issue4546)
@@ -1035,6 +1039,12 @@  def copyfile(src, dest, hardlink=False, 
                 shutil.copystat(src, dest)
             else:
                 shutil.copymode(src, dest)
+                if oldstat and oldstat.stat:
+                    newstat = filestat(dest)
+                    if newstat.isambig(oldstat):
+                        # stat of copied file is ambiguous to original one
+                        advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
+                        os.utime(dest, (advanced, advanced))
         except shutil.Error as inst:
             raise Abort(str(inst))