Merge pull request #51 from narrrl/fix/emptied-folder-direct-switch

fix(fuse): let an emptied mirror folder switch to on-demand at once

crates/pdfs-fuse/src/devices.rs

@@ -989,9 +989,15 @@ impl Core {
                     .db
                     .sync_entries(id)
                     .map_err(|e| SwitchBlocked::Failed(format!("db: {e:?}")))?;
-                if let Some(reason) =
-                    sync::mirror_tree_unsynced(&local, &baseline, &self.ignore_rules(&local))
-                {
+                let rules = self.ignore_rules(&local);
+                // Emptied before any pass saw it, the folder is still `idle`, and
+                // the check below would refuse the switch for good. The push pass
+                // of a queued switch forgets those paths instead (`docs/BUGS.md`
+                // B255).
+                if sync::mirror_tree_gone(&local, &baseline, &rules) {
+                    return Err(SwitchBlocked::NotNow);
+                }
+                if let Some(reason) = sync::mirror_tree_unsynced(&local, &baseline, &rules) {
                     return Err(SwitchBlocked::Failed(format!(
                         "going online-only would delete what is only on this computer: {reason}"
                     )));

crates/pdfs-fuse/src/sim/daemon.rs

@@ -3002,4 +3002,69 @@ mod tests {
         assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
         let _ = std::fs::remove_dir_all(&dir);
     }
+
+    /// Emptied before any pass saw it, the folder is still `idle`, and the
+    /// switch was refused for good: every path read as "deleted here but not on
+    /// Proton Drive yet" (B255, found by the account run).
+    #[test]
+    #[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
+    fn a_folder_emptied_before_any_pass_still_goes_on_demand() {
+        let drive = FakeDrive::new();
+        let dir = scratch("ondemand-emptied-idle");
+        let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
+        let local = dir.join("docs");
+        let id = sync_folder(&daemon, &local, "mirror");
+        std::fs::create_dir(local.join("sub")).unwrap();
+        std::fs::write(local.join("sub/a.txt"), b"alpha").unwrap();
+        std::fs::write(local.join("sub/b.txt"), b"beta").unwrap();
+        daemon.request(&Request::SyncNow { id: Some(id) }).unwrap();
+        let on_drive = |name: &str, content: &[u8]| {
+            drive.tree().iter().any(|(path, entry)| {
+                path.ends_with(name) && *entry == Entry::File(Arc::new(content.to_vec()))
+            })
+        };
+        assert!(
+            wait_until(Duration::from_secs(30), || on_drive("sub/a.txt", b"alpha")
+                && on_drive("sub/b.txt", b"beta")
+                && listed_folder(&daemon, id).is_some_and(|f| f.state == "idle")),
+            "the folder synced: {:?}",
+            listed_folder(&daemon, id)
+        );
+
+        // In one rename, so no pass sees half of it, and asked at once, well
+        // inside the watcher's debounce, so no pass sees it empty either.
+        std::fs::rename(local.join("sub"), dir.join("moved-away")).unwrap();
+        let asked = daemon.request(&Request::SetSyncFolderMode {
+            id,
+            mode: "ondemand".into(),
+        });
+        assert!(
+            matches!(asked, Ok(Response::Ok { .. })),
+            "the switch was refused: {asked:?}"
+        );
+        assert!(
+            listed_folder(&daemon, id)
+                .is_some_and(|f| f.pending_mode.as_deref() == Some("ondemand")),
+            "the switch waits for a pass: {:?}",
+            listed_folder(&daemon, id)
+        );
+
+        assert!(
+            wait_until(Duration::from_secs(30), || listed_folder(&daemon, id)
+                .is_some_and(|f| f.mode == "ondemand" && f.pending_mode.is_none())
+                && is_mounted(&local)),
+            "the queued switch landed: {:?}",
+            listed_folder(&daemon, id)
+        );
+        assert_eq!(std::fs::read(local.join("sub/a.txt")).unwrap(), b"alpha");
+        assert_eq!(std::fs::read(local.join("sub/b.txt")).unwrap(), b"beta");
+        assert!(
+            on_drive("sub/a.txt", b"alpha") && on_drive("sub/b.txt", b"beta"),
+            "nothing was trashed on Drive: {:?}",
+            drive.tree().keys().collect::<Vec<_>>()
+        );
+
+        assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
+        let _ = std::fs::remove_dir_all(&dir);
+    }
 }

crates/pdfs-fuse/src/sync.rs

@@ -2253,6 +2253,27 @@ pub(crate) fn mirror_tree_unsynced(
         .map(|r| format!("{r} was deleted here but not on Proton Drive yet"))
 }
 
+/// Whether a mirror folder's local copy is gone in full: the folder is still
+/// there, but not one tracked path of its baseline is.
+///
+/// [`mirror_tree_unsynced`] reads that as deletions not carried up yet, which
+/// would refuse the switch to on-demand for good. It is the wipe guard's case
+/// instead, which the push pass a queued switch runs settles without trashing
+/// anything on Drive (#46), so the switch waits for that pass.
+pub(crate) fn mirror_tree_gone(
+    root: &Path,
+    baseline: &HashMap<String, StoredSyncEntry>,
+    rules: &IgnoreRules,
+) -> bool {
+    let tracked = filter_baseline(baseline, rules);
+    root.is_dir()
+        && !tracked.is_empty()
+        && tracked.keys().all(|rel| {
+            std::fs::symlink_metadata(root.join(rel_to_path(rel)))
+                .is_err_and(|e| e.kind() == std::io::ErrorKind::NotFound)
+        })
+}
+
 /// The walk behind both checks. `rules`, when given, only names an ignored
 /// path as such in the reason; it is still something only this disk has.
 fn unsynced_local(
@@ -3538,6 +3559,26 @@ mod tests {
         assert!(reason.unwrap().contains("link"));
     }
 
+    /// Emptied before any pass saw it, the folder waits for the push pass that
+    /// forgets those paths, instead of being refused for good (B255).
+    #[test]
+    fn only_a_local_copy_gone_in_full_counts_as_gone() {
+        let (root, baseline) = synced_subtree("evict-gone");
+        let rules = rules_for(&[]);
+        let whole = mirror_tree_gone(&root, &baseline, &rules);
+        std::fs::remove_dir_all(root.join("dir/sub")).unwrap();
+        let partly = mirror_tree_gone(&root, &baseline, &rules);
+        std::fs::remove_dir_all(root.join("dir")).unwrap();
+        let emptied = mirror_tree_gone(&root, &baseline, &rules);
+        let none_synced = mirror_tree_gone(&root, &HashMap::new(), &rules);
+        std::fs::remove_dir_all(&root).unwrap();
+        let no_root = mirror_tree_gone(&root, &baseline, &rules);
+        assert!(!whole && !partly, "a deletion with survivors is a real one");
+        assert!(emptied);
+        assert!(!none_synced, "an empty baseline has nothing to lose");
+        assert!(!no_root, "a missing folder is the pass's error to report");
+    }
+
     /// The same rule one level up: if the sync root itself cannot be read, the
     /// pass sees nothing, and "nothing" must never mean "delete everything".
     #[test]

docs/BUGS.md

@@ -402,14 +402,20 @@ folder lost its files is not known yet.
 
 **Fix.** The push pass for a switch to on-demand forgets the baseline rows of a local copy that
 is gone in full, instead of trashing them on Drive, so the switch goes ahead and the mount shows
-what Drive has. **Sync From Scratch…** in the app and `pdfs sync reset ID` clear a mirror
+what Drive has. A switch asked for before any pass saw the folder empty found it still `idle`,
+and the check for what only this computer has refused it at once: every path read as "deleted
+here but not on Proton Drive yet". The account run found that. Such a switch now waits for the
+same push pass (`mirror_tree_gone` in `crates/pdfs-fuse/src/sync.rs`, checked in
+`apply_sync_folder_mode`). **Sync From Scratch…** in the app and `pdfs sync reset ID` clear a mirror
 folder's baseline, so the next pass compares both sides as on the first sync: one-sided files
 are copied across, files that differ keep both versions, and nothing is deleted. Restore now
 logs why it skipped a folder.
 
 **Test:** The simulations `a_folder_whose_local_copy_is_gone_still_goes_on_demand` (which
-failed in exactly the reported state before the fix) and
-`a_folder_whose_local_copy_is_gone_downloads_again_after_a_reset`.
+failed in exactly the reported state before the fix),
+`a_folder_emptied_before_any_pass_still_goes_on_demand` (which failed with the account run's
+error) and `a_folder_whose_local_copy_is_gone_downloads_again_after_a_reset`; the acceptance case
+"an emptied mirror folder switches to on-demand".
 
 ---
 

docs/CHANGELOG.md

@@ -70,9 +70,10 @@ Schema: **44**, unchanged.
 
 - **A synced folder whose files were all gone from this computer no longer gets stuck** (#46).
   The sync still refuses to delete everything on Proton Drive, but a switch to online only now
-  goes ahead and shows what Proton Drive has, instead of waiting for good. To download the
-  files again instead, sync the folder from scratch. Restoring folders also logs why it
-  skipped a folder.
+  goes ahead and shows what Proton Drive has, instead of waiting for good. This includes a
+  switch asked for right after the files went, which was refused with "… was deleted here but
+  not on Proton Drive yet". To download the files again instead, sync the folder from scratch.
+  Restoring folders also logs why it skipped a folder.
 
 - **The first sync of a folder shows how far its scan got** (#46). Before anything downloads,
   the first pass reads every item on Proton Drive, which takes many minutes for a large folder.

scripts/fuse-acceptance.py

@@ -4014,30 +4014,36 @@ class Janitor:
         return [
             f"{name} ({entry['uid']}) is still in the trash"
             for entry in matches
-            if not self.purge(entry["uid"], name)
+            if not self.purge(entry["uid"], name, listed=True)
         ]
 
-    def purge(self, uid: str, name: str) -> bool:
+    def purge(self, uid: str, name: str, listed: bool = False) -> bool:
         """Permanently delete one trashed node the run owns, by uid.
 
         By uid and never by name: a name can match something of the user's, a
         uid cannot. The node has to show up in the trash first, which is also
         what proves the trash step before it worked, and it has to be gone from
-        a fresh listing afterwards.
+        a fresh listing afterwards. `listed` says the caller just found it in
+        the trash.
 
         The server's own answer comes first, though: it only deletes a node
         that is in the trash, and it answers per uid. A listing is a whole-trash
         enumeration, which on a large trash can time out on every attempt and
         would otherwise keep a folder the server would delete on request.
         """
-        with contextlib.suppress(Exception):
+        last_error = ""
+        try:
             reply = self.daemon.command("delete-forever", uid, stdin="yes\n")
             if re.search(r"\bpermanently deleted 1 item", reply):
                 print(f"[cleanup] permanently deleted {name}")
                 return True
+        except Exception as error:  # noqa: BLE001 - retried, then reported
+            last_error = f": {error}"
         deadline = time.monotonic() + self.daemon.timeout
-        attempted = False
-        last_error = ""
+        # A node seen in the trash before that delete and gone after it was
+        # deleted by it, whatever it answered. Counting it as never sent kept
+        # waiting for a node that was already gone, and failed the run.
+        attempted = listed
         while True:
             try:
                 if self.daemon.trashed(uid):
@@ -4149,7 +4155,7 @@ class Janitor:
         for entry in trashed:
             if STALE_ROOT.match(entry["name"]) and entry["name"] not in live_names:
                 print(f"[reap] permanently deleting an abandoned root from the trash: {entry['name']}")
-                if not orphan.purge(entry["uid"], entry["name"]):
+                if not orphan.purge(entry["uid"], entry["name"], listed=True):
                     leftovers.append(f"{entry['name']} ({entry['uid']}) is still in the trash")
         return leftovers