diff --git a/CHANGELOG.md b/CHANGELOG.md index 146ba146..82d47e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **`rm` no longer counts a commit and the commit that reverts it** (#664). Neither has a + copy on a remote, and a branch that changes nothing proves nothing to the squash rule, + so after #653 and #659 `rm` and `dl --ls --json` still counted both. Now a counted + commit whose tree is the tree under the counted commit it sits on drops out with that + commit, when both have one parent and the lower one changes something. A commit is in + one pair at most and the pairs are taken from the bottom, so a revert of a revert + still counts. A ref or a worktree's HEAD on the reverted commit holds the pair back, + and so does a second commit that grew from it, such as a branch or the stash. A merge, + a root commit, an empty commit and a revert of a commit that another rule cleared + still count, and so does an earlier draft of a commit that was edited later. + ## [0.59.0] - 2026-09-30 ### Added diff --git a/docs/cleanup.md b/docs/cleanup.md index d6b5d4c1..22e06fed 100644 --- a/docs/cleanup.md +++ b/docs/cleanup.md @@ -984,7 +984,7 @@ cherry-pick and a squash merge all put the same change on the remote under a new hash, so by hash the old commit is on no remote ref. A kinisi_ros workspace refused `rm` over 23 commits that way, and none of them was lost: its remote branch had been rebased, a merged PR had been squashed into `main`, and five of the commits -were merges of `origin/main`. Three more rules now run after the count: +were merges of `origin/main`. Four more rules now run after the count: - A commit drops out when a remote ref holds a copy of it: a commit that makes the same change in the same place. The candidates come from the patch id, @@ -1032,12 +1032,29 @@ were merges of `origin/main`. Three more rules now run after the count: branch's first-parent line, so work added after the squash stays counted and the squashed commits under it do not. A branch that changes nothing since it left the remote ref proves nothing, because any merge of it gives the remote - ref's tree, so a commit and its revert on their own still count. A commit + ref's tree, so this rule leaves a commit and its revert on their own to the + next one. A commit drops out only when every ref that reaches it is a branch that passed: a second branch that grew from it and does not pass holds it back, and so do the stash, a local tag and a detached HEAD. - -All three rules can only take commits out of the count, and only when git +- A commit and the commit that reverts it drop out together. Together they + change nothing, so a push of them would change nothing either. A kinisi_ros + workspace refused `rm` over a probe commit and its revert, left on a backup + branch after the PR was squashed. The pair is two counted commits with one + parent each, the second on top of the first, where the second's tree is the + tree under the first and the first's tree is not. Trees, not patches, so the + revert must take back the whole change, byte for byte. An empty commit is + never the first of a pair, because its message is all it holds. A root + commit is never one either, because there is no tree under it, and nor is a + merge on either side. When another rule already cleared the first commit, + the revert stays counted: it is the only record of taking that change out + again. A commit is in one pair at most, and the pairs are taken from the + bottom, so a revert of a revert stays counted, because it puts the change + back. The pair stays counted when anything else holds the state of the + first commit: a branch, a tag or a worktree's HEAD on it, or a second + commit that grew from it, such as another branch or the stash. + +All four rules can only take commits out of the count, and only when git answers. A question git refuses clears nothing, and so does a merge with a conflict. That is the limit of the third rule: when the remote edited the lines the squash wrote, the merge conflicts, and the squashed commits still count, diff --git a/rust/devlaunch-core/src/clients/git.rs b/rust/devlaunch-core/src/clients/git.rs index 9ac5ed88..2f6eab1e 100644 --- a/rust/devlaunch-core/src/clients/git.rs +++ b/rust/devlaunch-core/src/clients/git.rs @@ -46,7 +46,7 @@ //! module, so the spans are wired in M4b/M5 against the real registry rather than //! guessed at here. The names above are the list to wire. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io::Read as _; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -1230,6 +1230,51 @@ impl<'r> Git<'r> { .map(|stdout| stdout.lines().map(str::to_owned).collect()) } + /// Every commit no remote-tracking ref contains, with its tree and its + /// parents, and the tree of each pushed commit one of them has as a parent. + /// + /// What the revert rule reads: a commit and its revert, and whether any + /// other commit grew from the reverted one. `--all` with nothing excluded, + /// so a commit that only a tag or `refs/original` reaches is still in the + /// graph. When that tag came from the remote, or it is `refs/original`, such + /// a commit is never counted, so it can only hold a pair back. + /// `--boundary` lists the pushed parents too, marked `-`, which is where the + /// tree under a commit made on top of the remote comes from. + /// + /// `--no-show-signature` because `log.showSignature` would put gpg's lines + /// between the commits. A line in any other shape reads as no graph at + /// all (`None`): a commit left out of it could be the one that holds a pair + /// back. + pub(crate) fn unpushed_graph(&self, clone: &Path) -> GitAnswer> { + self.about( + clone, + &[ + "log", + "--no-color", + "--no-show-signature", + "--boundary", + "--format=%m %H %T %P", + "--all", + "--not", + "--remotes", + ], + ) + .map(|stdout| commit_graph_in(&stdout)) + } + + /// The commit each ref and each worktree's HEAD names, tags peeled. + /// + /// The revert rule's other half: a ref on the reverted commit holds the + /// state with that commit's change in it. `rev-list --all` reads every + /// ref under `refs/` and the HEAD of every worktree, detached or not, and + /// `--no-walk` lists the tips without their history. A linked worktree's + /// own `refs/worktree/*` and `refs/bisect/*` are not read, as the count + /// does not read them either. + pub(crate) fn ref_tips(&self, clone: &Path) -> GitAnswer> { + self.about(clone, &["rev-list", "--no-walk", "--all"]) + .map(|stdout| stdout.lines().map(str::to_owned).collect()) + } + /// Every tag in the bare cache at *bare*, with the object each one names. /// /// `--git-dir` and no work tree, because a bare has none, and no cwd, because @@ -2201,6 +2246,76 @@ impl RemoteRef { } } +/// The unpushed commits of a clone, from [`Git::unpushed_graph`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct CommitGraph { + /// Each commit no remote-tracking ref contains, by full hash. + unpushed: HashMap, + /// The tree of each pushed commit that an unpushed one has as a parent. + pushed_trees: HashMap, +} + +/// One unpushed commit in a [`CommitGraph`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct GraphCommit { + pub(crate) tree: String, + /// Full hashes, the first parent first. + pub(crate) parents: Vec, +} + +impl CommitGraph { + /// Every unpushed commit, by full hash. + pub(crate) fn unpushed(&self) -> impl Iterator { + self.unpushed + .iter() + .map(|(hash, commit)| (hash.as_str(), commit)) + } + + /// The unpushed commit *hash*, or `None` when it is pushed or unknown. + pub(crate) fn commit(&self, hash: &str) -> Option<&GraphCommit> { + self.unpushed.get(hash) + } + + /// The tree of *hash*, pushed or not, when the graph lists it. + pub(crate) fn tree_of(&self, hash: &str) -> Option<&str> { + self.unpushed + .get(hash) + .map(|commit| commit.tree.as_str()) + .or_else(|| self.pushed_trees.get(hash).map(String::as_str)) + } +} + +/// The graph in [`Git::unpushed_graph`] output, one ` +/// ` line per commit, or `None` when a line is in any other shape. +fn commit_graph_in(output: &str) -> Option { + let mut graph = CommitGraph { + unpushed: HashMap::new(), + pushed_trees: HashMap::new(), + }; + for line in output.lines() { + let mut fields = line.split(' ').filter(|field| !field.is_empty()); + let (Some(mark), Some(hash), Some(tree)) = (fields.next(), fields.next(), fields.next()) + else { + return None; + }; + let parents: Vec = fields.map(str::to_owned).collect(); + match mark { + ">" => { + let commit = GraphCommit { + tree: tree.to_owned(), + parents, + }; + graph.unpushed.insert(hash.to_owned(), commit); + } + "-" => { + graph.pushed_trees.insert(hash.to_owned(), tree.to_owned()); + } + _ => return None, + } + } + Some(graph) +} + /// What [`Git::text_merge`] found a clone can merge with: the `--attr-source` /// that names the empty tree in the clone's object format. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/rust/devlaunch-core/src/domain/workspace_state.rs b/rust/devlaunch-core/src/domain/workspace_state.rs index fc264a12..77de72e0 100644 --- a/rust/devlaunch-core/src/domain/workspace_state.rs +++ b/rust/devlaunch-core/src/domain/workspace_state.rs @@ -609,6 +609,10 @@ fn unsaved(git: &Git<'_>, clone: &Path, bare: BareCache<'_>) -> Unsaved { if !left.is_empty() { copied.extend(squashed_onto_a_remote(git, clone, &left, &local_tags)); } + let left = not_among(listed.iter(), &copied); + if !left.is_empty() { + copied.extend(reverted_in_pairs(git, clone, &left)); + } if let Some(commits) = NonEmpty::of(not_among(listed.iter(), &copied)) { let by_tags = owed_to_tags(git, clone, &local_tags) .and_then(|by_tags| by_tags.leaving_out(&copied)); @@ -797,6 +801,129 @@ fn squashed_onto_a_remote( cleared } +/// The counted commits that come in pairs of a commit and its revert, as full +/// hashes. +/// +/// What [`already_on_a_remote`] and [`squashed_onto_a_remote`] leave: a commit +/// and the commit that reverts it. Neither has a copy on a remote, and a branch +/// that changes nothing proves nothing to the squash rule. A kinisi_ros +/// workspace refused `rm` over a probe commit and its revert that way, with +/// the PR (kinisi_ros#11898) squashed into `main`. Together the two commits +/// change nothing, so a push of them would change nothing either. +/// +/// **A pair** is a counted commit *R* whose one parent is a counted commit *C* +/// with one parent of its own, where *R*'s tree is the tree under *C* and +/// *C*'s tree is not. Trees, not patches, so the revert must take back the +/// whole change, byte for byte. So: +/// +/// - an empty *C* is no pair, because an empty commit holds only its message; +/// - a root *C* is no pair, because there is no tree under it; +/// - a merge is no pair on either side, because its change is not one +/// commit's; +/// - a *C* that another rule cleared is no pair, because then *R* is the one +/// record of taking a change on a remote out again. +/// +/// **A commit is in one pair at most, and the pairs are taken from the +/// bottom.** A revert of a revert puts the change back, so the third commit +/// holds what the first did and stays counted while the first two drop out. +/// +/// **A pair drops out only when nothing else holds the state of *C*.** A ref or +/// a worktree's HEAD on *C* holds it, and so does a second commit that grew +/// from *C*: a branch, or the stash. Then both commits stay counted. Every ref +/// that reaches *C* then reaches it through *R*. "Every ref" is every ref +/// `--all` reads, as for the count: a linked worktree's own `refs/worktree/*` +/// and `refs/bisect/*` are not among them. +/// +/// A cycle in the graph, which only `refs/replace` or grafts can make, holds +/// no pair. +/// +/// It runs only when the other rules left a commit counted, and it costs one +/// `git log` of the unpushed graph, and one `rev-list` of the ref tips only +/// when a pair is found. **Every failure clears nothing**: a refusal on either, +/// or a graph in a shape this cannot read. +fn reverted_in_pairs(git: &Git<'_>, clone: &Path, counted: &[String]) -> Vec { + let Some(Some(graph)) = git.unpushed_graph(clone).said() else { + return Vec::new(); + }; + let is_counted = |hash: &str| { + graph.commit(hash).is_some() + && counted + .iter() + .any(|line| is_among(line, std::slice::from_ref(&hash.to_owned()))) + }; + let reverts = |hash: &str| -> Option { + let revert = graph.commit(hash)?; + let [reverted] = revert.parents.as_slice() else { + return None; + }; + let [under] = graph.commit(reverted)?.parents.as_slice() else { + return None; + }; + let under_tree = graph.tree_of(under)?; + let changed = graph.tree_of(reverted)? != under_tree; + (changed && revert.tree == under_tree && is_counted(hash) && is_counted(reverted)) + .then(|| reverted.clone()) + }; + struct Pair { + reverted: String, + revert: String, + } + let mut taken: HashMap> = HashMap::new(); + for (hash, _) in graph.unpushed() { + let mut chain: Vec = Vec::new(); + let mut at = hash.to_owned(); + let mut walked: HashSet = HashSet::new(); + while !taken.contains_key(&at) { + if !walked.insert(at.clone()) { + // A cycle, which only `refs/replace` or grafts can make: git + // shows the replaced parents. Nothing on it is a pair. + for pair in chain.drain(..) { + taken.insert(pair.revert, None); + } + break; + } + match reverts(&at) { + Some(reverted) => { + chain.push(Pair { + reverted: reverted.clone(), + revert: at, + }); + at = reverted; + } + None => { + taken.insert(at.clone(), None); + } + } + } + for pair in chain.into_iter().rev() { + let under_a_pair = matches!(taken.get(&pair.reverted), Some(Some(_))); + taken.insert(pair.revert.clone(), (!under_a_pair).then_some(pair)); + } + } + let pairs: Vec = taken.into_values().flatten().collect(); + if pairs.is_empty() { + return Vec::new(); + } + let Some(tips) = git.ref_tips(clone).said() else { + return Vec::new(); + }; + let mut children: HashMap<&str, usize> = HashMap::new(); + for (_, commit) in graph.unpushed() { + for parent in &commit.parents { + *children.entry(parent.as_str()).or_default() += 1; + } + } + let mut cleared: Vec = pairs + .into_iter() + .filter(|pair| { + !tips.contains(&pair.reverted) && children.get(pair.reverted.as_str()) == Some(&1) + }) + .flat_map(|pair| [pair.reverted, pair.revert]) + .collect(); + cleared.sort(); + cleared +} + /// How far down a branch [`squashed_onto_a_remote`] looks for a commit that /// passes. /// diff --git a/rust/devlaunch-core/src/domain/workspace_state/tests.rs b/rust/devlaunch-core/src/domain/workspace_state/tests.rs index 3c03c8c5..5348378e 100644 --- a/rust/devlaunch-core/src/domain/workspace_state/tests.rs +++ b/rust/devlaunch-core/src/domain/workspace_state/tests.rs @@ -821,10 +821,11 @@ fn a_merge_that_dropped_a_deleted_side_branch_leaves_that_branch_counted() { } #[test] -fn a_commit_and_its_revert_are_not_cleared_by_a_remote_that_moved() { +fn the_squash_rule_does_not_clear_a_commit_and_its_revert_by_a_remote_that_moved() { // The branch changes nothing since it left `origin/feature`, so any merge of // it into `origin/feature` gives `origin/feature`'s tree. An empty change - // proves nothing, and the two commits stay counted. + // proves nothing, so the squash rule leaves the two commits counted. The + // revert rule is what clears them, and it is asked on its own here. let fixture = Fixture::new(); let clone = fixture.clone(); write(&clone.join("notes.txt"), "one\n"); @@ -836,8 +837,21 @@ fn a_commit_and_its_revert_are_not_cleared_by_a_remote_that_moved() { commit(&mate, "mate"); git(&mate, &["push", "-q", "origin", "feature"]); git(&clone, &["fetch", "-q", "origin"]); + let counted: Vec = git( + &clone, + &["log", "--oneline", "feature", "--not", "--remotes"], + ) + .lines() + .map(str::to_owned) + .collect(); + assert_eq!(counted.len(), 2); + let runner = ProcessRunner::new(); - assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); + assert_eq!( + squashed_onto_a_remote(&Git::new(&runner), &clone, &counted, &[]), + Vec::::new() + ); + assert_eq!(held(&clone), Unsaved::NothingToLose); } #[test] @@ -1303,6 +1317,530 @@ fn a_squash_eight_commits_under_the_tip_is_not_looked_for() { assert_eq!(would_lose(&held(&clone)), "11 unpushed commit(s)"); } +// ------------------------------------------------- a commit and its revert + +/// A commit on `feature` that writes `probe.txt`, then the commit +/// `git revert` makes of it. +fn a_commit_and_its_revert(clone: &Path) { + write(&clone.join("probe.txt"), "probe\n"); + commit(clone, "probe (revert before merge)"); + git_as_author(clone, &["revert", "--no-edit", "HEAD"]); +} + +/// The unpushed commits a `WouldLose` names, sorted, or a failure naming the +/// arm that came back. +fn unpushed_commits(unsaved: &Unsaved) -> Vec { + let Unsaved::WouldLose(losses) = unsaved else { + panic!("expected a WouldLose: {unsaved:?}"); + }; + let mut commits: Vec = losses + .iter() + .flat_map(|loss| match loss { + Loss::Unpushed { commits, .. } => commits.iter().cloned().collect(), + _ => Vec::new(), + }) + .collect(); + commits.sort(); + commits +} + +/// *revs* as `log --oneline` names them, sorted. +fn onelines(clone: &Path, revs: &[&str]) -> Vec { + let mut lines: Vec = revs + .iter() + .map(|rev| git(clone, &["log", "--oneline", "--no-color", "-1", rev])) + .collect(); + lines.sort(); + lines +} + +#[test] +fn a_commit_and_its_revert_hold_nothing_unsaved() { + // kinisi_ros#11898's workspace: a probe commit and its revert, adjacent on + // a branch whose PR was squashed. Together they change nothing, so + // deleting them loses nothing. + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + assert_eq!(by_sha(&clone, "feature"), 2); + + assert_eq!(held(&clone), Unsaved::NothingToLose); +} + +#[test] +fn a_commit_and_its_revert_hold_nothing_unsaved_in_a_sha256_repository() { + let fixture = Fixture::named_by("sha256"); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + + assert_eq!(held(&clone), Unsaved::NothingToLose); +} + +#[test] +fn the_revert_rule_pairs_signed_commits_where_log_shows_signatures() { + // `log.showSignature` puts the signature check's lines between the + // commits the revert rule reads, and a line it cannot read clears nothing. + // The rule is asked on its own, with the count listed without signatures. + let fixture = Fixture::new(); + let clone = fixture.clone(); + let key = fixture.path("signing_key"); + let made = Command::new("ssh-keygen") + .args(["-q", "-t", "ed25519", "-N", "", "-C", "t@t", "-f"]) + .arg(&key) + .output() + .expect("ssh-keygen is installed"); + assert!(made.status.success(), "{made:?}"); + for (name, value) in [ + ("gpg.format", "ssh"), + ("user.signingkey", key.to_str().expect("utf-8")), + ("commit.gpgsign", "true"), + ("log.showSignature", "true"), + ] { + git(&clone, &["config", name, value]); + } + a_commit_and_its_revert(&clone); + assert!(git(&clone, &["cat-file", "commit", "HEAD~1"]).contains("gpgsig")); + let counted: Vec = git( + &clone, + &[ + "log", + "--oneline", + "--no-color", + "--no-show-signature", + "feature", + "--not", + "--remotes", + ], + ) + .lines() + .map(str::to_owned) + .collect(); + let mut pair = vec![ + git(&clone, &["rev-parse", "HEAD~1"]), + git(&clone, &["rev-parse", "HEAD"]), + ]; + pair.sort(); + let runner = ProcessRunner::new(); + + assert_eq!( + reverted_in_pairs(&Git::new(&runner), &clone, &counted), + pair + ); +} + +#[test] +fn the_commits_around_a_commit_and_its_revert_stay_counted() { + let fixture = Fixture::new(); + let clone = fixture.clone(); + write(&clone.join("before.txt"), "work\n"); + commit(&clone, "before"); + a_commit_and_its_revert(&clone); + write(&clone.join("after.txt"), "work\n"); + commit(&clone, "after"); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_commit_that_undoes_only_part_of_the_one_before_stays_counted() { + // The tree after the second commit is not the tree before the first, so + // `probe.txt` exists nowhere else. + let fixture = Fixture::new(); + let clone = fixture.clone(); + write(&clone.join("probe.txt"), "one\ntwo\n"); + commit(&clone, "draft"); + write(&clone.join("probe.txt"), "one\n"); + commit(&clone, "edit the draft"); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn two_empty_commits_stay_counted() { + // The second one's tree is the tree under the first, as a revert's is, but + // an empty commit holds only its message, and nothing compares a message. + let fixture = Fixture::new(); + let clone = fixture.clone(); + for message in ["the only record", "and another"] { + git_as_author(&clone, &["commit", "-q", "--allow-empty", "-m", message]); + } + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_root_commit_and_its_revert_stay_counted() { + // A root commit has no tree under it to compare with. + let fixture = Fixture::new(); + let clone = fixture.clone(); + git(&clone, &["checkout", "-q", "--orphan", "orphan"]); + git(&clone, &["rm", "-q", "-r", "-f", "."]); + write(&clone.join("n.txt"), "x\n"); + commit(&clone, "a root of its own"); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + assert_eq!(by_sha(&clone, "orphan"), 2); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_revert_of_a_revert_stays_counted() { + // The third commit puts `probe.txt` back, so the clone is its only copy. + // A commit is in one pair at most, and the pairs are taken from the + // bottom, so the first two drop out and the third stays. + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + let third = onelines(&clone, &["feature"]); + + let unsaved = held(&clone); + + assert_eq!(would_lose(&unsaved), "1 unpushed commit(s)"); + assert_eq!(unpushed_commits(&unsaved), third); +} + +#[test] +fn a_revert_of_a_revert_that_is_reverted_again_holds_nothing_unsaved() { + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + assert_eq!(by_sha(&clone, "feature"), 4); + + assert_eq!(held(&clone), Unsaved::NothingToLose); +} + +#[test] +fn a_branch_on_one_pair_holds_back_only_that_pair() { + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + git(&clone, &["branch", "keep", "feature~1"]); + let held_back = onelines(&clone, &["feature~1", "feature"]); + + let unsaved = held(&clone); + + assert_eq!(would_lose(&unsaved), "2 unpushed commit(s)"); + assert_eq!(unpushed_commits(&unsaved), held_back); +} + +#[test] +fn a_local_tag_on_the_reverted_commit_holds_both_back() { + // The tag holds the state with `probe.txt` in it. + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + git(&clone, &["tag", "probe", "feature~1"]); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_branch_on_the_reverted_commit_holds_both_back() { + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + git(&clone, &["branch", "keep", "feature~1"]); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_detached_head_on_the_reverted_commit_holds_both_back() { + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + let linked = fixture.path("detached"); + git( + &clone, + &[ + "worktree", + "add", + "-q", + "--detach", + linked.to_str().expect("utf-8"), + "feature~1", + ], + ); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_stash_made_on_the_reverted_commit_holds_both_back() { + // The stash is a merge whose first parent is the reverted commit, so the + // revert is not the only commit that grew from it. The two stash commits + // count, and so do the pair. + let fixture = Fixture::new(); + let clone = fixture.clone(); + write(&clone.join("probe.txt"), "probe\n"); + commit(&clone, "probe"); + write(&clone.join("stashed.txt"), "half a plan\n"); + git(&clone, &["add", "-A"]); + git_as_author(&clone, &["stash", "-q"]); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + + assert_eq!(would_lose(&held(&clone)), "4 unpushed commit(s)"); +} + +#[test] +fn a_branch_that_grew_from_the_reverted_commit_holds_both_back() { + // `other` builds on the state with `probe.txt` in it, and its tip is not + // the reverted commit, so only the second child shows it. + let fixture = Fixture::new(); + let clone = fixture.clone(); + write(&clone.join("probe.txt"), "probe\n"); + commit(&clone, "probe"); + git(&clone, &["checkout", "-q", "-b", "other"]); + write(&clone.join("more.txt"), "more\n"); + commit(&clone, "more"); + git(&clone, &["checkout", "-q", "feature"]); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + + assert_eq!(would_lose(&held(&clone)), "3 unpushed commit(s)"); +} + +#[test] +fn a_merge_that_takes_the_tree_back_stays_counted() { + // The merge has the tree under the probe, as a revert would, but it is a + // merge, and it drops `side.txt` as well. + let fixture = Fixture::new(); + let clone = fixture.clone(); + git(&clone, &["checkout", "-q", "-b", "side"]); + write(&clone.join("side.txt"), "side\n"); + commit(&clone, "side"); + git(&clone, &["checkout", "-q", "feature"]); + write(&clone.join("probe.txt"), "probe\n"); + commit(&clone, "probe"); + git_as_author( + &clone, + &[ + "merge", + "-q", + "--no-ff", + "-s", + "ours", + "--no-commit", + "side", + ], + ); + git(&clone, &["rm", "-q", "probe.txt"]); + commit(&clone, "merge side, dropping everything"); + assert_eq!( + git(&clone, &["rev-parse", "HEAD^{tree}"]), + git(&clone, &["rev-parse", "HEAD~2^{tree}"]) + ); + + assert_eq!(would_lose(&held(&clone)), "3 unpushed commit(s)"); +} + +#[test] +fn a_merge_with_an_edit_of_its_own_and_its_revert_stay_counted() { + // `git revert -m 1` takes the tree back to the merge's first parent, but + // the merge is not a single commit's change: it brought `side` in and made + // an edit of its own. + let fixture = Fixture::new(); + let clone = fixture.clone(); + git(&clone, &["checkout", "-q", "-b", "side"]); + write(&clone.join("side.txt"), "side\n"); + commit(&clone, "side"); + git(&clone, &["checkout", "-q", "feature"]); + git_as_author(&clone, &["merge", "-q", "--no-ff", "--no-commit", "side"]); + write(&clone.join("merge.txt"), "the merge's own\n"); + commit(&clone, "merge side, with an edit"); + git_as_author(&clone, &["revert", "--no-edit", "-m", "1", "HEAD"]); + + assert_eq!(would_lose(&held(&clone)), "3 unpushed commit(s)"); +} + +#[test] +fn the_revert_of_a_commit_a_remote_holds_a_copy_of_stays_counted() { + // The probe is on `origin/main` as a cherry-pick, so the copy rule clears + // it, and the revert is the only record of taking it out again. + let fixture = Fixture::new(); + let clone = fixture.clone(); + write(&clone.join("probe.txt"), "probe\n"); + commit(&clone, "probe"); + git(&clone, &["push", "-q", "origin", "feature"]); + let mate = teammate(&fixture); + git(&mate, &["checkout", "-q", "main"]); + git_as_author(&mate, &["cherry-pick", "origin/feature"]); + git(&mate, &["push", "-q", "origin", "main"]); + git(&mate, &["push", "-q", "origin", "--delete", "feature"]); + git(&clone, &["fetch", "-q", "--prune", "origin"]); + git_as_author(&clone, &["revert", "--no-edit", "HEAD"]); + assert_eq!(by_sha(&clone, "feature"), 3); + + assert_eq!(would_lose(&held(&clone)), "2 unpushed commit(s)"); +} + +#[test] +fn a_replace_ref_that_makes_a_cycle_does_not_hang_the_revert_rule() { + // git follows `refs/replace`, so a replacement of the probe whose parent is + // its revert makes `git log` print a cycle: each commit reverts the other. + // The walk down the pairs must stop, and a cycle is no pair. + let fixture = Fixture::new(); + let clone = fixture.clone(); + a_commit_and_its_revert(&clone); + let probe = git(&clone, &["rev-parse", "HEAD~1"]); + let revert = git(&clone, &["rev-parse", "HEAD"]); + let tree = git(&clone, &["rev-parse", "HEAD~1^{tree}"]); + let object = format!( + "tree {tree}\nparent {revert}\nauthor t 1 +0000\ncommitter t 1 +0000\n\nprobe\n" + ); + let path = fixture.path("replacement"); + write(&path, &object); + let replacement = git( + &clone, + &[ + "hash-object", + "-t", + "commit", + "-w", + path.to_str().expect("utf-8"), + ], + ); + git(&clone, &["replace", "-f", &probe, &replacement]); + + let (sender, receiver) = std::sync::mpsc::channel(); + let asked = clone.clone(); + std::thread::spawn(move || { + let _ = sender.send(held(&asked)); + }); + let answer = receiver + .recv_timeout(std::time::Duration::from_secs(30)) + .expect("the guard answers in 30 s"); + + assert!( + matches!(answer, Unsaved::WouldLose(_)), + "a cycle is no pair: {answer:?}" + ); +} + +/// A counted commit `c…` and its revert `a…` on top of the pushed `e…`, as +/// `log --boundary` lists them; `c…` changes `2…` to `1…` and `a…` puts `2…` +/// back. +const PAIR_GRAPH: &str = "> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ +2222222222222222222222222222222222222222 cccccccccccccccccccccccccccccccccccccccc\n\ +> cccccccccccccccccccccccccccccccccccccccc 1111111111111111111111111111111111111111 \ +eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\n\ +- eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee 2222222222222222222222222222222222222222 \n"; + +/// A clone at `/ws` whose unpushed graph is *graph* and whose ref tips are +/// *tips*. +fn scripted_pair(graph: Response, tips: Response) -> ScriptedRunner { + let at = |verb: &'static str| ["git", "--git-dir=/ws/.git", "--work-tree=/ws", verb]; + ScriptedRunner::new() + .with_script(at("log"), graph) + .with_script(at("rev-list"), tips) +} + +#[test] +fn a_commit_graph_git_will_not_list_clears_nothing() { + let counted = [ + "aaaaaaa Revert \"probe\"".to_owned(), + "ccccccc probe".to_owned(), + ]; + let tips = || Response::stdout("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"); + let refused = scripted_pair(Response::failed(128, "fatal: nope"), tips()); + + assert_eq!( + reverted_in_pairs(&Git::new(&refused), Path::new("/ws"), &counted), + Vec::::new() + ); + + // The control: the same answers with the graph read clear the pair. + let read = scripted_pair(Response::stdout(PAIR_GRAPH), tips()); + assert_eq!( + reverted_in_pairs(&Git::new(&read), Path::new("/ws"), &counted), + vec![ + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_owned(), + "cccccccccccccccccccccccccccccccccccccccc".to_owned(), + ] + ); +} + +#[test] +fn ref_tips_git_will_not_list_clear_nothing() { + // What was not read may be a ref on the reverted commit. + let counted = [ + "aaaaaaa Revert \"probe\"".to_owned(), + "ccccccc probe".to_owned(), + ]; + let refused = scripted_pair( + Response::stdout(PAIR_GRAPH), + Response::failed(128, "fatal: nope"), + ); + + assert_eq!( + reverted_in_pairs(&Git::new(&refused), Path::new("/ws"), &counted), + Vec::::new() + ); +} + +#[test] +fn a_commit_graph_line_in_another_shape_clears_nothing() { + // A line that could not be read may be a second commit on the reverted + // one, so the whole graph is unread. + let counted = [ + "aaaaaaa Revert \"probe\"".to_owned(), + "ccccccc probe".to_owned(), + ]; + let child = "dddddddddddddddddddddddddddddddddddddddd 3333333333333333333333333333333333333333 \ + cccccccccccccccccccccccccccccccccccccccc"; + for odd_line in [ + format!("? {child}"), + "> dddddddddddddddddddddddddddddddddddddddd".to_owned(), + ] { + let odd = scripted_pair( + Response::stdout(format!("{PAIR_GRAPH}{odd_line}\n")), + Response::stdout("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"), + ); + + assert_eq!( + reverted_in_pairs(&Git::new(&odd), Path::new("/ws"), &counted), + Vec::::new(), + "{odd_line}" + ); + } +} + +#[test] +fn a_revert_no_longer_counted_does_not_clear_the_commit_under_it() { + // Another rule cleared the revert, so the probe is counted alone, and + // nothing in the clone takes its change back out. + let counted = ["ccccccc probe".to_owned()]; + let read = scripted_pair( + Response::stdout(PAIR_GRAPH), + Response::stdout("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"), + ); + + assert_eq!( + reverted_in_pairs(&Git::new(&read), Path::new("/ws"), &counted), + Vec::::new() + ); +} + +#[test] +fn a_reverted_commit_no_longer_counted_does_not_pair() { + // Another rule cleared the probe, so only its revert is still counted, and + // the revert alone is the record of taking the probe out. + let counted = ["aaaaaaa Revert \"probe\"".to_owned()]; + let read = scripted_pair( + Response::stdout(PAIR_GRAPH), + Response::stdout("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"), + ); + + assert_eq!( + reverted_in_pairs(&Git::new(&read), Path::new("/ws"), &counted), + Vec::::new() + ); +} + #[test] fn a_merge_of_the_default_branch_adds_nothing_of_its_own() { let fixture = Fixture::new();