mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ba414c4446 | ||
|
|
6f2b4bb80a |
No files matched your search
@@ -1467,7 +1467,11 @@ fn cmd_sync_restore(yes: bool, device: Option<String>) -> Result<()> {
|
||||
items: chosen,
|
||||
},
|
||||
None => CtlRequest::RestoreSyncFolders { items: chosen },
|
||||
})?)
|
||||
})?)?;
|
||||
// The daemon acks before it works, so the outcome, and any folder it had
|
||||
// to skip, only shows up later.
|
||||
println!("Run `pdfs activity` to see which folders were restored.");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Expand a leading `~` in a hand-typed path. The daemon requires an absolute
|
||||
|
||||
@@ -1526,9 +1526,17 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U
|
||||
let core = core.clone();
|
||||
std::thread::spawn(move || {
|
||||
match core.restore_sync_folders(&items, device.as_deref()) {
|
||||
Ok(message) => {
|
||||
Ok(outcome) => {
|
||||
core.touch_profile();
|
||||
core.log_activity(ActivityKind::Download, &message, "", true);
|
||||
if !outcome.complete() {
|
||||
warn!(skipped = ?outcome.skipped, "restore skipped folders");
|
||||
}
|
||||
core.log_activity(
|
||||
ActivityKind::Download,
|
||||
outcome.summary(),
|
||||
outcome.skipped.join("; "),
|
||||
outcome.complete(),
|
||||
);
|
||||
}
|
||||
Err(e) => {
|
||||
warn!(error = %e, "restore failed");
|
||||
|
||||
@@ -388,7 +388,7 @@ impl Core {
|
||||
&self,
|
||||
items: &[pdfs_core::control::RestoreItem],
|
||||
device: Option<&str>,
|
||||
) -> CoreResult<String> {
|
||||
) -> CoreResult<RestoreOutcome> {
|
||||
if items.is_empty() {
|
||||
return Err(CoreError::invalid("nothing to restore"));
|
||||
}
|
||||
@@ -434,10 +434,19 @@ impl Core {
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let id = self
|
||||
.db
|
||||
.sync_folder_add(&local.to_string_lossy(), &item.remote_uid, &device.share_id)
|
||||
.map_err(|e| CoreError::internal(format!("db: {e:?}")))?;
|
||||
let id = match self.db.sync_folder_add(
|
||||
&local.to_string_lossy(),
|
||||
&item.remote_uid,
|
||||
&device.share_id,
|
||||
) {
|
||||
Ok(id) => id,
|
||||
Err(e) => {
|
||||
// The folders restored before this one still need their
|
||||
// watches.
|
||||
let _ = self.sync_tx.send(sync::SyncMsg::Rewatch);
|
||||
return Err(CoreError::internal(format!("db: {e:?}")));
|
||||
}
|
||||
};
|
||||
// `ondemand` is applied through the normal request path, which
|
||||
// mounts the FUSE session and handles the folder being busy — the
|
||||
// restore does not get its own copy of that logic.
|
||||
@@ -454,12 +463,35 @@ impl Core {
|
||||
for reason in &skipped {
|
||||
info!(reason, "restore skipped a folder");
|
||||
}
|
||||
let mut message = format!("restored {restored} folder(s)");
|
||||
if !skipped.is_empty() {
|
||||
message.push_str(&format!("; skipped: {}", skipped.join(", ")));
|
||||
}
|
||||
info!(restored, skipped = skipped.len(), "restore complete");
|
||||
Ok(message)
|
||||
Ok(RestoreOutcome { restored, skipped })
|
||||
}
|
||||
}
|
||||
|
||||
/// What a restore did: how many folders it attached, and why it left out the
|
||||
/// others.
|
||||
pub(crate) struct RestoreOutcome {
|
||||
pub(crate) restored: usize,
|
||||
pub(crate) skipped: Vec<String>,
|
||||
}
|
||||
|
||||
impl RestoreOutcome {
|
||||
/// The activity entry's headline.
|
||||
pub(crate) fn summary(&self) -> String {
|
||||
match (self.restored, self.skipped.len()) {
|
||||
(n, 0) => format!("restored {n} folder(s)"),
|
||||
(0, k) => format!("restored no folders; skipped {k}"),
|
||||
(n, k) => format!("restored {n} folder(s); skipped {k}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether the restore did all it was asked. A skipped folder is a
|
||||
/// failure the user has to see: the reply went out as "restoring folders"
|
||||
/// before any work, and a restore that skipped every folder used to be
|
||||
/// logged as a success, so nothing anywhere said that nothing would
|
||||
/// download (`docs/BUGS.md` B269).
|
||||
pub(crate) fn complete(&self) -> bool {
|
||||
self.skipped.is_empty()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1162,14 +1162,22 @@ impl Core {
|
||||
let uids = self
|
||||
.rt
|
||||
.block_on(self.drive.enumerate_folder_children_node_uids(folder))
|
||||
.map_err(|e| format!("enumerate {folder}: {e}"))?;
|
||||
.map_err(|e| format!("enumerate {folder}: {e}"));
|
||||
let uids = match unlisted_gap(walk, prefix, uids)? {
|
||||
Some(uids) => uids,
|
||||
None => return Ok(()),
|
||||
};
|
||||
if uids.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
let nodes = self
|
||||
.rt
|
||||
.block_on(self.drive.enumerate_nodes_light(&uids))
|
||||
.map_err(|e| format!("resolve nodes: {e}"))?;
|
||||
.map_err(|e| format!("resolve nodes: {e}"));
|
||||
let nodes = match unlisted_gap(walk, prefix, nodes)? {
|
||||
Some(nodes) => nodes,
|
||||
None => return Ok(()),
|
||||
};
|
||||
// The SDK leaves out a node it cannot decrypt, and says so only in its
|
||||
// log. Its name went with it, so all this can record is the folder.
|
||||
if nodes.len() < uids.len() {
|
||||
@@ -1213,10 +1221,14 @@ impl Core {
|
||||
let sized: HashMap<String, (i64, Option<String>)> = if stale.is_empty() {
|
||||
HashMap::new()
|
||||
} else {
|
||||
self.rt
|
||||
let full = self
|
||||
.rt
|
||||
.block_on(self.drive.enumerate_nodes(&stale))
|
||||
.map_err(|e| format!("resolve nodes: {e}"))?
|
||||
.iter()
|
||||
.map_err(|e| format!("resolve nodes: {e}"));
|
||||
let Some(full) = unlisted_gap(walk, prefix, full)? else {
|
||||
return Ok(());
|
||||
};
|
||||
full.iter()
|
||||
.map(|n| {
|
||||
(
|
||||
n.uid.to_string(),
|
||||
@@ -2034,6 +2046,34 @@ struct RemoteWalk<'a> {
|
||||
gaps: &'a mut RemoteGaps,
|
||||
}
|
||||
|
||||
/// A listing call's result in [`Core::walk_remote`]. A failure below the root
|
||||
/// records `prefix` as unlisted and returns `None`, so the walk goes on
|
||||
/// without that folder. A failure at the root still fails the pass, since
|
||||
/// nothing would be left to compare.
|
||||
///
|
||||
/// One listing error, after the SDK's own retries, used to fail the whole
|
||||
/// walk. Nothing is applied before the walk ends, so on a link that drops a
|
||||
/// request now and then a large first sync started over from nothing on
|
||||
/// every pass and never got past it (`docs/BUGS.md` B271).
|
||||
fn unlisted_gap<T>(
|
||||
walk: &mut RemoteWalk<'_>,
|
||||
prefix: &str,
|
||||
result: Result<T, String>,
|
||||
) -> Result<Option<T>, PassAbort> {
|
||||
match result {
|
||||
Ok(value) => Ok(Some(value)),
|
||||
Err(error) if prefix.is_empty() => Err(error.into()),
|
||||
Err(error) => {
|
||||
warn!(
|
||||
folder = prefix,
|
||||
error, "sync: a folder on Drive could not be listed; leaving it out of this pass"
|
||||
);
|
||||
walk.gaps.unlisted(prefix);
|
||||
Ok(None)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether the file at `path` is no longer what the scan saw as `planned`.
|
||||
/// A file that is gone has nothing left to lose, so it has not changed.
|
||||
fn changed_since_scan(path: &Path, planned: &LocalItem) -> bool {
|
||||
|
||||
@@ -236,6 +236,9 @@ pub(super) struct RemoteGaps {
|
||||
short: HashSet<String>,
|
||||
/// Paths seen, but whose entry could not be read.
|
||||
unread: HashSet<String>,
|
||||
/// Folders whose listing failed outright, after the SDK's retries. Nothing
|
||||
/// under one is known, so all of it is left out of the pass (B271).
|
||||
unlisted: HashSet<String>,
|
||||
}
|
||||
|
||||
impl RemoteGaps {
|
||||
@@ -249,8 +252,13 @@ impl RemoteGaps {
|
||||
self.unread.insert(rel);
|
||||
}
|
||||
|
||||
/// The listing of `folder` failed, so nothing under it was walked.
|
||||
pub(super) fn unlisted(&mut self, folder: &str) {
|
||||
self.unlisted.insert(folder.to_string());
|
||||
}
|
||||
|
||||
pub(super) fn is_empty(&self) -> bool {
|
||||
self.short.is_empty() && self.unread.is_empty()
|
||||
self.short.is_empty() && self.unread.is_empty() && self.unlisted.is_empty()
|
||||
}
|
||||
|
||||
/// Whether `rel` may be missing from `remote` only because the walk could
|
||||
@@ -272,7 +280,7 @@ impl RemoteGaps {
|
||||
let mut at = parent_rel(rel);
|
||||
loop {
|
||||
if remote_dirs.contains_key(at) {
|
||||
return self.short.contains(at);
|
||||
return self.short.contains(at) || self.unlisted.contains(at);
|
||||
}
|
||||
if at.is_empty() {
|
||||
return false;
|
||||
@@ -295,6 +303,9 @@ impl RemoteGaps {
|
||||
.iter()
|
||||
.map(|rel| format!("{rel} (cannot be read on Proton Drive)")),
|
||||
)
|
||||
.chain(self.unlisted.iter().map(|rel| {
|
||||
format!("{rel} (could not be listed on Proton Drive; tried again next pass)")
|
||||
}))
|
||||
.collect();
|
||||
lines.sort_unstable();
|
||||
lines
|
||||
@@ -844,6 +855,32 @@ mod file_plan_tests {
|
||||
assert!(gaps.hides("top.txt", &remote, &dirs));
|
||||
}
|
||||
|
||||
/// A folder whose listing failed was walked no further: nothing under it
|
||||
/// reads as deleted on Drive, and a sibling that listed still does
|
||||
/// (`docs/BUGS.md` B271).
|
||||
#[test]
|
||||
fn a_folder_that_failed_to_list_hides_everything_under_it() {
|
||||
let remote = HashMap::from([
|
||||
("docs".to_string(), folder()),
|
||||
("pics".to_string(), folder()),
|
||||
]);
|
||||
let dirs = HashMap::from([
|
||||
(String::new(), uid("root")),
|
||||
("docs".to_string(), uid("docs")),
|
||||
("pics".to_string(), uid("pics")),
|
||||
]);
|
||||
let mut gaps = RemoteGaps::default();
|
||||
gaps.unlisted("docs");
|
||||
assert!(gaps.hides("docs/a.txt", &remote, &dirs));
|
||||
assert!(gaps.hides("docs/sub/deep.txt", &remote, &dirs));
|
||||
assert!(!gaps.hides("docs", &remote, &dirs));
|
||||
assert!(!gaps.hides("pics/gone.jpg", &remote, &dirs));
|
||||
assert_eq!(
|
||||
gaps.report(),
|
||||
["docs (could not be listed on Proton Drive; tried again next pass)"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_file_whose_size_could_not_be_read_is_hidden() {
|
||||
let remote: HashMap<String, RemoteItem> = HashMap::new();
|
||||
|
||||
+19
-9
@@ -69,9 +69,7 @@ original wording.
|
||||
| Entry | Title | What is left |
|
||||
|---|---|---|
|
||||
| [B273](#b273--a-restore-proposes-another-computers-paths-when-the-two-share-a-hostname) | A restore proposes another computer's paths when the two share a hostname | Not fixed |
|
||||
| [B271](#b271--one-failed-listing-makes-a-large-first-sync-start-over-from-nothing) | One failed listing makes a large first sync start over from nothing | Not fixed |
|
||||
| [B270](#b270--one-file-left-in-a-mirror-folder-clears-the-wipe-guard-for-all-the-others) | One file left in a mirror folder clears the wipe guard for all the others | Not fixed |
|
||||
| [B269](#b269--a-restore-that-skipped-every-folder-reports-success) | A restore that skipped every folder reports success | Not fixed |
|
||||
| [B267](#b267--a-key-failure-at-a-start-from-a-warm-cache-shows-as-offline-for-good) | A key failure at a start from a warm cache shows as offline for good | Not fixed |
|
||||
| [B265](#b265--address-keys-whose-token-is-for-an-inactive-account-key-never-open) | Address keys whose token is for an inactive account key never open | Needs an SDK change; not reproduced |
|
||||
| [B134](#b134--a-file-deleted-online-while-the-drain-checks-its-write-is-kept-as-a-conflict-copy) | A file deleted online while the drain checks its write is kept as a conflict copy | Only with `local_first` off |
|
||||
@@ -86,6 +84,7 @@ original wording.
|
||||
|
||||
| Entry | Title | What is left |
|
||||
|---|---|---|
|
||||
| [B269](#b269--a-restore-that-skipped-every-folder-reports-success) | A restore that skipped every folder reports success | The app shows the result only in the activity feed; no offer to replace or reset the folder in the way |
|
||||
| [B268](#b268--pdfs-diagnose-says-no-problems-found-while-sync-is-stuck) | `pdfs diagnose` says "No problems found" while sync is stuck | The sync engine's state is not checked |
|
||||
| [B263](#b263--a-mount-that-can-never-succeed-retries-every-5-s-and-tells-only-the-journal) | A mount that can never succeed retries every 5 s and tells only the journal | Errors no retry fixes back off like network errors |
|
||||
| [B262](#b262--watching-a-large-synced-folder-holds-up-the-sync-engine-and-a-failed-watch-is-never-retried) | Watching a large synced folder holds up the sync engine, and a failed watch is never retried | The first pass still waits for the watch; a failed watch is not shown in the app |
|
||||
@@ -110,6 +109,7 @@ a live account since.
|
||||
|
||||
| Entry | Title |
|
||||
|---|---|
|
||||
| [B271](#b271--one-failed-listing-makes-a-large-first-sync-start-over-from-nothing) | One failed listing makes a large first sync start over from nothing |
|
||||
| [B266](#b266--pin-unpin-search-and-node-loads-report-a-database-error-as-offline) | Pin, unpin, search and node loads report a database error as "offline" |
|
||||
| [B264](#b264--a-mount-that-fails-after-starting-its-workers-leaves-them-running) | A mount that fails after starting its workers leaves them running |
|
||||
| [B261](#b261--a-folder-that-has-never-synced-reads-up-to-date) | A folder that has never synced reads "up to date" |
|
||||
@@ -416,7 +416,7 @@ folder list has now.
|
||||
|
||||
### B271 — One failed listing makes a large first sync start over from nothing
|
||||
|
||||
**Status:** Open
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-31, code audit after issue #46: a folder of about 19,900 items restored on a
|
||||
second computer sat at `scanning…` with nothing downloaded, and the journal showed
|
||||
`RequestTimeout (http 408)`.
|
||||
@@ -433,8 +433,13 @@ the walk is done. Files that did download keep their baseline rows, so only the
|
||||
but on a first sync that is all of it. Progress is also counted only after a folder's full
|
||||
listing, so a large folder shows `scanning…` with 0 checked for a long time.
|
||||
|
||||
**Fix.** Not done. Record a failed subtree as a gap, as undecryptable nodes already are: no
|
||||
deletions under it, and the rest of the pass still applies.
|
||||
**Fix.** A listing that fails below the sync root records that folder as a gap in
|
||||
`RemoteGaps`, next to the folders whose listing came back short. The planner deletes nothing
|
||||
under a gap, the rest of the pass still applies, and the pass report names the folder as
|
||||
tried again next pass. A failed listing of the root itself still fails the pass, since there is
|
||||
nothing left to apply. The progress count inside one large folder is unchanged.
|
||||
|
||||
**Test:** `a_folder_that_failed_to_list_hides_everything_under_it` in `sync::planner`.
|
||||
|
||||
---
|
||||
|
||||
@@ -461,7 +466,7 @@ choice to the user: delete on the other side, or sync from scratch.
|
||||
|
||||
### B269 — A restore that skipped every folder reports success
|
||||
|
||||
**Status:** Open
|
||||
**Status:** Partly fixed
|
||||
**Found:** 2026-10-31, code audit after issue #46: the journal showed `restore complete
|
||||
restored=0 skipped=1` several times, and the user saw nothing.
|
||||
**Where:** the `RestoreSyncFolders` and `RestoreDeviceFolders` handler in
|
||||
@@ -476,9 +481,14 @@ result, `restored 0 folder(s); skipped: …`, is then logged as a successful act
|
||||
reason is only in the activity text and the journal. When the database insert fails partway,
|
||||
the `?` also skips the `Rewatch` for the folders already restored.
|
||||
|
||||
**Fix.** Not done. Log a restore that skipped folders as a failure, name the reason in the app
|
||||
and the CLI, and offer to replace or reset the folder in the way. Send the `Rewatch` on every
|
||||
exit path.
|
||||
**Fix.** `restore_sync_folders` returns what it restored and what it skipped, with the reason
|
||||
for each skip. The daemon logs a restore that skipped any folder as a failed activity, with the
|
||||
reasons as its detail, and warns in the journal. A database error partway now sends the
|
||||
`Rewatch` for the folders already restored before it returns. `pdfs devices join` points to
|
||||
`pdfs activity` for the result.
|
||||
|
||||
Left: the app shows the result only in the activity feed, and nothing offers to replace or reset
|
||||
the folder in the way.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in new issue
Block a user