diff --git a/crates/but-core/src/worktree/checkout/function.rs b/crates/but-core/src/worktree/checkout/function.rs index c63f263d2c9..afc8c40570f 100644 --- a/crates/but-core/src/worktree/checkout/function.rs +++ b/crates/but-core/src/worktree/checkout/function.rs @@ -1,5 +1,5 @@ use anyhow::bail; -use bstr::BStr; +use bstr::{BStr, ByteSlice as _}; use but_error::bail_precondition; use but_oxidize::{ObjectIdExt, OidExt as _}; use gix::{ @@ -73,6 +73,7 @@ pub fn safe_checkout_from_head( .peel_to_tree()?; let mut conflict_occurred = false; if old_tree.id() != new_tree.id() { + ensure_index_has_no_conflicts(&git2_repo)?; // Reopen to ensure that there is no "object memory" (i.e. all object // writes actually happen on disk). let mut repo = gix::open(repo.git_dir())?; @@ -192,3 +193,34 @@ pub fn safe_checkout_from_head( conflict_occurred, }) } + +/// Refuse to check out while `.git/index` still holds unresolved stage 1/2/3 entries. +/// +/// Such entries are left behind by a checkout that was allowed to conflict with +/// uncommitted changes, or by a merge run outside GitButler. libgit2 would reject the +/// checkout anyway with a raw `Conflict` error; failing here keeps worktree, index and +/// `HEAD` untouched and classifies the error as a precondition the user can act on. +/// +/// This runs after the merge-base override's index rewrite on purpose: when that +/// rewrite falls back to overwriting the index with the override tree, the stages are +/// gone and the checkout may proceed — the intended steamroll semantic for restoring a +/// snapshot or committing the conflicted file itself. +fn ensure_index_has_no_conflicts(git2_repo: &git2::Repository) -> anyhow::Result<()> { + let index = git2_repo.index()?; + if !index.has_conflicts() { + return Ok(()); + } + let mut paths = Vec::new(); + for conflict in index.conflicts()? { + let conflict = conflict?; + if let Some(entry) = conflict.our.or(conflict.their).or(conflict.ancestor) { + paths.push(format!("{:?}", entry.path.as_slice().as_bstr())); + } + } + paths.sort(); + paths.dedup(); + bail_precondition!( + "Cannot update the worktree while files have unresolved conflicts: {}. Resolve the conflicts, then mark each file as resolved, e.g. with `but resolve `.", + paths.join(", ") + ); +} diff --git a/crates/but-core/tests/core/worktree/checkout.rs b/crates/but-core/tests/core/worktree/checkout.rs index b20233e5592..5f00f2d6ffd 100644 --- a/crates/but-core/tests/core/worktree/checkout.rs +++ b/crates/but-core/tests/core/worktree/checkout.rs @@ -143,6 +143,59 @@ fn conflicted_commits_cannot_be_checked_out() -> anyhow::Result<()> { Ok(()) } +#[test] +fn unresolved_index_conflicts_refuse_checkout_before_mutation() -> anyhow::Result<()> { + let (repo, _tmp) = writable_scenario("merge-with-two-branches-conflict"); + let index_before = visualize_index(&*repo.index()?); + snapbox::assert_data_eq!( + index_before.as_str(), + snapbox::str![[r#" +100644:e69de29 file:1 +100644:e6c4914 file:2 +100644:e33f5e9 file:3 + +"#]] + ); + let status_before = git_status(&repo)?; + snapbox::assert_data_eq!( + status_before.as_str(), + snapbox::str![[r#" +UU file + +"#]] + ); + + // A no-op checkout of the current head doesn't touch index or worktree, + // so it may proceed - materializations that keep the head tree (e.g. a reword) + // must still work while conflicts are unresolved. + let head_commit = repo.head_commit()?; + safe_checkout_from_head(head_commit.id, &repo, Default::default()) + .expect("no-op checkouts succeed despite index conflicts"); + + let target = repo.rev_parse_single("A")?.detach(); + let err = safe_checkout_from_head(target, &repo, Default::default()) + .expect_err("a real checkout must refuse while the index has unresolved conflicts"); + assert_eq!( + err.to_string(), + "Cannot update the worktree while files have unresolved conflicts: \"file\". Resolve the conflicts, then mark each file as resolved, e.g. with `but resolve `.", + ); + + // Nothing changed: worktree, index and HEAD are preserved. + assert_eq!(visualize_index(&*repo.index()?), index_before); + assert_eq!(git_status(&repo)?, status_before); + snapbox::assert_data_eq!( + visualize_commit_graph_all(&repo)?, + snapbox::str![[r#" +* 88d7acc (A) 10 to 20 +| * 47334c6 (HEAD -> merge, B) 20 to 30 +|/ +* 15bcd1b (main) init + +"#]] + ); + Ok(()) +} + #[test] fn pure_deletion_checkout_does_not_restore_unrelated_worktree_deletions() -> anyhow::Result<()> { let (repo, _tmp) = writable_scenario_slow("all-file-types-renamed-and-modified"); @@ -1154,4 +1207,67 @@ fn cancelling_consumed_changes_keeps_a_concurrent_edit() -> anyhow::Result<()> { Ok(()) } +/// The merge-base override path must keep working while the index has unresolved +/// conflicts: snapshot restore (undo) always passes an override and is the escape +/// hatch from exactly this state. The override's index rewrite falls back to +/// overwriting the index with the override tree (patching a conflicted index fails), +/// which clears the stages before the conflict guard runs, and the checkout then +/// persists the conflict-free index. +#[test] +fn merge_base_override_steamrolls_stale_index_conflicts() -> anyhow::Result<()> { + let (repo, _tmp) = writable_scenario("merge-with-two-branches-conflict"); + assert!( + git2::Repository::open(repo.git_dir())? + .index()? + .has_conflicts(), + "the fixture starts out mid-merge with unresolved index stages" + ); + + // Restore-shaped call: the override is the current workdir tree (like the + // pre-restore snapshot's), so uncommitted content cancels out in the merge and + // the destination tree is checked out as-is. + let wd_blob = repo.write_blob(std::fs::read(repo.workdir_path("file").expect("non-bare"))?)?; + let mut editor = repo.empty_tree().edit()?; + editor.upsert("file", EntryKind::Blob, wd_blob.detach())?; + let wd_tree = editor.write()?.detach(); + + let target = repo.rev_parse_single("A")?.detach(); + safe_checkout_from_head( + target, + &repo, + checkout::Options { + merge_base_override: Some(wd_tree), + ..Default::default() + }, + ) + .expect("an override checkout succeeds despite index conflicts"); + + // The conflict stages are gone from the on-disk index and everything matches `A`. + assert!( + !git2::Repository::open(repo.git_dir())? + .index()? + .has_conflicts(), + "the override checkout persistently cleared the conflict stages" + ); + snapbox::assert_data_eq!( + visualize_index(&*repo.index()?), + snapbox::str![[r#" +100644:e33f5e9 file + +"#]] + ); + snapbox::assert_data_eq!(git_status(&repo)?, snapbox::str![""]); + snapbox::assert_data_eq!( + visualize_commit_graph_all(&repo)?, + snapbox::str![[r#" +* 88d7acc (HEAD -> merge, A) 10 to 20 +| * 47334c6 (B) 20 to 30 +|/ +* 15bcd1b (main) init + +"#]] + ); + Ok(()) +} + mod utils {}