A push leaves out only the refs its report refused, and a pull request's head copied from someone's fork says why it is not counted
A push now drops a ref from what it records only when the report says `ng` for it, or every ref when the unpack failed. A ref the report does not mention (a 200 with just `unpack ok`, or a report cut short) is left to record_push's checks against the store, so a ref that moved still starts its event, webhooks, workflows, rules and About run. The test merge's head copy comes from the pull request's source: its working copy, counted to this repository, or a branch of someone's own fork, counted to that fork. Neither is billed to the repository the pull request is in, whose owner did not push them; the comment, the architecture page, the billing guide and Drift's list now say so. The test fixtures spell their control bytes and newlines as escapes. Part of #319
- G1t-Session:
- ses_01m5fv26pcecxtmdsf3k7r74vg
7 files+79−600/7 viewed
| 118 | 118 | moved meanwhile, or a failed unpack; for a push over git, every ref it | |
| 119 | 119 | reports on refused). Only bytes the store took reach `stored_bytes`. A | |
| 120 | 120 | push and a write of one ref read the store's report the same way, one | |
| 121 | − | parser for both (`land.rs` `read_answer`, report-status v1 or v2), and a | |
| 122 | − | push records only the refs its report says moved: a ref the store refused | |
| 123 | − | starts no event, webhook or workflow, and a push refused whole records | |
| 124 | − | nothing. | |
| 121 | + | parser for both (`land.rs` `read_answer`, report-status v1 or v2). A push | |
| 122 | + | records no ref its report refused: that ref starts no event, webhook or | |
| 123 | + | workflow, and a push whose unpack failed, or whose every ref was refused, | |
| 124 | + | records nothing. A ref the report does not mention is checked against | |
| 125 | + | the store, as every recorded ref is. | |
| 125 | 126 | ||
| 126 | 127 | g1t's own copies are not reserved or counted: a pull request's test merge | |
| 127 | 128 | (`refs/pull/N/merge`, a merge commit g1t makes), its head copied in from | |
| 128 | − | the working copy, the head kept when a working copy is removed and the | |
| 129 | − | branch a removed working copy is made again with. What they hold was | |
| 130 | − | counted when it was pushed, to the repository that is the working copy's | |
| 131 | − | root, or was written by g1t. | |
| 129 | + | where the change is, the head kept when a working copy is removed and the | |
| 130 | + | branch a removed working copy is made again with. Each holds what g1t | |
| 131 | + | wrote, or what was counted when it was pushed: to the repository a working | |
| 132 | + | copy belongs to, or, for a pull request opened from a branch of someone's | |
| 133 | + | own fork, to that fork. The repository the pull request is in is never | |
| 134 | + | charged for objects its owner did not push. | |
| 132 | 135 | ||
| 133 | 136 | A write that stops without knowing whether the store kept it (the store | |
| 134 | 137 | answered with an error, stopped answering, answered a push with a fatal |
| 491 | 491 | pull requests' working copies, so what is charged is never more than what | |
| 492 | 492 | is stored. A push, merge or update that storage refused, for example | |
| 493 | 493 | because its branch moved meanwhile, is not counted. Neither are the copies | |
| 494 | − | g1t makes for its own use, such as a pull request's test merge: what they | |
| 495 | − | hold was counted when it was pushed, or g1t wrote it. Public repositories | |
| 494 | + | g1t makes for its own use, such as a pull request's test merge, or its head | |
| 495 | + | copied in from someone's fork: g1t wrote them, or they were counted where | |
| 496 | + | they were pushed (a fork's to the fork), never to a repository whose owner | |
| 497 | + | did not push them. Public repositories | |
| 496 | 498 | are never charged, and neither is a free workspace: its pushes to private | |
| 497 | 499 | repositories stop at 1 GB instead. On the plan they never stop. | |
| 498 | 500 |
| 116 | 116 | plus what each pull request's working copy holds beyond it, beside | |
| 117 | 117 | `stored_bytes`. It reads only. Above zero, g1t counts less than the store | |
| 118 | 118 | holds: pushes made with a credential straight to the store (sandboxes' | |
| 119 | − | `git_access`), ended pushes that landed, and g1t's own refs (a pull | |
| 120 | − | request's test merge, `refs/pull/N/merge`) are not counted. Below zero, | |
| 119 | + | `git_access`), ended pushes that landed, g1t's own refs (a pull | |
| 120 | + | request's test merge, `refs/pull/N/merge`), and a pull request's head | |
| 121 | + | copied in from someone's fork are not counted. Below zero, | |
| 121 | 122 | g1t counted objects that a later push made unreachable (a force push), or | |
| 122 | 123 | packs that sent objects the store already had: g1t asks for whole packs. | |
| 123 | 124 | Before #307, g1t also counted writes the store refused (a merge or update |
| 279 | 279 | } | |
| 280 | 280 | ||
| 281 | 281 | impl Answer { | |
| 282 | − | /// The refs the answer says moved: none when its unpack failed, and | |
| 283 | − | /// `None` when it says nothing either way (a fatal error, or no | |
| 284 | − | /// report), which leaves it to the store's refs. | |
| 285 | − | pub(crate) fn moved_refs(&self) -> Option<&[String]> { | |
| 282 | + | /// Whether a push records `git_ref` from this answer: not when the | |
| 283 | + | /// report refused it (`ng`), nor any ref when its unpack failed. A ref | |
| 284 | + | /// the report does not mention, or an answer that says nothing either | |
| 285 | + | /// way (a fatal error, no report), is left to the store's refs, which | |
| 286 | + | /// record_push checks (pushed.rs `pushed_to`, lib.rs `publish_deleted`): | |
| 287 | + | /// a report cut short never hides a ref that moved. | |
| 288 | + | pub(crate) fn records(&self, git_ref: &str) -> bool { | |
| 286 | 289 | match self { | |
| 287 | − | Answer::Report(report) if report.unpack_failed => Some(&[]), | |
| 288 | − | Answer::Report(report) => Some(&report.moved), | |
| 289 | − | Answer::Fatal(_) | Answer::Unread => None, | |
| 290 | + | Answer::Report(report) => !report.unpack_failed && !report.refused.iter().any(|refused| refused == git_ref), | |
| 291 | + | Answer::Fatal(_) | Answer::Unread => true, | |
| 290 | 292 | } | |
| 291 | 293 | } | |
| 292 | 294 | } |
| 2454 | 2454 | None | |
| 2455 | 2455 | }; | |
| 2456 | 2456 | // The store's answer, read once: its reservation settles by it, and | |
| 2457 | − | // only the refs it says moved are recorded. | |
| 2457 | + | // the refs it refused are not recorded. | |
| 2458 | 2458 | let reply = answered.as_ref().map(|(_, report)| land::read_answer(report)); | |
| 2459 | 2459 | if let Some(reserved) = &public_reserved { | |
| 2460 | 2460 | match public_storage::push_settlement(status, forwarded.rate_limited, reply.as_ref()) { | |
| ⋯ | |||
| 2530 | 2530 | } | |
| 2531 | 2531 | } | |
| 2532 | 2532 | after.ended(status, None); | |
| 2533 | − | // Only what the report says moved is recorded: a ref the store | |
| 2534 | − | // refused starts no event, webhook, workflow or rule, and a push | |
| 2535 | − | // refused whole records nothing. An answer that says neither (a | |
| 2536 | − | // fatal error, no report) leaves each ref to record_push, which | |
| 2537 | − | // checks where it is in the store. | |
| 2533 | + | // A ref the report refused is not recorded: it starts no event, | |
| 2534 | + | // webhook, workflow or rule, and a push whose unpack failed records | |
| 2535 | + | // nothing. A ref the report does not mention, or an answer that | |
| 2536 | + | // says neither (a fatal error, no report), is left to record_push, | |
| 2537 | + | // which checks where it is in the store. | |
| 2538 | 2538 | let (mut pushed, mut deleted) = (forwarded.pushed, forwarded.deleted); | |
| 2539 | − | if let Some(moved) = reply.as_ref().and_then(land::Answer::moved_refs) { | |
| 2540 | − | pushed.retain(|pushed| moved.contains(&pushed.git_ref)); | |
| 2541 | − | deleted.retain(|(git_ref, _)| moved.contains(git_ref)); | |
| 2539 | + | if let Some(reply) = &reply { | |
| 2540 | + | pushed.retain(|pushed| reply.records(&pushed.git_ref)); | |
| 2541 | + | deleted.retain(|(git_ref, _)| reply.records(git_ref)); | |
| 2542 | 2542 | } | |
| 2543 | 2543 | // A reserved push is counted by its reservation, settled above. | |
| 2544 | 2544 | let pack_bytes = if public_reserved.is_some() { 0 } else { forwarded.pack_bytes }; | |
| 900 | 900 | ||
| 901 | 901 | #[test] | |
| 902 | 902 | fn a_push_the_store_answered_with_a_fatal_error_stays_ended_for_staff() { | |
| 903 | − | let fatal = [crate::git_http::pkt_line(b"fatal: the remote end hung up unexpectedly | |
| 904 | − | "), b"0000".to_vec()].concat(); | |
| 905 | − | let after_progress = [crate::git_http::pkt_line(b"Receiving objects: 40% | |
| 906 | − | "), fatal.clone()].concat(); | |
| 907 | − | let err = [crate::git_http::pkt_line(b"ERR pack exceeds the store's limit | |
| 908 | − | "), b"0000".to_vec()].concat(); | |
| 903 | + | let fatal = [crate::git_http::pkt_line(b"\x03fatal: the remote end hung up unexpectedly\n"), b"0000".to_vec()].concat(); | |
| 904 | + | let after_progress = [crate::git_http::pkt_line(b"\x02Receiving objects: 40%\n"), fatal.clone()].concat(); | |
| 905 | + | let err = [crate::git_http::pkt_line(b"ERR pack exceeds the store's limit\n"), b"0000".to_vec()].concat(); | |
| 909 | 906 | for answer in [&fatal, &after_progress, &err] { | |
| 910 | 907 | let read = crate::land::read_answer(answer); | |
| 911 | 908 | assert!(matches!(read, Answer::Fatal(_)), "{read:?}"); | |
| ⋯ | |||
| 913 | 910 | // answered so is (Unclear). | |
| 914 | 911 | assert_eq!(push_settlement(200, false, Some(&read)), Settlement::End); | |
| 915 | 912 | assert_eq!(settled(answer, "refs/heads/main"), Settlement::End); | |
| 916 | − | // Nothing is announced from it: record_push checks the store. | |
| 917 | − | assert_eq!(read.moved_refs(), None); | |
| 913 | + | // Every ref is left to record_push, which checks the store. | |
| 914 | + | assert!(read.records("refs/heads/main")); | |
| 918 | 915 | } | |
| 919 | 916 | let outcome = push_outcome(200, Some(&crate::land::read_answer(&fatal))); | |
| 920 | 917 | assert_eq!(outcome, "The store answered the push with 200 and a fatal error: fatal: the remote end hung up unexpectedly"); | |
| ⋯ | |||
| 943 | 940 | let read = crate::land::read_answer(answer); | |
| 944 | 941 | let verdict = crate::land::verdict(answer, "refs/heads/main", true); | |
| 945 | 942 | let Answer::Report(report) = &read else { panic!("{read:?}") }; | |
| 946 | − | // The ref a write of one ref is counted for is the one a push | |
| 947 | − | // records, and a push refused whole is a refusal of that ref. | |
| 948 | − | let recorded = read.moved_refs().unwrap().iter().any(|moved| moved == "refs/heads/main"); | |
| 949 | − | assert_eq!(verdict == Verdict::Moved, recorded, "{report:?}"); | |
| 943 | + | // A write of one ref is counted when the report says it moved, | |
| 944 | + | // and a ref refused there is one a push does not record. | |
| 945 | + | assert_eq!(verdict == Verdict::Moved, report.moved("refs/heads/main"), "{report:?}"); | |
| 946 | + | assert_eq!(matches!(verdict, Verdict::Refused(_)), !read.records("refs/heads/main"), "{report:?}"); | |
| 950 | 947 | if refused_whole(report) { | |
| 951 | 948 | assert!(matches!(verdict, Verdict::Refused(_)), "{report:?}"); | |
| 952 | 949 | } | |
| ⋯ | |||
| 956 | 953 | assert_eq!(v2.moved, ["refs/heads/main"]); | |
| 957 | 954 | assert!(v2.refused.is_empty()); | |
| 958 | 955 | // One parser: neither reads the lines itself. | |
| 959 | − | let code = include_str!("public_storage.rs").split("#[cfg(test)] | |
| 960 | − | mod tests").next().unwrap_or_default(); | |
| 956 | + | let code = include_str!("public_storage.rs").split("#[cfg(test)]\nmod tests").next().unwrap_or_default(); | |
| 961 | 957 | assert!(!code.contains("starts_with(\"ok \")") && !code.contains("starts_with(\"ng \")")); | |
| 962 | 958 | } | |
| 963 | 959 | ||
| 964 | 960 | #[test] | |
| 965 | − | fn a_push_records_only_the_refs_its_report_says_moved() { | |
| 966 | − | let moved = |answer: Vec<u8>| crate::land::read_answer(&answer).moved_refs().map(<[String]>::to_vec); | |
| 967 | − | let main = || vec!["refs/heads/main".to_owned()]; | |
| 968 | − | // Every ref refused, or the unpack failed: none. | |
| 969 | − | assert_eq!(moved(report(&["unpack ok", "ng refs/heads/main non-fast-forward", "ng refs/tags/v1 already exists"])), Some(vec![])); | |
| 970 | − | assert_eq!(moved(report(&["unpack error: invalid pack", "ok refs/heads/main"])), Some(vec![])); | |
| 971 | − | // One moved beside one refused (a hidden ref): that one. | |
| 972 | − | assert_eq!(moved(banded(&["unpack ok", "ok refs/heads/main", "ng refs/pull/1/head deny updating a hidden ref"])), Some(main())); | |
| 973 | − | assert_eq!(moved(report(&["unpack ok", "ok refs/heads/main", "option refname refs/heads/main"])), Some(main())); | |
| 974 | − | // No report: left to the store's refs. | |
| 975 | − | assert_eq!(moved(Vec::new()), None); | |
| 961 | + | fn a_push_records_every_ref_but_those_its_report_refused() { | |
| 962 | + | let (main, tag, pull) = ("refs/heads/main", "refs/tags/v1", "refs/pull/1/head"); | |
| 963 | + | let recorded = |answer: Vec<u8>| { | |
| 964 | + | let read = crate::land::read_answer(&answer); | |
| 965 | + | [main, tag, pull].into_iter().filter(|git_ref| read.records(git_ref)).collect::<Vec<_>>() | |
| 966 | + | }; | |
| 967 | + | // Every ref refused, a tag among them: none. | |
| 968 | + | assert!(recorded(report(&["unpack ok", "ng refs/heads/main non-fast-forward", "ng refs/tags/v1 already exists", "ng refs/pull/1/head x"])).is_empty()); | |
| 969 | + | // The unpack failed: none, whatever the refs say. | |
| 970 | + | assert!(recorded(report(&["unpack error: invalid pack", "ok refs/heads/main"])).is_empty()); | |
| 971 | + | // One moved beside one refused (a hidden ref): all but the refused. | |
| 972 | + | assert_eq!(recorded(banded(&["unpack ok", "ok refs/heads/main", "ng refs/pull/1/head deny updating a hidden ref"])), [main, tag]); | |
| 973 | + | assert_eq!(recorded(report(&["unpack ok", "ok refs/heads/main", "option refname refs/heads/main", "ng refs/tags/v1 already exists"])), [main, pull]); | |
| 974 | + | // A report that mentions only some refs, or none (just `unpack ok`, | |
| 975 | + | // or cut short): the rest are left to record_push's store checks. | |
| 976 | + | assert_eq!(recorded(report(&["unpack ok", "ok refs/heads/main"])), [main, tag, pull]); | |
| 977 | + | assert_eq!(recorded(report(&["unpack ok"])), [main, tag, pull]); | |
| 978 | + | assert_eq!(recorded(banded(&["unpack ok", "ng refs/tags/v1 already exists"])), [main, pull]); | |
| 979 | + | // No report, or a fatal error: left to the store too. | |
| 980 | + | assert_eq!(recorded(Vec::new()), [main, tag, pull]); | |
| 981 | + | assert_eq!(recorded([crate::git_http::pkt_line(b"\x03fatal: hung up\n"), b"0000".to_vec()].concat()), [main, tag, pull]); | |
| 976 | 982 | // The push path filters what it records by it, pushed and deleted. | |
| 977 | 983 | let answer = body_of(include_str!("lib.rs"), "answer_git"); | |
| 978 | − | let filtered = answer.find("reply.as_ref().and_then(land::Answer::moved_refs)").expect("filtered by the report"); | |
| 984 | + | let filtered = answer.find("if let Some(reply) = &reply {").expect("filtered by the report"); | |
| 979 | 985 | let done = answer.find("after.push = Some(PushDone {").expect("recorded"); | |
| 980 | 986 | assert!(filtered < done); | |
| 981 | − | assert!(answer.contains("pushed.retain(|pushed| moved.contains(&pushed.git_ref));")); | |
| 982 | − | assert!(answer.contains("deleted.retain(|(git_ref, _)| moved.contains(git_ref));")); | |
| 987 | + | assert!(answer.contains("pushed.retain(|pushed| reply.records(&pushed.git_ref));")); | |
| 988 | + | assert!(answer.contains("deleted.retain(|(git_ref, _)| reply.records(git_ref));")); | |
| 983 | 989 | assert!(!answer[done..].contains("forwarded.pushed") && !answer[done..].contains("forwarded.deleted")); | |
| 984 | 990 | } | |
| 985 | 991 | ||
| 354 | 354 | let head = a.head.clone(); | |
| 355 | 355 | ||
| 356 | 356 | // The head, kept in the repository, its objects brought from the fork. | |
| 357 | − | // Not reserved or counted here: they are the pull request's working | |
| 358 | − | // copy's, counted to this repository (its root, registry.rs | |
| 359 | − | // `add_stored_bytes`) when they were pushed there, so counting the | |
| 360 | − | // copy too would count them twice. | |
| 357 | + | // Not reserved or counted here. They come from the pull request's | |
| 358 | + | // source (contracts `Pull::source_id`): its working copy, whose | |
| 359 | + | // pushes were counted to this repository (its root, registry.rs | |
| 360 | + | // `add_stored_bytes`) when they were made, so counting the copy too | |
| 361 | + | // would count them twice; or a branch of someone's own fork | |
| 362 | + | // (`head_repo`, `fork_of` NULL), whose pushes were counted to that | |
| 363 | + | // fork, not to this repository. Either way this repository's owner | |
| 364 | + | // did not push them, so they are not billed here; Drift counts the | |
| 365 | + | // fork's as the store holding more than g1t counts. | |
| 361 | 366 | let held_head = find(&refs, &head_ref); | |
| 362 | 367 | if held_head.as_deref() != Some(head.as_str()) { | |
| 363 | 368 | let write = git.access(Scope::Write).await?; |
Comments
Sign in to comment.