diff --git a/rslib/src/collection/backup.rs b/rslib/src/collection/backup.rs
index a2e6bcc7e..504db757b 100644
--- a/rslib/src/collection/backup.rs
+++ b/rslib/src/collection/backup.rs
@@ -87,13 +87,17 @@ fn backup_inner<P: AsRef<Path>>(
limits: BackupLimits,
tr: &I18n,
) -> Result<()> {
- write_backup(col_data, backup_folder.as_ref(), tr)?;
+ write_backup(col_data, backup_folder.as_ref(), tr, Local::now())?;
thin_backups(backup_folder, limits)
}
-fn write_backup<S: AsRef<OsStr>>(col_data: &[u8], backup_folder: S, tr: &I18n) -> Result<()> {
- let out_path =
- Path::new(&backup_folder).join(format!("{}", Local::now().format(BACKUP_FORMAT_STRING)));
+fn write_backup<S: AsRef<OsStr>>(
+ col_data: &[u8],
+ backup_folder: S,
+ tr: &I18n,
+ now: DateTime<Local>,
+) -> Result<()> {
+ let out_path = Path::new(&backup_folder).join(format!("{}", now.format(BACKUP_FORMAT_STRING)));
export_colpkg_from_data(out_path, col_data, tr)
}
@@ -253,6 +257,140 @@ impl BackupFilter {
mod test {
use super::*;
+ fn backup_timestamp() -> DateTime<Local> {
+ Local.timestamp_opt(1_700_000_000, 0).unwrap()
+ }
+
+ fn existing_backup(backup_dir: &Path) -> (PathBuf, Vec<u8>) {
+ let (col, _collection_dir) = open_fs_test_collection("collection");
+ let col_path = col.col_path.clone();
+ let tr = col.tr.clone();
+ col.close(None).unwrap();
+ let col_data = std::fs::read(col_path).unwrap();
+ write_backup(&col_data, backup_dir, &tr, backup_timestamp()).unwrap();
+ let path = read_dir(backup_dir)
+ .unwrap()
+ .next()
+ .unwrap()
+ .unwrap()
+ .path();
+ let original = std::fs::read(&path).unwrap();
+ (path, original)
+ }
+
+ #[test]
+ fn backup_preserves_existing_archive_when_timestamps_collide() {
+ let dir = tempfile::tempdir().unwrap();
+ let (path, original) = existing_backup(dir.path());
+
+ let result = write_backup(
+ b"replacement collection",
+ dir.path(),
+ &I18n::new(&["en"]),
+ backup_timestamp(),
+ );
+
+ assert!(
+ std::fs::read(&path).unwrap() == original,
+ "backup with the same timestamp overwrote the existing archive"
+ );
+ // A collision may be rejected or resolved with a different filename.
+ if let Err(error) = result {
+ assert!(
+ matches!(
+ &error,
+ AnkiError::FileIoError { source }
+ if source.source.kind() == std::io::ErrorKind::AlreadyExists
+ ),
+ "unexpected backup error: {error:?}"
+ );
+ }
+ }
+
+ #[cfg(unix)]
+ #[test]
+ fn interrupted_backup_preserves_existing_archive() {
+ let dir = tempfile::tempdir().unwrap();
+ let (path, original) = existing_backup(dir.path());
+
+ backup_with_file_size_limit(dir.path());
+
+ assert!(
+ std::fs::read(&path).unwrap() == original,
+ "interrupted backup overwrote the existing archive"
+ );
+ }
+
+ #[cfg(unix)]
+ const BACKUP_DIR: &str = "ANKI_TEST_INTERRUPTED_BACKUP_DIR";
+
+ #[cfg(unix)]
+ #[test]
+ fn interrupted_backup_leaves_no_visible_archive() {
+ if let Some(backup_dir) = std::env::var_os(BACKUP_DIR) {
+ use rand::RngCore;
+ use rand::SeedableRng;
+
+ let had_backup = read_dir(&backup_dir).unwrap().next().is_some();
+ // Incompressible data ensures the archive exceeds the child's file-size limit.
+ let mut col_data = vec![0; 64 * 1024];
+ rand::rngs::StdRng::seed_from_u64(22181).fill_bytes(&mut col_data);
+ let error = write_backup(
+ &col_data,
+ backup_dir,
+ &I18n::new(&["en"]),
+ backup_timestamp(),
+ )
+ .unwrap_err();
+ assert!(
+ matches!(
+ &error,
+ AnkiError::FileIoError { source }
+ if source.source.kind() == std::io::ErrorKind::FileTooLarge
+ || (had_backup && source.source.kind() == std::io::ErrorKind::AlreadyExists)
+ ),
+ "expected a write failure or rejection of an existing backup, got {error:?}"
+ );
+ return;
+ }
+
+ let dir = tempfile::tempdir().unwrap();
+ backup_with_file_size_limit(dir.path());
+
+ let backups: Vec<_> = read_dir(dir.path())
+ .unwrap()
+ .map(|entry| entry.unwrap())
+ .filter_map(Backup::from_entry)
+ .collect();
+ assert!(
+ backups.is_empty(),
+ "interrupted backup left a visible archive: {backups:?}"
+ );
+ }
+
+ #[cfg(unix)]
+ fn backup_with_file_size_limit(backup_dir: &Path) {
+ // Isolate the process-wide file-size limit. Ignoring SIGXFSZ makes writes
+ // return an error instead of terminating the process.
+ let output = std::process::Command::new("sh")
+ .args(["-c", "ulimit -f 1 && trap '' XFSZ && exec \"$@\"", "--"])
+ .arg(std::env::current_exe().unwrap())
+ .args([
+ "--exact",
+ "collection::backup::test::interrupted_backup_leaves_no_visible_archive",
+ "--nocapture",
+ ])
+ .env(BACKUP_DIR, backup_dir)
+ .output()
+ .unwrap();
+ assert!(
+ output.status.success(),
+ "backup failure injection failed:\n{}\n{}",
+ String::from_utf8_lossy(&output.stdout),
+ String::from_utf8_lossy(&output.stderr)
+ );
+ }
+
macro_rules! backup {
($num_days_from_ce:expr) => {
Backup {
on d1d484c8249a8266a6fe9203694f9087726c6ba2