[PATCH 2 of 2] Reorder rename operations to minimise risk of leaving repository in unknown state

[Steve Borho]

On Sun, Oct 4, 2009 at 1:43 PM, Adrian Buehlmann <adrian at cadifra.com> wrote:

> On 29.09.2009 00:00, Laurens Holst wrote:
> > # HG changeset patch
> > # User Grauw <laurens.hg at grauw.nl>
> > # Date 1254174954 -7200
> > # Node ID c6d8d8a535d478f6fdc00dd6e111c34cf7491c2b
> > # Parent  844c64c7cc2123c5ae16fbfcd27f16fa3e232eca
> > Reorder rename operations to minimise risk of leaving repository in
> unknown state
> > because of interfering virus scanner. As suggested by Steve Borho, see
> > http://bitbucket.org/tortoisehg/stable/issue/580/#comment-57703
> > Issue 1840 - AVG anti-virus interferes with disk access
> >
> > diff -r 844c64c7cc21 -r c6d8d8a535d4 mercurial/util.py
> > --- a/mercurial/util.py       Fri Aug 07 01:15:16 2009 +0200
> > +++ b/mercurial/util.py       Mon Sep 28 23:55:54 2009 +0200
> > @@ -444,8 +444,8 @@
> >
> >          temp = tempname(dst)
> >          os.rename(dst, temp)
> > +        os.rename(src, dst)
> >          os.unlink(temp)
> > -        os.rename(src, dst)
> >
> >  def unlink(f):
> >      """unlink and remove the directory if it is empty"""
>
> As we have discussed in this thread, this may still leave serious
> inconsistencies.
>
> Leaking temporaries when facing rude AV-scanners seems like
> the lesser evil.
>
> So we probably should just ignore if os.unlink fails here. We can't
> do much about it anyway.
>
> Counterproposal to the patch above:
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -442,7 +442,14 @@ def rename(src, dst):
>
>         temp = tempname(dst)
>         os.rename(dst, temp)
> -        os.unlink(temp)
> +        try:
> +            os.unlink(temp)
> +        except:
> +            # Some rude AV-scanners on Windows may cause the unlink to
> +            # fail. Not aborting here just leaks the temp file, whereas
> +            # aborting at this point may leave serious inconsistencies.
> +            # Ideally, we would notify the user here.
> +            pass
>          os.rename(src, dst)
>
>  def unlink(f):
>

+1 for me.  This is better than the status quo.

--
Steve Borho
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://selenic.com/pipermail/mercurial-devel/attachments/20091005/428091bb/attachment.htm