Patchwork [2,of,2,STABLE?] record: don't dereference symlinks while copying over timestamps

login
register
mail settings
Submitter Siddharth Agarwal
Date Dec. 12, 2015, 8:05 a.m.
Message ID <f8d89da0f5b5dc003730.1449907549@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11980/
State Superseded
Headers show

Comments

Siddharth Agarwal - Dec. 12, 2015, 8:05 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1449907271 28800
#      Sat Dec 12 00:01:11 2015 -0800
# Branch stable
# Node ID f8d89da0f5b5dc0037301180c7a189590c6c5470
# Parent  03e439fcc25a889ebc07f2143810dba2b7cf1fcc
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r f8d89da0f5b5
record: don't dereference symlinks while copying over timestamps

Previously, we could be calling os.utime (via shutil.copystat) on a symlink.
os.utime dereferences symlinks, so this would have caused the timestamp of the
target to be set. On a read-only or similarly weird filesystem, this might
cause an exception to be raised.

This is pretty hard to test because conjuring up a read-only filesystem for
test purposes is non-trivial.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -7,7 +7,7 @@ 
 
 from node import hex, bin, nullid, nullrev, short
 from i18n import _
-import os, sys, errno, re, tempfile, cStringIO, shutil
+import os, sys, errno, re, tempfile, cStringIO
 import util, scmutil, templater, patch, error, templatekw, revlog, copies
 import match as matchmod
 import repair, graphmod, revset, phases, obsolete, pathutil
@@ -166,8 +166,7 @@  def dorecord(ui, repo, commitfunc, cmdsu
                                                dir=backupdir)
                 os.close(fd)
                 ui.debug('backup %r as %r\n' % (f, tmpname))
-                util.copyfile(repo.wjoin(f), tmpname)
-                shutil.copystat(repo.wjoin(f), tmpname)
+                util.copyfile(repo.wjoin(f), tmpname, copytime=True)
                 backups[f] = tmpname
 
             fp = cStringIO.StringIO()
@@ -216,15 +215,12 @@  def dorecord(ui, repo, commitfunc, cmdsu
                         # to be treated as unmodified
                         dirstate.normallookup(realname)
 
-                    util.copyfile(tmpname, repo.wjoin(realname))
-                    # Our calls to copystat() here and above are a
-                    # hack to trick any editors that have f open that
-                    # we haven't modified them.
+                    # copytime=True here and above are a hack to trick any
+                    # editors that have f open that we haven't modified them.
                     #
-                    # Also note that this racy as an editor could
-                    # notice the file's mtime before we've finished
-                    # writing it.
-                    shutil.copystat(tmpname, repo.wjoin(realname))
+                    # Also note that this racy as an editor could notice the
+                    # file's mtime before we've finished writing it.
+                    util.copyfile(tmpname, repo.wjoin(realname), copytime=True)
                     os.unlink(tmpname)
                 if tobackup:
                     os.rmdir(backupdir)