[PATCH 2 of 2] util/rename: update to work better with virus scanners on Windows

[Matt Mackall]

On Tue, 2009-02-17 at 08:53 +0100, Sune Foldager wrote:
> Matt Mackall wrote:
> 
> > [complicated patch]
> 
> Hmm, hardly complicated?, but it breaks down in two parts: a) don't rely on
> exceptions for something that always happens on windows (this makes it a bit
> more complicated), and b) use mktemp and not mkstemp (this reduces
> complication by 2 lines).
> 
> > I queued up this patch yesterday, is there any reason this won't work?
> > rename: simplify forced renaming
> 
> No, it'll work I bet. But it kinda sucks from an aesthetic point of view,
> honestly :-p. I mean, no offense, but it takes a weird scenario "we need a
> file name, so let's make a file and get rid of it right away" and turns it
> into an even weirder one "we need a file name, so let's make a file, get rid
> of it and use its former name to make something". It hurts my eyes and sense
> of code-aesthetics ;-).

> My patch, apart from the complicated part which we could drop, brings the
> scenario to "we need a file name, so let's make one". And we could throw in
> our own mktemp if needed (yes it's already there, but the deprecation issue
> and all). Even just an ad-hoc in-line one-liner to construct a file name
> would be preferred over this use of mkstemp IMHO. But of course all this is
> not my call, but I implore you sincerely to chose quality over pragmatism
> here :-) (...and so should Python instead of deprecating mktemp.)

You've really lost me here. Your code does:

        temp = tempfile.mktemp(dir=os.path.dirname(dst) or '.')
        os.rename(dst, temp)
        os.unlink(temp)
        os.rename(src, dst)

My code does:

        temp = dst + "-force-rename"
        os.rename(dst, temp)
        os.unlink(temp)
        os.rename(src, dst)

As far as I can see, the difference between my fix and yours (ignoring
the extra bits you added) is I use a constant name and you use a random
one.

> If we could at least do it any other way than making pointless files, even
> if it's not the way I do it in my patch.

You did read the comment for the existing code, right? We MUST make
pointless files to work reliably, because Windows will allow us to
successfully delete a file and STILL not allow us to rename to it.

-- 
http://selenic.com : development and support for Mercurial and Linux