D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

AFFECTED FILES
  hgext/largefiles/overrides.py
  mercurial/cmdutil.py
  tests/test-rename.t

CHANGE DETAILS

diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -657,3 +657,15 @@
   [255]
   $ hg status -C
 
+check that stat information such as mtime is preserved - it's unclear whether
+the `touch` and `stat` commands are portable, so we mimic them using python.
+Not all platforms support precision of even one-second granularity, so we allow
+a rather generous fudge factor here; 1234567890 is 2009, and the primary thing
+we care about is that it's not the machine's current time; hopefully it's really
+unlikely for a machine to have such a broken clock that this test fails. :)
+
+  $ mkdir mtime
+  $ python -c 'import os; p="mtime/f"; t=1234567890; open(p, "w").close(); os.utime(p, (t, t))'
+  $ hg ci -qAm 'add mtime dir'
+  $ hg mv -q mtime mtime2
+  $ python -c 'import os, sys; p="mtime2/f"; sys.exit(int(not (os.stat(p).st_mtime < 1234567999)))'
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1186,7 +1186,7 @@
                     os.rename(src, tmp)
                     os.rename(tmp, target)
                 else:
-                    util.copyfile(src, target)
+                    util.copyfile(src, target, copystat=True)
                 srcexists = True
             except IOError as inst:
                 if inst.errno == errno.ENOENT:
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -667,15 +667,15 @@
         try:
             origcopyfile = util.copyfile
             copiedfiles = []
-            def overridecopyfile(src, dest):
+            def overridecopyfile(src, dest, *args, **kwargs):
                 if (lfutil.shortname in src and
                     dest.startswith(repo.wjoin(lfutil.shortname))):
                     destlfile = dest.replace(lfutil.shortname, '')
                     if not opts['force'] and os.path.exists(destlfile):
                         raise IOError('',
                             _('destination largefile already exists'))
                 copiedfiles.append((src, dest))
-                origcopyfile(src, dest)
+                origcopyfile(src, dest, *args, **kwargs)
 
             util.copyfile = overridecopyfile
             result += orig(ui, repo, listpats, opts, rename)



To: spectral, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
durin42 added inline comments.

INLINE COMMENTS

> test-rename.t:668
> +  $ mkdir mtime
> +  $ python -c 'import os; p="mtime/f"; t=1234567890; open(p, "w").close(); os.utime(p, (t, t))'
> +  $ hg ci -qAm 'add mtime dir'

You either need to do $PYTHON or do inline python, eg

  >>> import os
  >>> p="mtime/f"
  >>> t=1234567890
  >>> open(p, "w").close()
  >>> os.utime(p, (t, t))

> test-rename.t:671
> +  $ hg mv -q mtime mtime2
> +  $ python -c 'import os, sys; p="mtime2/f"; sys.exit(int(not (os.stat(p).st_mtime < 1234567999)))'

here too

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral updated this revision to Diff 6964.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2729?vs=6735&id=6964

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

AFFECTED FILES
  hgext/largefiles/overrides.py
  mercurial/cmdutil.py
  tests/test-rename.t

CHANGE DETAILS

diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -657,3 +657,25 @@
   [255]
   $ hg status -C
 
+check that stat information such as mtime is preserved - it's unclear whether
+the `touch` and `stat` commands are portable, so we mimic them using python.
+Not all platforms support precision of even one-second granularity, so we allow
+a rather generous fudge factor here; 1234567890 is 2009, and the primary thing
+we care about is that it's not the machine's current time; hopefully it's really
+unlikely for a machine to have such a broken clock that this test fails. :)
+
+  $ mkdir mtime
+Create the file (as empty), then update its mtime and atime to be 1234567890.
+  >>> import os
+  >>> filename = "mtime/f"
+  >>> mtime = 1234567890
+  >>> open(filename, "w").close()
+  >>> os.utime(filename, (mtime, mtime))
+  $ hg ci -qAm 'add mtime dir'
+  $ hg mv -q mtime mtime2
+Check the actual mtime on the file. Will return non-zero if the mtime is too
+new.
+  >>> import os
+  >>> import sys
+  >>> filename = "mtime2/f"
+  >>> sys.exit(int(not (os.stat(filename).st_mtime < 1234567999)))
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1186,7 +1186,7 @@
                     os.rename(src, tmp)
                     os.rename(tmp, target)
                 else:
