Pushing a new bookmark sends only what the remote lacks (closes #142) c8d9059e4e70
The push was negotiated against the remote's heads for that bookmark alone, so a bookmark new to the remote sent its whole history again, and OxygenHub's push protection scanned all of it. The heads of every bookmark the remote lists, of those held here, count as already there.
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
agent:claude@andymccall <[email protected]> · keyless, under your grant for changes,refs,seal · key ef2fa0419ab7
- When
- · 2026-10-25 17:34 UTC
- Signature
- ◈ sealed by agent:claude@andymccall
- Hash
c8d9059e4e70cd901264588cd73380954eb4442e3409111cae604586841ffe29- Depends on
- 157d02e95586 2b5ee371060e 72484195899a 8fd78eb52ad2 982864068a59 cb8b8082fa73 d82e44097323 e4595038502a
- Depended on by
- ff6987682725
- In bookmarks
- identical-sides main push-notes subrepo-pin-state
- In tags
- v0.10.0 v0.11.0 v0.7.0 v0.8.0 v0.9.0
- Edits
- 7 atoms, 0 tree operations, 3 files
Browse the tree at this change
modified CHANGELOG.md +1 −0
| … | 1 unchanged lines |
||
| 2 | 2 | ||
| 3 | 3 | ## Unreleased |
|
| 4 | 4 | ||
| 5 | + | - **Pushing a new bookmark sends only what the remote lacks (#142).** A push was worked out against the remote's heads for that bookmark alone, so a bookmark new to the remote sent its whole history again: slow on a big repository, and OxygenHub's push protection scanned all of it (and refused a proposal's bookmark for an example key in a change main had had for weeks). The heads of every bookmark the remote lists now count as already there. |
|
| 5 | 6 | - **A binary file's or subrepo pin's conflict can be settled from the command line (#140).** Two changes that replaced the same binary file, or moved the same subrepo pin, leave a replace conflict with no markers to edit, and every commit was refused until it was settled, which only a hosted edit could do. `oxy resolve PATH…` now settles it with what the working copy holds at the path (the file, or the subrepo's pin): the next commit is a resolution replacing every version with it. A path with no replace conflict is refused, and a choice is forgotten once committed. |
|
| 6 | 7 | - **The object store works where hard links are refused (#141).** Android forbids apps hard links in their own storage, so the OxygenHub app's phone runner could clone nothing (`Permission denied`). Where a link is refused, objects are renamed into place with `renameat2(RENAME_NOREPLACE)`: as atomic, and still never over a name that is taken. |
|
| 7 | 8 | - **Rendering a big tree is many times faster, and clone, pull, switch and `oxy workspace add` stop rendering it twice (#137).** Measured on the VS Code history's depth-0 clone (19,545 files, 166,425 changes), `oxy workspace add` took 467 s on Earth and 176 s on an 8-core workstation, 11 GB at its peak; it now takes 7.5 s and 1.8 GB on the workstation. Three causes: (1) whether one change depends on another, which rendering asks about every deleted line, was answered by a depth-first walk that compared a change with the one sought only on coming back up to it, so it went through thousands of changes first although the answer is nearly always a direct dependency (spec §8.1): 262,678 questions took 4.4 billion steps and 838 CPU-seconds, and now take 5 CPU-seconds, with the same answers; (2) finding the subrepos rendered every file of the tree, after every clone, pull, switch and `workspace add`, where reading the tree and the subrepos' pins is enough; (3) a new workspace started on the target state with an empty directory, so the checkout's first snapshot found the whole tree deleted and rendered it to say so. `OXY_TIMINGS=1` now also says how a checkout's files were ordered (`graph: single order`…) and how its blobs were read. |
|
| … | 267 unchanged lines |
modified crates/oxy-core/src/repo/sync.rs +20 −2
| … | 1493 unchanged lines |
||
| 1494 | 1494 | } |
|
| 1495 | 1495 | } |
|
| 1496 | 1496 | ||
| 1497 | - | fn open_named_remote(&self, name: &str) -> Result<Box<dyn crate::protocol::Remote>> { |
|
| 1497 | + | pub(crate) fn open_named_remote(&self, name: &str) -> Result<Box<dyn crate::protocol::Remote>> { |
|
| 1498 | 1498 | open_remote(&self.remote_url(name)?, &self.remote_options(name)) |
|
| 1499 | 1499 | } |
|
| 1500 | 1500 | ||
| … | 797 unchanged lines |
||
| 2298 | 2298 | ||
| 2299 | 2299 | /// `push_bookmark`, saying how a long push is going: one line per |
|
| 2300 | 2300 | /// batch sent (oxygen#95). |
|
| 2301 | + | /// What a push may take the remote to hold already: the heads of the |
|
| 2302 | + | /// bookmark pushed, and of every other bookmark it lists that are held |
|
| 2303 | + | /// here, so a new bookmark made from main sends only what main lacks |
|
| 2304 | + | /// (oxygen#142), not its whole history again. |
|
| 2305 | + | pub(crate) fn remote_holds( |
|
| 2306 | + | &self, |
|
| 2307 | + | refs: &[crate::protocol::RemoteRef], |
|
| 2308 | + | bookmark_heads: &[ChangeHash], |
|
| 2309 | + | ) -> Vec<ChangeHash> { |
|
| 2310 | + | let mut have: Vec<ChangeHash> = bookmark_heads.to_vec(); |
|
| 2311 | + | for h in refs.iter().flat_map(|r| &r.heads) { |
|
| 2312 | + | if !have.contains(h) && self.store.has(h) { |
|
| 2313 | + | have.push(*h); |
|
| 2314 | + | } |
|
| 2315 | + | } |
|
| 2316 | + | have |
|
| 2317 | + | } |
|
| 2318 | + | ||
| 2301 | 2319 | pub fn push_bookmark_with( |
|
| 2302 | 2320 | &self, |
|
| 2303 | 2321 | remote_name: &str, |
|
| … | 78 unchanged lines |
||
| 2382 | 2400 | return fail(format!("not a fast-forward: remote head {} is not in your state; pull first or use --force", h.short(12))); |
|
| 2383 | 2401 | } |
|
| 2384 | 2402 | } |
|
| 2385 | - | let to_send = self.negotiate(&remote_heads, &local_heads)?; |
|
| 2403 | + | let to_send = self.negotiate(&self.remote_holds(&refs, &remote_heads), &local_heads)?; |
|
| 2386 | 2404 | let key = self.secret_key()?.map(|(k, _)| k); |
|
| 2387 | 2405 | if let Some(key) = key { |
|
| 2388 | 2406 | // Seal this author's changes that carry no valid authenticator, |
|
| … | 394 unchanged lines |
modified crates/oxy-core/src/repo/tests.rs +41 −0
| … | 9204 unchanged lines |
||
| 9205 | 9205 | assert!(app_dir.join("notes.txt").exists()); |
|
| 9206 | 9206 | } |
|
| 9207 | 9207 | ||
| 9208 | + | /// oxygen#142: a new bookmark made from main sends what main lacks, not |
|
| 9209 | + | /// its whole history again. |
|
| 9208 | 9210 | #[test] |
|
| 9211 | + | fn a_new_bookmark_sends_only_what_the_remote_lacks() { |
|
| 9212 | + | hermetic(); |
|
| 9213 | + | let dir = tempfile::tempdir().unwrap(); |
|
| 9214 | + | let root = dir.path(); |
|
| 9215 | + | let _origin = Repo::init(&root.join("origin")).unwrap(); |
|
| 9216 | + | let url = root.join("origin").to_string_lossy().to_string(); |
|
| 9217 | + | let mut one = Repo::clone(&url, &root.join("one")) |
|
| 9218 | + | .unwrap() |
|
| 9219 | + | .with_home(root.join("home")); |
|
| 9220 | + | for n in 0..5 { |
|
| 9221 | + | write(&root.join("one"), "a.txt", format!("{n}\n").as_bytes()); |
|
| 9222 | + | one.commit(&format!("main {n}"), &[]).unwrap(); |
|
| 9223 | + | } |
|
| 9224 | + | assert!(one.push("origin", false).unwrap()[0].ok); |
|
| 9225 | + | one.bookmark_create("feature", None).unwrap(); |
|
| 9226 | + | one.bookmark_switch("feature", false).unwrap(); |
|
| 9227 | + | write(&root.join("one"), "b.txt", b"feature\n"); |
|
| 9228 | + | let added = one.commit("feature", &[]).unwrap(); |
|
| 9229 | + | let heads = one.bookmark_heads().unwrap(); |
|
| 9230 | + | let refs = one |
|
| 9231 | + | .open_named_remote("origin") |
|
| 9232 | + | .unwrap() |
|
| 9233 | + | .refs() |
|
| 9234 | + | .unwrap() |
|
| 9235 | + | .refs; |
|
| 9236 | + | assert!( |
|
| 9237 | + | refs.iter().all(|r| r.name != "feature"), |
|
| 9238 | + | "new to the remote" |
|
| 9239 | + | ); |
|
| 9240 | + | let sent = one |
|
| 9241 | + | .negotiate(&one.remote_holds(&refs, &[]), &heads) |
|
| 9242 | + | .unwrap(); |
|
| 9243 | + | assert_eq!(sent, vec![added], "only the change main lacks"); |
|
| 9244 | + | assert!(one.push_bookmark("origin", "feature", false).unwrap()[0].ok); |
|
| 9245 | + | let two = Repo::clone(&url, &root.join("two")).unwrap(); |
|
| 9246 | + | assert!(two.store().has(&added)); |
|
| 9247 | + | } |
|
| 9248 | + | ||
| 9249 | + | #[test] |
|
| 9209 | 9250 | fn a_bookmark_only_behind_its_remote_catches_up() { |
|
| 9210 | 9251 | // oxygen#74: switch takes the remote's state rather than a stale one. |
|
| 9211 | 9252 | hermetic(); |
|
| … | 1094 unchanged lines |