Remove or move every copy of a node at one place e8e6e0cac26a
Two lines of history that each put a node back where it was leave two live placements with the same parent and name; the tree shows them as one file. A removal or a move retired only the placement the tree showed, so the other outlived the change, and once its directory was removed it was an orphan. A change that removes or moves a node now retires its other placements at the same place too. Placements elsewhere, which the tree shows as a conflict, are left alone. Found by oxy git verify on VS Code at 9ab2056393c8: messageList.ts, moved back by 3ec707b8d7a and by the merge cc8e7e86641, kept one copy under src/vs/workbench/services/message/browser after the commit that removed it. A five-commit reproduction fails without the change. closes #88
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-10-10 02:45 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
e8e6e0cac26a5b4ee156d90a2c9f3879916a89737ebaeb0acbbd98e86366cccb- Depends on
- 170bb436e1d5 6e723b81f5b9 75167672ebcd 79b5a90ffd43 8fd78eb52ad2 982864068a59
- Depended on by
- c2e1d09047db 5552e69c7325
- In bookmarks
- bare-hashes carried-not-removed carry-identical doctor-trust identical-sides keyless-tags main merge-remote-bookmark only-help-103 paths-for-resolutions-and-amend push-notes show-stat subrepo-pin-state switch-says
- In tags
- v0.10.0 v0.11.0 v0.4.3 v0.4.4 v0.4.5 v0.4.6 v0.5.0 v0.5.1 v0.6.0 v0.7.0 v0.8.0 v0.9.0
- Edits
- 10 atoms, 0 tree operations, 3 files
Browse the tree at this change
modified CHANGELOG.md +2 −0
| … | 1 unchanged lines |
||
| 2 | 2 | ||
| 3 | 3 | ## Unreleased |
|
| 4 | 4 | ||
| 5 | + | - **Removing a file removes every copy of it (#88).** When two lines of history each put a file back in the same place, it has two placements there and shows as one file. Removing or moving it retired only one, so the other stayed behind, and once its directory went it was an orphan: a conflict in the state. It is the one difference the Git import of VS Code had in its first 31 000 commits. Both now go together, in a commit and in a Git import. |
|
| 6 | + | ||
| 5 | 7 | - **A quicker log and diff on long histories (#94, first part).** `oxy log` no longer reads the working copy's metadata through the store's cache, walks the history with a fast hash and without copying each change's metadata, and works out the shortest unique id prefixes for the changes it shows rather than for every change. The state index is read without copying the file histories in it, and its change list is checked on every core. On VS Code at 20 000 commits, `log -n 20` went from 74 ms to 40 ms and loading the state index from 43 ms to 28 ms, with the same output. |
|
| 6 | 8 | ||
| 7 | 9 | - **Faster commits, and the state index after a checkpoint (#93).** A state loaded through a checkpoint read the changes after it whole, text included, so it could never be saved as the workspace's state index and every command after a checkpoint loaded history the long way (25 s for the first commit on VS Code at 20 000 commits). It now reads them as the index needs them. `oxy commit PATHS` no longer takes a second snapshot to count what it left when it took everything. A one-file commit on VS Code went from about 450 ms to 300 ms; 70 ms of that is the store's fsync, which stays. |
|
| … | 152 unchanged lines |
modified crates/oxy-core/src/repo/snapshot.rs +33 −3
| … | 2260 unchanged lines |
||
| 2261 | 2261 | e: &FsEntry, |
|
| 2262 | 2262 | expected: &crate::state::FileState, |
|
| 2263 | 2263 | ) -> Result<()> { |
|
| 2264 | - | let entry = &self.base_snapshot.tree.entries[old]; |
|
| 2264 | + | let placement = self.base_snapshot.tree.entries[old].placement; |
|
| 2265 | 2265 | let (dir, name) = split_path(new); |
|
| 2266 | 2266 | let parent = self.ensure_dir(dir); |
|
| 2267 | 2267 | self.tree_ops.push(TreeOp::Move { |
|
| 2268 | 2268 | node: expected.node, |
|
| 2269 | - | old: op_ref(entry.placement), |
|
| 2269 | + | old: op_ref(placement), |
|
| 2270 | 2270 | parent, |
|
| 2271 | 2271 | name: name.to_string(), |
|
| 2272 | 2272 | }); |
|
| 2273 | + | for k in self.same_place(&expected.node, placement) { |
|
| 2274 | + | self.tree_ops.push(TreeOp::RemoveNode { |
|
| 2275 | + | node: expected.node, |
|
| 2276 | + | placement: op_ref(k), |
|
| 2277 | + | }); |
|
| 2278 | + | } |
|
| 2273 | 2279 | self.update_path(repo, old, e, expected)?; |
|
| 2274 | 2280 | // update_path reports under the old path; the rename is what the user sees. |
|
| 2275 | 2281 | self.statuses.retain(|s| s.path != old); |
|
| … | 85 unchanged lines |
||
| 2361 | 2367 | } |
|
| 2362 | 2368 | } |
|
| 2363 | 2369 | ||
| 2370 | + | /// The node's other live placements at the same place as `k`, the one |
|
| 2371 | + | /// the tree shows: two lines of history that put a node back where it |
|
| 2372 | + | /// was each leave a placement, and the tree shows them as one file. A |
|
| 2373 | + | /// change that removes or moves it retires them together, or the copy |
|
| 2374 | + | /// left behind outlives its directory (oxygen#88). |
|
| 2375 | + | fn same_place(&self, node: &NodeId, k: (ChangeHash, u32)) -> Vec<(ChangeHash, u32)> { |
|
| 2376 | + | let tree = self.base.tree(); |
|
| 2377 | + | let Some(shown) = tree.placement(&k) else { |
|
| 2378 | + | return Vec::new(); |
|
| 2379 | + | }; |
|
| 2380 | + | tree.live_placements(node) |
|
| 2381 | + | .into_iter() |
|
| 2382 | + | .filter(|(o, p)| *o != k && p.parent == shown.parent && p.name == shown.name) |
|
| 2383 | + | .map(|(o, _)| o) |
|
| 2384 | + | .collect() |
|
| 2385 | + | } |
|
| 2386 | + | ||
| 2364 | 2387 | fn remove_path(&mut self, path: &str, expected: &crate::state::FileState) { |
|
| 2365 | 2388 | let entry = &self.base_snapshot.tree.entries[path]; |
|
| 2389 | + | let placement = entry.placement; |
|
| 2366 | 2390 | self.tree_ops.push(TreeOp::RemoveNode { |
|
| 2367 | 2391 | node: expected.node, |
|
| 2368 | - | placement: op_ref(entry.placement), |
|
| 2392 | + | placement: op_ref(placement), |
|
| 2369 | 2393 | }); |
|
| 2394 | + | for k in self.same_place(&expected.node, placement) { |
|
| 2395 | + | self.tree_ops.push(TreeOp::RemoveNode { |
|
| 2396 | + | node: expected.node, |
|
| 2397 | + | placement: op_ref(k), |
|
| 2398 | + | }); |
|
| 2399 | + | } |
|
| 2370 | 2400 | if path.contains(".oxy-conflict-") || expected.is_conflicted() { |
|
| 2371 | 2401 | self.resolves = true; |
|
| 2372 | 2402 | } |
|
| … | 674 unchanged lines |
modified crates/oxy-core/src/repo/tests.rs +70 −0
| … | 7979 unchanged lines |
||
| 7980 | 7980 | let touches = repo.log_revset("touches(\"c.txt\") ~ @").unwrap(); |
|
| 7981 | 7981 | assert_eq!(hashes(touches), vec![c3, c1]); |
|
| 7982 | 7982 | } |
|
| 7983 | + | ||
| 7984 | + | /// oxygen#88 (vscode 9ab2056393c8): two lines of history that each move a |
|
| 7985 | + | /// file back where it was leave two placements at one place, which the |
|
| 7986 | + | /// tree shows as one file. Removing it, and then its directory, retires |
|
| 7987 | + | /// both, so no copy is left under a directory that is gone. |
|
| 7988 | + | #[test] |
|
| 7989 | + | fn git_import_removes_every_copy_of_a_file_at_one_place() { |
|
| 7990 | + | hermetic(); |
|
| 7991 | + | if std::process::Command::new("git") |
|
| 7992 | + | .arg("--version") |
|
| 7993 | + | .output() |
|
| 7994 | + | .is_err() |
|
| 7995 | + | { |
|
| 7996 | + | eprintln!("git not installed; skipping"); |
|
| 7997 | + | return; |
|
| 7998 | + | } |
|
| 7999 | + | let dir = tempfile::tempdir().unwrap(); |
|
| 8000 | + | let root = dir.path(); |
|
| 8001 | + | let g = root.join("src.git"); |
|
| 8002 | + | fs::create_dir_all(&g).unwrap(); |
|
| 8003 | + | let git = |args: &[&str]| { |
|
| 8004 | + | let out = std::process::Command::new("git") |
|
| 8005 | + | .arg("-C") |
|
| 8006 | + | .arg(&g) |
|
| 8007 | + | .args([ |
|
| 8008 | + | "-c", |
|
| 8009 | + | "user.name=Grace", |
|
| 8010 | + | "-c", |
|
| 8011 | + | "[email protected]", |
|
| 8012 | + | "-c", |
|
| 8013 | + | "commit.gpgsign=false", |
|
| 8014 | + | ]) |
|
| 8015 | + | .args(args) |
|
| 8016 | + | .output() |
|
| 8017 | + | .unwrap(); |
|
| 8018 | + | assert!( |
|
| 8019 | + | out.status.success(), |
|
| 8020 | + | "git {args:?}: {}", |
|
| 8021 | + | String::from_utf8_lossy(&out.stderr) |
|
| 8022 | + | ); |
|
| 8023 | + | }; |
|
| 8024 | + | git(&["init", "-q", "-b", "main"]); |
|
| 8025 | + | write(&g, "d/list.txt", b"one\ntwo\nthree\nfour\n"); |
|
| 8026 | + | write(&g, "keep.txt", b"k\n"); |
|
| 8027 | + | git(&["add", "."]); |
|
| 8028 | + | git(&["commit", "-qm", "base"]); |
|
| 8029 | + | fs::create_dir_all(g.join("d/sub")).unwrap(); |
|
| 8030 | + | git(&["mv", "d/list.txt", "d/sub/list.txt"]); |
|
| 8031 | + | git(&["commit", "-qm", "into sub"]); |
|
| 8032 | + | git(&["checkout", "-qb", "side"]); |
|
| 8033 | + | git(&["mv", "d/sub/list.txt", "d/list.txt"]); |
|
| 8034 | + | git(&["commit", "-qm", "back, on side"]); |
|
| 8035 | + | git(&["checkout", "-q", "main"]); |
|
| 8036 | + | git(&["mv", "d/sub/list.txt", "d/list.txt"]); |
|
| 8037 | + | git(&["commit", "-qm", "back, on main"]); |
|
| 8038 | + | git(&["merge", "-q", "--no-edit", "side"]); |
|
| 8039 | + | git(&["rm", "-rq", "d"]); |
|
| 8040 | + | git(&["commit", "-qm", "d goes"]); |
|
| 8041 | + | ||
| 8042 | + | let repo = Repo::init(&root.join("o")).unwrap(); |
|
| 8043 | + | repo.import_git(&g, "main").unwrap(); |
|
| 8044 | + | let v = repo.verify_git_import(&g, "main").unwrap(); |
|
| 8045 | + | assert!(v.mismatches.is_empty(), "{:?}", v.mismatches); |
|
| 8046 | + | let st = repo |
|
| 8047 | + | .load_state_history(&repo.bookmark_heads().unwrap()) |
|
| 8048 | + | .unwrap(); |
|
| 8049 | + | let view = st.tree().view(); |
|
| 8050 | + | assert!(view.conflicts.is_empty(), "{:?}", view.conflicts); |
|
| 8051 | + | assert_eq!(view.entries.keys().collect::<Vec<_>>(), vec!["keep.txt"]); |
|
| 8052 | + | } |