Ensure partial colpkg file removed if export fails

rslib/src/import_export/package/colpkg/export.rs

@@ -20,6 +20,7 @@ use zstd::{
 use super::super::{MediaEntries, MediaEntry, Meta, Version};
 use crate::{
     collection::CollectionBuilder,
+    io::atomic_rename,
     media::files::{filename_if_normalized, sha1_of_data},
     prelude::*,
 };
@@ -38,6 +39,7 @@ impl Collection {
         progress_fn: impl FnMut(usize),
     ) -> Result<()> {
         let colpkg_name = out_path.as_ref();
+        let temp_colpkg = NamedTempFile::new_in(colpkg_name.parent().ok_or(AnkiError::NotFound)?)?;
         let src_path = self.col_path.clone();
         let src_media_folder = if include_media {
             Some(self.media_folder.clone())
@@ -50,14 +52,16 @@ impl Collection {
         // exporting code should be downgrading to 18 instead of 11 (which will probably require
         // changing the boolean to an enum).
         self.close(true)?;
+
         export_collection_file(
-            &colpkg_name,
+            temp_colpkg.path(),
             &src_path,
             src_media_folder,
             legacy,
             &tr,
             progress_fn,
-        )
+        )?;
+        atomic_rename(temp_colpkg, colpkg_name)
     }
 }
 

rslib/src/import_export/package/colpkg/import.rs

@@ -22,6 +22,7 @@ use crate::{
         package::{MediaEntries, MediaEntry, Meta},
         ImportProgress,
     },
+    io::atomic_rename,
     media::files::normalize_filename,
     prelude::*,
 };
@@ -85,11 +86,7 @@ pub fn import_colpkg(
         });
 
     // Proceed with replacing collection, regardless of media import result
-    tempfile.as_file().sync_all()?;
+    atomic_rename(tempfile, &col_path)?;
-    tempfile.persist(&col_path).map_err(|err| err.error)?;
-    if !cfg!(windows) {
-        File::open(col_dir)?.sync_all()?;
-    }
 
     media_import_result
 }

rslib/src/import_export/package/colpkg/tests.rs

@@ -88,6 +88,8 @@ fn normalization_check_on_export() -> Result<()> {
             .unwrap_err(),
         AnkiError::MediaCheckRequired
     );
+    // file should have been cleaned up
+    assert!(!colpkg_name.exists());
 
     Ok(())
 }

rslib/src/io.rs

@@ -0,0 +1,19 @@
+use std::path::Path;
+
+use tempfile::NamedTempFile;
+
+use crate::prelude::*;
+
+pub(crate) fn atomic_rename(file: NamedTempFile, target: &Path) -> Result<()> {
+    file.as_file().sync_all()?;
+    file.persist(&target)
+        .map_err(|err| AnkiError::IoError(format!("write {target:?} failed: {err}")))?;
+    if !cfg!(windows) {
+        if let Some(parent) = target.parent() {
+            std::fs::File::open(parent)
+                .and_then(|file| file.sync_all())
+                .map_err(|err| AnkiError::IoError(format!("sync {parent:?} failed: {err}")))?;
+        }
+    }
+    Ok(())
+}

rslib/src/lib.rs

@@ -19,6 +19,7 @@ pub mod error;
 pub mod findreplace;
 pub mod i18n;
 pub mod import_export;
+mod io;
 pub mod latex;
 pub mod links;
 pub mod log;

rslib/src/sync/mod.rs

@@ -21,6 +21,7 @@ use crate::{
     deckconfig::DeckConfSchema11,
     decks::DeckSchema11,
     error::{SyncError, SyncErrorKind},
+    io::atomic_rename,
     notes::Note,
     notetype::{Notetype, NotetypeSchema11},
     prelude::*,
@@ -679,14 +680,7 @@ impl Collection {
         let db = open_and_check_sqlite_file(out_file.path())?;
         db.execute_batch("update col set ls=mod")?;
         drop(db);
-        // overwrite existing collection atomically
+        atomic_rename(out_file, &col_path)?;
-        out_file.as_file().sync_all()?;
-        out_file
-            .persist(&col_path)
-            .map_err(|e| AnkiError::IoError(format!("download save failed: {}", e)))?;
-        if !cfg!(windows) {
-            std::fs::File::open(col_folder)?.sync_all()?;
-        }
 
         Ok(())
     }