-                    util.copyfile(src, target)
+                    util.copyfile(src, target, copystat=True)
                 srcexists = True
             except IOError as inst:
                 if inst.errno == errno.ENOENT:
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -667,15 +667,15 @@
         try:
             origcopyfile = util.copyfile
             copiedfiles = []
-            def overridecopyfile(src, dest):
+            def overridecopyfile(src, dest, *args, **kwargs):
                 if (lfutil.shortname in src and
                     dest.startswith(repo.wjoin(lfutil.shortname))):
                     destlfile = dest.replace(lfutil.shortname, '')
                     if not opts['force'] and os.path.exists(destlfile):
                         raise IOError('',
                             _('destination largefile already exists'))
                 copiedfiles.append((src, dest))
-                origcopyfile(src, dest)
+                origcopyfile(src, dest, *args, **kwargs)
 
             util.copyfile = overridecopyfile
             result += orig(ui, repo, listpats, opts, rename)



To: spectral, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral marked 2 inline comments as done.
spectral added a comment.


  Neat, didn't know about the inline python stuff.  That's much nicer.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
durin42 requested changes to this revision.
durin42 added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> test-rename.t:681
> +  >>> filename = "mtime2/f"
> +  >>> sys.exit(int(not (os.stat(filename).st_mtime < 1234567999)))

Oh, probably don't do sys.exit here. Instead just always print the boolean result.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral updated this revision to Diff 6965.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2729?vs=6964&id=6965

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

AFFECTED FILES
  hgext/largefiles/overrides.py
  mercurial/cmdutil.py
  tests/test-rename.t

CHANGE DETAILS

diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -657,3 +657,24 @@
   [255]
   $ hg status -C
 
+check that stat information such as mtime is preserved - it's unclear whether
+the `touch` and `stat` commands are portable, so we mimic them using python.
+Not all platforms support precision of even one-second granularity, so we allow
+a rather generous fudge factor here; 1234567890 is 2009, and the primary thing
+we care about is that it's not the machine's current time; hopefully it's really
+unlikely for a machine to have such a broken clock that this test fails. :)
+
+  $ mkdir mtime
+Create the file (as empty), then update its mtime and atime to be 1234567890.
+  >>> import os
+  >>> filename = "mtime/f"
+  >>> mtime = 1234567890
+  >>> open(filename, "w").close()
+  >>> os.utime(filename, (mtime, mtime))
+  $ hg ci -qAm 'add mtime dir'
+  $ hg mv -q mtime mtime2
+  >>> from __future__ import print_function
+  >>> import os
+  >>> filename = "mtime2/f"
+  >>> print(os.stat(filename).st_mtime < 1234567999)
+  True
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1186,7 +1186,7 @@
                     os.rename(src, tmp)
                     os.rename(tmp, target)
                 else:
-                    util.copyfile(src, target)
+                    util.copyfile(src, target, copystat=True)
                 srcexists = True
             except IOError as inst:
                 if inst.errno == errno.ENOENT:
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -667,15 +667,15 @@
         try:
             origcopyfile = util.copyfile
             copiedfiles = []
-            def overridecopyfile(src, dest):
+            def overridecopyfile(src, dest, *args, **kwargs):
                 if (lfutil.shortname in src and
                     dest.startswith(repo.wjoin(lfutil.shortname))):
                     destlfile = dest.replace(lfutil.shortname, '')
                     if not opts['force'] and os.path.exists(destlfile):
                         raise IOError('',
                             _('destination largefile already exists'))
                 copiedfiles.append((src, dest))
-                origcopyfile(src, dest)
+                origcopyfile(src, dest, *args, **kwargs)
 
             util.copyfile = overridecopyfile
             result += orig(ui, repo, listpats, opts, rename)



