Skip to content

Commit 1d9f2ca

Browse files
committed
Local copy: per-source loop uses transfer driver
- `copy_files_with_progress_inner`'s execute phase now runs through `drive_transfer_serial_sync` in `transfer_driver.rs`. The closure carries `&mut transaction`, `&mut created_dirs`, `&mut tracker`, `&mut apply_to_all_resolution`, and `&mut last_progress_time` via the driver's `FnMut` bound and threads them into `copy_single_item`. Per-iter cancellation, bulk-skip prelude, and the paired progress/status emit move into the driver. - Pre-flight scan, dry-run handling, disk-space check, `CopyTransaction::new()`, and the bulk-skip filter over `scan_result.files` stay outside the driver (pre-loop concerns). - The bulk-skip filter runs once over `scan_result.files`; surviving files form the aligned `(paths, file iter)` pair the driver iterates. `pre_skip_paths` handed to the driver is empty. - Post-loop dispatch matches on `PostLoopIntent`: `Completed` → recheck `RollingBack` (preserves the click-during-last-millisecond race fix from `1de4255d`), commit, emit complete; `Cancelled` → branch on `state.intent` for RollingBack-vs-Stopped, commit, emit cancelled, surface `WriteOperationError::Cancelled` to the caller; `Failed(e)` → rollback, emit error, propagate. - `rollback_with_progress` unchanged (sink-aware since M1). - `copy_integration_test.rs` and `tests.rs` pass without modification. Full `./scripts/check.sh --rust` green: 1813 unit + 28 integration tests. - Zero new `unsafe` / `transmute` (the sync driver's `FnMut` bound captures `&mut state` directly; no Arc<Mutex> ceremony). - CLAUDE.md `copy.rs` row updated; new "Key decisions" entry for the closure-iterator alignment pattern.
1 parent 63b6728 commit 1d9f2ca

2 files changed

Lines changed: 142 additions & 124 deletions

File tree

apps/desktop/src-tauri/src/file_system/write_operations/CLAUDE.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ network mounts, cross-filesystem moves, and name/path length limits.
1818
| `helpers.rs` | Validation (`validate_sources`, `validate_destination_writable` via `libc::access`, `validate_disk_space` via `statvfs`). Conflict resolution (`tokio::sync::oneshot` channel wait for Stop mode). `safe_overwrite_file`/`safe_overwrite_dir` (temp+rename). `find_unique_name`. `run_cancellable`. `is_same_filesystem` (device IDs). Background cleanup helpers: `remove_file_in_background`, `remove_dir_all_in_background`. |
1919
| `scan.rs` | `scan_sources` (recursive walk, emits progress), `dry_run_scan`, shared `walk_dir_recursive` walker. The `on_progress` callback receives `(files, dirs, bytes, current_file, current_dir)`; the walker reads `current_dir` from `path.parent()` so the UI can show "in directory: …" alongside the filename. Scan emit sites populate `WriteProgressEvent.current_dir` plus index-derived `expected_files_total` / `expected_bytes_total` (via `WriteProgressEvent::with_scan_meta`) so the frontend renders a real progress bar during the foolproof re-scan. Expected totals come from `crate::indexing::expected_totals::expected_totals_for_sources` (`None` when the index doesn't cover all sources; the FE falls back to a tally-only display). |
2020
| `scan_preview.rs` | Scan preview subsystem for Copy dialog live stats: `start_scan_preview`, `cancel_scan_preview`, `is_scan_preview_complete`. Background scans (local and volume-based) with result caching. Emits `expected_files_total` / `expected_bytes_total` (sampled once at scan start from the drive index) on every `scan-preview-progress` event, alongside the running tallies and `current_dir`. |
21-
| `copy.rs` | `copy_files_with_progress`: scan → disk space check → per-file copy via `copy_single_item`. `CopyTransaction` for rollback. |
21+
| `copy.rs` | `copy_files_with_progress`: scan → disk space check → per-file copy via `copy_single_item`. `CopyTransaction` for rollback. The per-source execute loop runs through `drive_transfer_serial_sync` (`transfer_driver.rs`); the closure captures `&mut transaction` / `&mut created_dirs` / `&mut tracker` / `&mut apply_to_all_resolution` and threads them into `copy_single_item`. Pre-flight scan / dry-run / disk-space / bulk-skip filtering stay outside the driver. Post-loop dispatch matches on `PostLoopIntent` (Completed / Cancelled / Failed) and reproduces the historic three-arm shape — including the post-completion `RollingBack` recheck for the click-during-the-last-millisecond race (commit `1de4255d`). |
2222
| `move_op.rs` | Same-fs: `fs::rename`. Cross-fs: copy to `.cmdr-staging-<uuid>`, atomic rename, delete sources. |
2323
| `delete.rs` | Scan, delete files first, then directories in reverse/deepest-first order. Not rollbackable. Also contains `delete_volume_files_with_progress` for non-local volumes (MTP): scans via `volume.list_directory()`, deletes via `volume.delete()` per item. |
2424
| `eta.rs` | `EtaEstimator`: time-weighted EWMA per axis (bytes, files), τ ≈ 3 s. Combines via `max(ETA_bytes, ETA_files)`. One per `WriteOperationState`, fed by `state.enrich_progress` at every `write-progress` emit site. See [ETA + throughput](#eta--throughput) below. |
@@ -225,6 +225,9 @@ exits, partial files or staging directories may remain on disk. These use the `.
225225
**Decision**: `transfer_driver.rs` ships as two sibling entry points (sync + async), not one generic-over-AsyncFnMut-or-FnMut driver. Conflict resolution lives in the driver for the async path, in the closure for the sync path.
226226
**Why**: `copy_files_with_progress_inner` is sync inside `spawn_blocking`; the three volume ops are async. A single generic driver would either force the sync path through a `Pin<Box<dyn Future>>` per source (allocation per call, no real benefit since the I/O is sync) or use a trait so gnarly that the closures stop reading as straight-line transfer code. Two siblings share `TransferContext`, `TransferOutcome`, `TransferLoopOutcome`, and `build_pre_skip_set` / `emit_progress_and_status` helpers — the duplication is small. For conflict resolution: local-FS conflicts surface mid-flight at parent directories inside `copy_single_item` (a file blocking `create_dir_all`), which the driver can't pre-detect via top-level `dest.get_metadata`; so the sync driver delegates conflict resolution to the closure entirely. Volume ops have only top-level conflicts that always reduce to `resolve_volume_conflict`, so the async driver owns that dispatch (uniform shape across all 3 volume ops, exactly what we want to deduplicate). The data-safety contract (closure never invoked for pre-skipped / resolved-as-Skip / post-cancel) is enforced in both shapes by the driver's loop structure and pinned by `transfer_driver_tests.rs`. See `docs/specs/transfer-driver-refactor-plan.md` § "Design decisions" and § "Concurrent driver scope" for the full rationale; the concurrent path stays inline in `copy_volumes_with_progress` (1-of-4 abstraction not worth its weight).
227227

228+
**Decision**: `copy_files_with_progress_inner` aligns `scan_result.files` to the driver's `&[PathBuf]` API via a paired `Vec<&FileInfo>` and a closure-captured `slice::Iter` advanced in lock-step with the driver iteration.
229+
**Why**: The sync driver iterates a generic `&[PathBuf]`, but the local-FS copy loop needs the full `FileInfo` (for `dest_path`, `is_symlink`, `size`, and the `SourceItemTracker` key). Three alternatives were rejected: (a) indexing into `scan_result.files` by `ctx.files_done_so_far` — wrong, the cumulative counter is bytes-affecting and includes bulk-skipped files, so the index would shift; (b) extending `TransferContext` with a generic associated payload — couples the driver to local-FS specifics; (c) cloning the `FileInfo` slice for `sources` — copies on the hot path. The iterator approach is O(0) memory beyond the path vec and matches the driver's iteration order exactly (`pre_skip_paths` is empty because we pre-filter `scan_result.files` ourselves, so the driver invokes the closure once per surviving file). The `.expect()` is justified inline; if the driver ever stopped calling the closure once per source the test suite would break.
230+
228231
**Decision**: `delete_files_start` routes to either `delete_files_with_progress` (local, uses `walkdir` + `fs::remove_file`) or `delete_volume_files_with_progress` (non-local, uses `Volume` trait) based on `volume_id`.
229232
**Why**: MTP volumes can't use `walkdir` or `fs::remove_*`. Rather than refactoring the existing local delete to go through the Volume trait (which would add overhead for local ops), we keep the fast local path and add a parallel volume-aware path. Both emit identical events so the frontend progress dialog works unchanged.
230233

0 commit comments

Comments
 (0)