working_copy: consider it an error if temp file cannot be renamed to target

Unlike the other places I fixed in 134940d2bb05, the calls in
`working_copy.rs` should not simply use an existing file if the target
file was open. They should probably try again instead, but I'll leave
that for later.

lib/src/file_util.rs

@@ -19,7 +19,7 @@ use tempfile::{NamedTempFile, PersistError};
 
 // Like NamedTempFile::persist(), but also succeeds if the target already
 // exists.
-pub fn persist_temp_file<P: AsRef<Path>>(
+pub fn persist_content_addressed_temp_file<P: AsRef<Path>>(
     temp_file: NamedTempFile,
     new_path: P,
 ) -> Result<File, PersistError> {
@@ -51,7 +51,7 @@ mod tests {
         let target = temp_dir.join("file");
         let mut temp_file = NamedTempFile::new_in(&temp_dir).unwrap();
         temp_file.write_all(b"contents").unwrap();
-        assert!(persist_temp_file(temp_file, &target).is_ok());
+        assert!(persist_content_addressed_temp_file(temp_file, &target).is_ok());
     }
 
     #[test_case(false ; "existing file open")]
@@ -68,6 +68,6 @@ mod tests {
             drop(file);
         }
 
-        assert!(persist_temp_file(temp_file, &target).is_ok());
+        assert!(persist_content_addressed_temp_file(temp_file, &target).is_ok());
     }
 }

lib/src/index.rs

@@ -31,7 +31,7 @@ use itertools::Itertools;
 use tempfile::NamedTempFile;
 
 use crate::commit::Commit;
-use crate::file_util::persist_temp_file;
+use crate::file_util::persist_content_addressed_temp_file;
 use crate::store::{ChangeId, CommitId};
 
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
@@ -618,7 +618,7 @@ impl MutableIndex {
         let mut temp_file = NamedTempFile::new_in(&dir)?;
         let file = temp_file.as_file_mut();
         file.write_all(&buf).unwrap();
-        persist_temp_file(temp_file, &index_file_path)?;
+        persist_content_addressed_temp_file(temp_file, &index_file_path)?;
 
         let mut cursor = Cursor::new(&buf);
         ReadonlyIndex::load_from(&mut cursor, dir, index_file_id_hex, hash_length)

lib/src/index_store.rs

@@ -24,7 +24,7 @@ use tempfile::NamedTempFile;
 
 use crate::commit::Commit;
 use crate::dag_walk;
-use crate::file_util::persist_temp_file;
+use crate::file_util::persist_content_addressed_temp_file;
 use crate::index::{MutableIndex, ReadonlyIndex};
 use crate::op_store::OperationId;
 use crate::operation::Operation;
@@ -152,7 +152,10 @@ impl IndexStore {
         let mut temp_file = NamedTempFile::new_in(&self.dir)?;
         let file = temp_file.as_file_mut();
         file.write_all(index.name().as_bytes()).unwrap();
-        persist_temp_file(temp_file, &self.dir.join("operations").join(op_id.hex()))?;
+        persist_content_addressed_temp_file(
+            temp_file,
+            &self.dir.join("operations").join(op_id.hex()),
+        )?;
         Ok(())
     }
 }

lib/src/local_store.rs

@@ -22,7 +22,7 @@ use blake2::{Blake2b, Digest};
 use protobuf::{Message, ProtobufError};
 use tempfile::{NamedTempFile, PersistError};
 
-use crate::file_util::persist_temp_file;
+use crate::file_util::persist_content_addressed_temp_file;
 use crate::repo_path::{RepoPath, RepoPathComponent};
 use crate::store::{
     ChangeId, Commit, CommitId, Conflict, ConflictId, ConflictPart, FileId, MillisSinceEpoch,
@@ -141,7 +141,7 @@ impl Store for LocalStore {
         encoder.finish()?;
         let id = FileId(hasher.finalize().to_vec());
 
-        persist_temp_file(temp_file, self.file_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.file_path(&id))?;
         Ok(id)
     }
 
@@ -160,7 +160,7 @@ impl Store for LocalStore {
         hasher.update(&target.as_bytes());
         let id = SymlinkId(hasher.finalize().to_vec());
 
-        persist_temp_file(temp_file, self.symlink_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.symlink_path(&id))?;
         Ok(id)
     }
 
@@ -187,7 +187,7 @@ impl Store for LocalStore {
 
         let id = TreeId(Blake2b::digest(&proto_bytes).to_vec());
 
-        persist_temp_file(temp_file, self.tree_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.tree_path(&id))?;
         Ok(id)
     }
 
@@ -210,7 +210,7 @@ impl Store for LocalStore {
 
         let id = CommitId(Blake2b::digest(&proto_bytes).to_vec());
 
-        persist_temp_file(temp_file, self.commit_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.commit_path(&id))?;
         Ok(id)
     }
 
@@ -233,7 +233,7 @@ impl Store for LocalStore {
 
         let id = ConflictId(Blake2b::digest(&proto_bytes).to_vec());
 
-        persist_temp_file(temp_file, self.conflict_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.conflict_path(&id))?;
         Ok(id)
     }
 }

lib/src/simple_op_store.rs

@@ -22,7 +22,7 @@ use blake2::{Blake2b, Digest};
 use protobuf::{Message, ProtobufError};
 use tempfile::{NamedTempFile, PersistError};
 
-use crate::file_util::persist_temp_file;
+use crate::file_util::persist_content_addressed_temp_file;
 use crate::op_store::{
     OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata, View, ViewId,
 };
@@ -99,7 +99,7 @@ impl OpStore for SimpleOpStore {
 
         let id = ViewId(Blake2b::digest(&proto_bytes).to_vec());
 
-        persist_temp_file(temp_file, self.view_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.view_path(&id))?;
         Ok(id)
     }
 
@@ -122,7 +122,7 @@ impl OpStore for SimpleOpStore {
 
         let id = OperationId(Blake2b::digest(&proto_bytes).to_vec());
 
-        persist_temp_file(temp_file, self.operation_path(&id))?;
+        persist_content_addressed_temp_file(temp_file, self.operation_path(&id))?;
         Ok(id)
     }
 }

lib/src/working_copy.rs

@@ -33,7 +33,6 @@ use thiserror::Error;
 
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
-use crate::file_util::persist_temp_file;
 use crate::gitignore::GitIgnoreFile;
 use crate::lock::FileLock;
 use crate::matchers::EverythingMatcher;
@@ -238,7 +237,11 @@ impl TreeState {
         // there is no unknown data in it
         self.update_read_time();
         proto.write_to_writer(temp_file.as_file_mut()).unwrap();
-        persist_temp_file(temp_file, self.state_path.join("tree_state")).unwrap();
+        // TODO: Retry if persisting fails (it will on Windows if the file happened to
+        // be open for read).
+        temp_file
+            .persist(self.state_path.join("tree_state"))
+            .unwrap();
     }
 
     fn file_state(&self, path: &Path) -> Option<FileState> {
@@ -642,7 +645,9 @@ impl WorkingCopy {
     fn write_proto(&self, proto: crate::protos::working_copy::Checkout) {
         let mut temp_file = NamedTempFile::new_in(&self.state_path).unwrap();
         proto.write_to_writer(temp_file.as_file_mut()).unwrap();
-        persist_temp_file(temp_file, self.state_path.join("checkout")).unwrap();
+        // TODO: Retry if persisting fails (it will on Windows if the file happened to
+        // be open for read).
+        temp_file.persist(self.state_path.join("checkout")).unwrap();
     }
 
     fn read_proto(&self) -> crate::protos::working_copy::Checkout {