To: spectral, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG9e460318ca4b: copyfile: preserve stat info (mtime, etc.) when doing copies/renames (authored by spectral, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2729?vs=6965&id=6967

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

AFFECTED FILES
  hgext/largefiles/overrides.py
  mercurial/cmdutil.py
  tests/test-rename.t

CHANGE DETAILS

diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -657,3 +657,24 @@
   [255]
   $ hg status -C
 
+check that stat information such as mtime is preserved - it's unclear whether
+the `touch` and `stat` commands are portable, so we mimic them using python.
+Not all platforms support precision of even one-second granularity, so we allow
+a rather generous fudge factor here; 1234567890 is 2009, and the primary thing
+we care about is that it's not the machine's current time; hopefully it's really
+unlikely for a machine to have such a broken clock that this test fails. :)
+
+  $ mkdir mtime
+Create the file (as empty), then update its mtime and atime to be 1234567890.
+  >>> import os
+  >>> filename = "mtime/f"
+  >>> mtime = 1234567890
+  >>> open(filename, "w").close()
+  >>> os.utime(filename, (mtime, mtime))
+  $ hg ci -qAm 'add mtime dir'
+  $ hg mv -q mtime mtime2
+  >>> from __future__ import print_function
+  >>> import os
+  >>> filename = "mtime2/f"
+  >>> print(os.stat(filename).st_mtime < 1234567999)
+  True
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1186,7 +1186,7 @@
                     os.rename(src, tmp)
                     os.rename(tmp, target)
                 else:
-                    util.copyfile(src, target)
+                    util.copyfile(src, target, copystat=True)
                 srcexists = True
             except IOError as inst:
                 if inst.errno == errno.ENOENT:
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -667,15 +667,15 @@
         try:
             origcopyfile = util.copyfile
             copiedfiles = []
-            def overridecopyfile(src, dest):
+            def overridecopyfile(src, dest, *args, **kwargs):
                 if (lfutil.shortname in src and
                     dest.startswith(repo.wjoin(lfutil.shortname))):
                     destlfile = dest.replace(lfutil.shortname, '')
                     if not opts['force'] and os.path.exists(destlfile):
                         raise IOError('',
                             _('destination largefile already exists'))
                 copiedfiles.append((src, dest))
-                origcopyfile(src, dest)
+                origcopyfile(src, dest, *args, **kwargs)
 
             util.copyfile = overridecopyfile
             result += orig(ui, repo, listpats, opts, rename)



To: spectral, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
indygreg added a comment.


  I'm sorry, but we cannot ship this as is.
 
  The reason is mtime based build systems, like GNU make.
 
  We can't have version control modifying files without bumping their mtime because this invalidates the target freshness checks of mtime-based build systems.
 
  If we want to preserve mtime on file copy or move, I believe it is safe to do that if and only if the destination file didn't already exist. But if the destination exists, we need to ensure the mtime of the new file is greater than the mtime of the old file.
 
  Reading this patch, I /think/ the previous behavior was buggy in edge cases because we never ensured the mtime of the replacement was newer than the existing file. In 99.99% of cases, it will be because the existing file was created sometime in the past. But if bad clocks or other wonky things are in play, there's no guarantee that *wall clock now* is greater than the mtime of the existing destination file. The correct thing to do in this situation is read the mtime of the existing file and ensure the mtime of the new file is at least 1s greater than the previous mtime (1s because not all filesystems preserve microsecond or millisecond mtime granularity).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42
Cc: indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral added a comment.


  In https://phab.mercurial-scm.org/D2729#45664, @indygreg wrote:
 
  > I'm sorry, but we cannot ship this as is.
  >
  > The reason is mtime based build systems, like GNU make.
  >
  > We can't have version control modifying files without bumping their mtime because this invalidates the target freshness checks of mtime-based build systems.
 
 
  If the user does an `mv` in the shell, at least on Linux, it preserves mtime.  If they do a `cp`, it doesn't (the file gets the current timestamp).  This includes when overwriting a file.  I think I'd be fine with mimicking this behavior (only preserve mtime on `hg mv`) if that would make this safer or easier to reason about.
 
  > If we want to preserve mtime on file copy or move, I believe it is safe to do that if and only if the destination file didn't already exist. But if the destination exists, we need to ensure the mtime of the new file is greater than the mtime of the old file.
 
  `hg mv` and `hg cp` require `--after` if the destination file already exists; in those cases, we don't seem to touch the working directory at all (including not modifying the mtime, even with my patch).  With `--after`, this is purely a VCS operation that afaict "shouldn't" have any effect on build systems, so I think we're safe here for that concern?
 
  > Reading this patch, I /think/ the previous behavior was buggy in edge cases because we never ensured the mtime of the replacement was newer than the existing file. In 99.99% of cases, it will be because the existing file was created sometime in the past. But if bad clocks or other wonky things are in play, there's no guarantee that *wall clock now* is greater than the mtime of the existing destination file. The correct thing to do in this situation is read the mtime of the existing file and ensure the mtime of the new file is at least 1s greater than the previous mtime (1s because not all filesystems preserve microsecond or millisecond mtime granularity).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42
