[PATCH 1 of 2 stable] bookmarks: create undo.bookmarks using repo.opener instead of util.copyfile

[Adrian Buehlmann]

On 2010-12-07 10:04, Brodie Rao wrote:
> # HG changeset patch
> # User Brodie Rao <brodie at bitheap.org>
> # Date 1291712584 -39600
> # Branch stable
> # Node ID 0590493fe86b3b2d0a6dca7d554d77d4849b898e
> # Parent  5e51254ad4d4c80669f462e310b2677f2b3c54a7
> bookmarks: create undo.bookmarks using repo.opener instead of util.copyfile
> 
> This more closely matches how the other undo files are created, and we
> don't care about settings permissions or times on the file, which can
> fail if the user running hg doesn't own the file.
> 
> diff --git a/hgext/bookmarks.py b/hgext/bookmarks.py
> --- a/hgext/bookmarks.py
> +++ b/hgext/bookmarks.py
> @@ -44,8 +44,14 @@ def write(repo):
>      can be copied back on rollback.
>      '''
>      refs = repo._bookmarks
> -    if os.path.exists(repo.join('bookmarks')):
> -        util.copyfile(repo.join('bookmarks'), repo.join('undo.bookmarks'))
> +
> +    try:
> +        bms = repo.opener('bookmarks').read()
> +    except IOError:

Nitpick:

Any reason why not limiting the "swallowing of exceptions" to:

       except IOError, err:
           if err.errno != errno.ENOENT:
               raise

Or is it ok to ignore an existing bookmarks file that you can't read?

(You probably don't want to hear this, but e.g. Windows raises an
IOError with errno.EACCES for files in scheduled delete state. I'm
currently working on that front).

> +        bms = None
> +    if bms is not None:
> +        repo.opener('undo.bookmarks', 'w').write(bms)
> +
>      if repo._bookmarkcurrent not in refs:
>          setcurrent(repo, None)
>      wlock = repo.wlock()

Sorry to be nitpickish, but I've just recently started to believe we
should be a bit pickier about swallowing exceptions.

You just happen to be my first victim (I'm not even happy any more with
some of my own changes I did in the past).

Or someone please tell me if and why I am wrong and I'll shut up on this.