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

login
register
mail settings
Submitter Siddharth Agarwal
Date Dec. 12, 2015, 7:03 p.m.
Message ID <478e7d8d0a8d36c62d64.1449946986@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11994/
State Accepted
Headers show

Comments

Siddharth Agarwal - Dec. 12, 2015, 7:03 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1449946685 28800
#      Sat Dec 12 10:58:05 2015 -0800
# Branch stable
# Node ID 478e7d8d0a8d36c62d641f298abf2b18f227ec14
# Parent  2398f014d0f3cb09364f551d0158d096c26c84fb
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r 478e7d8d0a8d
record: don't dereference symlinks while copying over stat data

Previously, we could be calling os.utime or os.chflags (via shutil.copystat) on
a symlink. These functions dereference 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.
Matt Mackall - Dec. 16, 2015, 8:22 p.m.
On Sat, 2015-12-12 at 11:03 -0800, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1449946685 28800
> #      Sat Dec 12 10:58:05 2015 -0800
> # Branch stable
> # Node ID 478e7d8d0a8d36c62d641f298abf2b18f227ec14
> # Parent  2398f014d0f3cb09364f551d0158d096c26c84fb
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r
> 478e7d8d0a8d
> record: don't dereference symlinks while copying over stat data

These are queued for stable, thanks.

-- 
Mathematics is the supreme nostalgia of our time.

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, copystat=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.
+                    # copystat=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), copystat=True)
                     os.unlink(tmpname)
                 if tobackup:
                     os.rmdir(backupdir)