[PATCH] util.rename: do not abort if os.unlink fails (issue1840)

Greg Ward greg-hg at gerg.ca
Tue Oct 6 18:42:32 UTC 2009


On Tue, Oct 6, 2009 at 2:36 PM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>> That seems sensible to improve hg-stable in the face of broken AV scanners.
>>
>> <bikeshedding>
>> But it looks like util.rename() is getting awfully hairy just to
>> support weird Windows edge cases.  Perhaps it should be split into a
>> one-line trivial function in posix.py, and a hairy ugly pile of
>> Windows-specific edge cases in windows.py.  Then have util.py import
>> one or the other.
>> </bikeshedding>
>
> Does that mean you don't want that patch?

No, I think it's a good patch if it makes life a little easier for
people afflicted by broken AV scanners on a broken OS.  But I think
that, if accepted for hg-stable, it should be followed by by a patch
to refactor util.rename() into a simple posix version and a hairy
Windows version.

(IMHO util.rename() is right on the complexity threshold for
refactoring.  Your patch will push it over the threshold.  That sort
of refactoring does not belong on stable branches, but I think your
patch does.  So util.rename() will be too hairy for the rest of the
1.3 cycle, but IMHO that's tolerable.)

Of course, this is all just me spouting half-baked opinions about
software design and refactoring.  You can of course tell me where to
shove it if you disagree.  ;-)

Greg




More information about the Mercurial-devel mailing list