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

[Brodie Rao]

On Dec 6, 2010, at 3:29 AM, Matt Mackall wrote:

> On Sun, 2010-12-05 at 18:16 +1100, Brodie Rao wrote:
>> # HG changeset patch
>> # User Brodie Rao <brodie at bitheap.org>
>> # Date 1291531599 -39600
>> # Branch stable
>> # Node ID a99156f9d3dcf0e32db794ad5f574bb8b0fc3c0e
>> # Parent  5e51254ad4d4c80669f462e310b2677f2b3c54a7
>> util.copyfile: don't bail if shutil.copystat() raises OSError
>>
>> This allows operations using util.copyfile() to work even if
>> shutil.copystat() fails.
>>
>> This is particularly important when working with repositories where
>> the current user is the not the owner but has write access to
>> it. Without this patch, doing something like updating a bookmark  
>> would
>> fail because the user doesn't have permission to call utime() on
>> .hg/undo.bookmarks.
>
> I don't think that analysis is quite right. Bookmarks are written with
> atomictemp files, which don't use copyfile. The undo.bookmarks is
> presumably created on commit to prepare for a rollback.
>
> A more robust approach would probably include attempting to first  
> delete
> the target. This maximizes our odds of successfully writing to an
> existing file. See localrepo:wwrite. If we can't delete files, we're
> likely to soon have other problems writing and creating files.

My example is purely about the .hg/undo.bookmarks file, which is  
created with util.copyfile().

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/).

This changeset added the shutil.copystat() call:

changeset:   7767:b2410ed2cbe9
user:        Will Maier <willmaier at ml1.net>
date:        Mon Feb 09 07:55:42 2009 -0600
summary:     Use shutil.copystat in copyfile().

On the mailing list, Will said this:

> With this change, vim doesn't complain when a user records hunks
> while the editor has an open buffer for the file under review. This
> change also didn't cause any test failures on my system
> (OpenBSD/i386), which suggests that nobody had made assumptions
> about the old behavior.

I think it's acceptable that util.copyfile() still succeeds even if  
its shutil.copystat() call fails.

The only downside I can see with this approach is that you could lose  
the execute bit when util.copyfile() is used in the working directory.  
In that case, perhaps it could emit a warning. Or bookmarks could use  
something other than util.copyfile() to create its undo file.