Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion crates/but-core/src/worktree/checkout/function.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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)?;
Comment thread
krlvi marked this conversation as resolved.
// 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())?;
Expand Down Expand Up @@ -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 <path>`.",
paths.join(", ")
);
}
116 changes: 116 additions & 0 deletions crates/but-core/tests/core/worktree/checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path>`.",
);

// 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");
Expand Down Expand Up @@ -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 {}
Loading