More robust pack file rename on Windows

Simplify the existing solution for `DiskObjectStore._complete_thin_pack`
and repeat it for `MemoryObjectStore.move_in_pack` that has the same
need.

Checking the platform before unlinking was needed before since
`WindowsError` was caught, which is only defined on Windows. Since this
is a subclass of `OSError`, there is no real need to be that specific.
The platform check can remain as a minor time saver, since it seems to
be guaranteed that unlinking is never necessary on other platforms.

Another alternative could be using `shutil.move`, which silently
replaces existing targets. Python 3.3 and newer also have `os.replace`
for similar functionality.

The previous code comment mentioning that a possibly existing target
file should have the same content as the source file seems to be false,
which is the reason for
`test_object_store.DiskObjectStoreTests.test_repack_existing` failing on
Windows. This commit should mend this test.

dulwich/object_store.py

@@ -651,14 +651,16 @@ class DiskObjectStore(PackBasedObjectStore):
         # Move the pack in.
         entries.sort()
         pack_base_name = self._get_pack_basepath(entries)
+        target_pack = pack_base_name + '.pack'
         if sys.platform == 'win32':
+            # Windows might have the target pack file lingering. Attempt
+            # removal, silently passing if the target does not exist.
             try:
-                os.rename(path, pack_base_name + '.pack')
+                os.remove(target_pack)
-            except WindowsError:
+            except (IOError, OSError) as e:
-                os.remove(pack_base_name + '.pack')
+                if e.errno != errno.ENOENT:
-                os.rename(path, pack_base_name + '.pack')
+                    raise
-        else:
+        os.rename(path, target_pack)
-            os.rename(path, pack_base_name + '.pack')
 
         # Write the index.
         index_file = GitFile(pack_base_name + '.idx', 'wb')
@@ -715,19 +717,16 @@ class DiskObjectStore(PackBasedObjectStore):
             return self._pack_cache[basename]
         except KeyError:
             pass
-        else:
+        target_pack = basename + '.pack'
-            os.unlink(path)
+        if sys.platform == 'win32':
-        try:
+            # Windows might have the target pack file lingering. Attempt
-            os.rename(path, basename + ".pack")
+            # removal, silently passing if the target does not exist.
-        except OSError as e:
+            try:
-            if e.errno == errno.EEXIST:
+                os.remove(target_pack)
-                # This can happen on Windows..
+            except (IOError, OSError) as e:
-                # It's safe to ignore this, since if the file already exists,
+                if e.errno != errno.ENOENT:
-                # it should have the same contents as the one we just
+                    raise
-                # generated.
+        os.rename(path, target_pack)
-                os.unlink(path)
-            else:
-                raise
         final_pack = Pack(basename)
         self._add_known_pack(basename, final_pack)
         return final_pack