Cc: indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D2729#45787, @spectral wrote:
 
  > In https://phab.mercurial-scm.org/D2729#45664, @indygreg wrote:
  >
  > > I'm sorry, but we cannot ship this as is.
  > >
  > > The reason is mtime based build systems, like GNU make.
  > >
  > > We can't have version control modifying files without bumping their mtime because this invalidates the target freshness checks of mtime-based build systems.
  >
  >
  > If the user does an `mv` in the shell, at least on Linux, it preserves mtime.  If they do a `cp`, it doesn't (the file gets the current timestamp).  This includes when overwriting a file.  I think I'd be fine with mimicking this behavior (only preserve mtime on `hg mv`) if that would make this safer or easier to reason about.
 
 
  Greg has some concerns about this patch and it sounds like you will make some changes. The patch was queued by Augie, but I'm dropping it for now, so we don't have a slightly controversial patch written by a Googler queued by two other Googlers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42
Cc: martinvonz, indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral updated this revision to Diff 7190.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2729?vs=6967&id=7190

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

AFFECTED FILES
  hgext/largefiles/overrides.py
  mercurial/cmdutil.py
  tests/test-rename.t

CHANGE DETAILS

diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -657,3 +657,24 @@
   [255]
   $ hg status -C
 
+check that stat information such as mtime is preserved - it's unclear whether
+the `touch` and `stat` commands are portable, so we mimic them using python.
+Not all platforms support precision of even one-second granularity, so we allow
+a rather generous fudge factor here; 1234567890 is 2009, and the primary thing
+we care about is that it's not the machine's current time; hopefully it's really
+unlikely for a machine to have such a broken clock that this test fails. :)
+
+  $ mkdir mtime
+Create the file (as empty), then update its mtime and atime to be 1234567890.
+  >>> import os
+  >>> filename = "mtime/f"
+  >>> mtime = 1234567890
+  >>> open(filename, "w").close()
+  >>> os.utime(filename, (mtime, mtime))
+  $ hg ci -qAm 'add mtime dir'
+  $ hg mv -q mtime mtime2
+  >>> from __future__ import print_function
+  >>> import os
+  >>> filename = "mtime2/f"
+  >>> print(os.stat(filename).st_mtime < 1234567999)
+  True
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1187,7 +1187,7 @@
                     os.rename(src, tmp)
                     os.rename(tmp, target)
                 else:
-                    util.copyfile(src, target)
+                    util.copyfile(src, target, copystat=True)
                 srcexists = True
             except IOError as inst:
                 if inst.errno == errno.ENOENT:
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -667,15 +667,15 @@
         try:
             origcopyfile = util.copyfile
             copiedfiles = []
-            def overridecopyfile(src, dest):
+            def overridecopyfile(src, dest, *args, **kwargs):
                 if (lfutil.shortname in src and
                     dest.startswith(repo.wjoin(lfutil.shortname))):
                     destlfile = dest.replace(lfutil.shortname, '')
                     if not opts['force'] and os.path.exists(destlfile):
                         raise IOError('',
                             _('destination largefile already exists'))
                 copiedfiles.append((src, dest))
-                origcopyfile(src, dest)
+                origcopyfile(src, dest, *args, **kwargs)
 
             util.copyfile = overridecopyfile
             result += orig(ui, repo, listpats, opts, rename)



