[PATCH stable] util.copyfile: don't bail if shutil.copystat() raises OSError

Matt Mackall mpm at selenic.com
Sun Dec 5 17:12:05 CST 2010


On Mon, 2010-12-06 at 09:30 +1100, Brodie Rao wrote:
> There's no issue with creating the file, copying .hg/bookmarks to .hg/ 
> undo.bookmarks, or rolling it back. The issue is that shutil.copystat 
> () fails when the user running hg doesn't have permission to change  
> atime/mtime, ownership, or file permissions on the .hg/undo.bookmarks  
> file, but *does* have permission to create the file (and write to  
> other files in .hg/).

The issue is that you can't make the utimes() call on files you don't
own, except to the current time.

And my point is that if we delete the target file first, we can
guarantee that we OWN the target file. This is a simpler and more robust
approach, will result in copystat actually succeeding, will avoid
several other obscure permission problems, and is a pattern we use
elsewhere in the code.

See?


But let's look at why we need this again:

http://mercurial.markmail.org/thread/kwl42s6dp5uhccih#query:+page:1
+mid:kwl42s6dp5uhccih+state:results

The stated goal here is to avoid upsetting VIM when record does
pointless copies. There are two problems here: 1) we're doing pointless
copies and 2) there's a race between copying the file and updating the
time that can be arbitrarily large, which means this isn't a real fix.

A real fix would avoid the pointless copies instead of adding extra
overhead and complexity to every copyfile call. And we'd also avoid the
meaningless failures on undo.bookmarks.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list