file_util: don't try to overwrite existing content-addressed file on Windows

The doc says persist() replaces the destination file as rename() would do
on Unix. persist_noclobber() doesn't, and is probably more reliable on Windows.
I don't know if persist() is completely atomic on Windows, but if it isn't, it
might be the source of the "permission denied" error under highly contended
situation.

https://docs.rs/tempfile/latest/tempfile/struct.NamedTempFile.html#method.persist
https://github.com/Stebalien/tempfile/blob/v3.8.0/src/file/imp/windows.rs#L77

We could use persist_noclobber() on all platforms, but it's more involved on
Unix.

https://github.com/Stebalien/tempfile/blob/v3.8.0/src/file/imp/unix.rs#L107

lib/src/file_util.rs

@@ -90,21 +90,33 @@ pub fn normalize_path(path: &Path) -> PathBuf {
     }
 }
 
-// Like NamedTempFile::persist(), but also succeeds if the target already
-// exists.
+/// Like `NamedTempFile::persist()`, but doesn't try to overwrite the existing
+/// target on Windows.
 pub fn persist_content_addressed_temp_file<P: AsRef<Path>>(
     temp_file: NamedTempFile,
     new_path: P,
 ) -> io::Result<File> {
-    match temp_file.persist(&new_path) {
-        Ok(file) => Ok(file),
-        Err(PersistError { error, file: _ }) => {
-            if let Ok(existing_file) = File::open(new_path) {
-                Ok(existing_file)
-            } else {
-                Err(error)
+    if cfg!(windows) {
+        // On Windows, overwriting file can fail if the file is opened without
+        // FILE_SHARE_DELETE for example. We don't need to take a risk if the
+        // file already exists.
+        match temp_file.persist_noclobber(&new_path) {
+            Ok(file) => Ok(file),
+            Err(PersistError { error, file: _ }) => {
+                if let Ok(existing_file) = File::open(new_path) {
+                    Ok(existing_file)
+                } else {
+                    Err(error)
+                }
             }
         }
+    } else {
+        // On Unix, rename() is atomic and should succeed even if the
+        // destination file exists. Checking if the target exists might involve
+        // non-atomic operation, so don't use persist_noclobber().
+        temp_file
+            .persist(new_path)
+            .map_err(|PersistError { error, file: _ }| error)
     }
 }