To: spectral, #hg-reviewers, durin42
Cc: martinvonz, indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral added a reviewer: indygreg.
spectral added a comment.


  In https://phab.mercurial-scm.org/D2729#46372, @martinvonz wrote:
 
  > Greg has some concerns about this patch and it sounds like you will make some changes. The patch was queued by Augie, but I'm dropping it for now, so we don't have a slightly controversial patch written by a Googler queued by two other Googlers.
 
 
  Made it only do this for `hg cp`.  Not sure if I need to reopen this somehow, or if that happens automatically..

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42, indygreg
Cc: martinvonz, indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
spectral updated this revision to Diff 7191.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2729?vs=7190&id=7191

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

AFFECTED FILES
  hgext/largefiles/overrides.py
  mercurial/cmdutil.py
  tests/test-rename.t

CHANGE DETAILS

diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -657,3 +657,36 @@
   [255]
   $ hg status -C
 
+check that stat information such as mtime is preserved on rename - it's unclear
+whether the `touch` and `stat` commands are portable, so we mimic them using
+python.  Not all platforms support precision of even one-second granularity, so
+we allow a rather generous fudge factor here; 1234567890 is 2009, and the
+primary thing we care about is that it's not the machine's current time;
+hopefully it's really unlikely for a machine to have such a broken clock that
+this test fails. :)
+
+  $ mkdir mtime
+Create the file (as empty), then update its mtime and atime to be 1234567890.
+  >>> import os
+  >>> filename = "mtime/f"
+  >>> mtime = 1234567890
+  >>> open(filename, "w").close()
+  >>> os.utime(filename, (mtime, mtime))
+  $ hg ci -qAm 'add mtime dir'
+"hg cp" does not preserve the mtime, so it should be newer than the 2009
+timestamp.
+  $ hg cp -q mtime mtime_cp
+  >>> from __future__ import print_function
+  >>> import os
+  >>> filename = "mtime_cp/f"
+  >>> print(os.stat(filename).st_mtime < 1234567999)
+  False
+"hg mv" preserves the mtime, so it should be ~equal to the 2009 timestamp
+(modulo some fudge factor due to not every system supporting 1s-level
+precision).
+  $ hg mv -q mtime mtime_mv
+  >>> from __future__ import print_function
+  >>> import os
+  >>> filename = "mtime_mv/f"
+  >>> print(os.stat(filename).st_mtime < 1234567999)
+  True
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1187,7 +1187,9 @@
                     os.rename(src, tmp)
                     os.rename(tmp, target)
                 else:
-                    util.copyfile(src, target)
+                    # Preserve stat info on renames, not on copies; this matches
+                    # Linux CLI behavior.
+                    util.copyfile(src, target, copystat=rename)
                 srcexists = True
             except IOError as inst:
                 if inst.errno == errno.ENOENT:
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -667,15 +667,15 @@
         try:
             origcopyfile = util.copyfile
             copiedfiles = []
-            def overridecopyfile(src, dest):
+            def overridecopyfile(src, dest, *args, **kwargs):
                 if (lfutil.shortname in src and
                     dest.startswith(repo.wjoin(lfutil.shortname))):
                     destlfile = dest.replace(lfutil.shortname, '')
                     if not opts['force'] and os.path.exists(destlfile):
                         raise IOError('',
                             _('destination largefile already exists'))
                 copiedfiles.append((src, dest))
-                origcopyfile(src, dest)
+                origcopyfile(src, dest, *args, **kwargs)
 
             util.copyfile = overridecopyfile
             result += orig(ui, repo, listpats, opts, rename)



To: spectral, #hg-reviewers, durin42, indygreg
Cc: martinvonz, indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D2729: copyfile: preserve stat info (mtime, etc.) when doing copies/renames

indygreg (Gregory Szorc)
In reply to this post by indygreg (Gregory Szorc)
yuja added a comment.


  Queued new version, thanks.
 
  FWIW, `phabup` complains that "You can not accept this revision because it
  has already been closed. Only open revisions can be accepted."

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2729

To: spectral, #hg-reviewers, durin42, indygreg
Cc: yuja, martinvonz, indygreg, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel