Skip to content
Merged
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
31 changes: 31 additions & 0 deletions specs/005-branch.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ git-loom branch merge [branch] [--all]
- Uses `git merge --no-ff` to create the merge topology
- If a remote branch is selected (with `--all`), a local tracking branch is
created automatically before weaving
- On merge conflict, saves state and pauses for `loom continue` / `loom abort`
- Errors if the branch is already woven or doesn't exist

### `branch unmerge`
Expand Down Expand Up @@ -179,6 +180,36 @@ a new feature from the integration point.

File targets are rejected with an error message.

## Conflict Recovery

### `branch new` (weaving)

When `branch new` triggers a weave operation that encounters conflicts, it uses
hard-fail: the rebase is automatically aborted and an error is reported. No
state is saved — the user must resolve the situation and retry.

### `branch merge`

`branch merge` supports resumable conflict handling via `loom continue` and
`loom abort` (see Spec 014).

When the merge encounters a conflict:

1. The state is saved to `.git/loom/state.json`
2. loom reports that the operation is paused and exits

The saved state contains:
- `branch_name`: the branch being woven into integration

After the user resolves the conflict:

- `loom continue` — completes the merge and prints the success message
- `loom abort` — calls `git merge --abort` and restores the original state

While the operation is paused, most other loom commands are blocked. Only
`loom show`, `loom diff`, `loom trace`, `loom continue`, and `loom abort`
are permitted.

## Prerequisites

- Git 2.38 or later
Expand Down
46 changes: 39 additions & 7 deletions src/branch/merge.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
use anyhow::{Result, bail};
use anyhow::{Context, Result, bail};
use git2::{BranchType, Repository};
use serde::{Deserialize, Serialize};

use crate::git;
use crate::git_commands::git_branch;
use crate::git_commands::git_merge::MergeOutcome;
use crate::msg;
use crate::transaction::{self, LoomState, Rollback};

#[derive(Serialize, Deserialize)]
struct MergeContext {
branch_name: String,
}

/// Weave an existing branch into the integration branch.
///
Expand All @@ -13,6 +21,7 @@ use crate::msg;
pub fn run(branch: Option<String>, all: bool) -> Result<()> {
let repo = git::open_repo()?;
let workdir = git::require_workdir(&repo, "merge")?;
let git_dir = repo.path().to_path_buf();
let info = git::gather_repo_info(&repo, false, 1)?;

let branch_name = match branch {
Expand All @@ -22,7 +31,7 @@ pub fn run(branch: Option<String>, all: bool) -> Result<()> {

// If this is a remote branch (contains '/'), create a local tracking branch
let local_name = if branch_name.contains('/') {
let local = branch_name.split('/').skip(1).collect::<Vec<_>>().join("/");
let local = git::upstream_local_branch(&branch_name);
git_branch::create(workdir, &local, &branch_name)?;
// Set up tracking
let mut local_branch = repo.find_branch(&local, BranchType::Local)?;
Expand All @@ -33,10 +42,34 @@ pub fn run(branch: Option<String>, all: bool) -> Result<()> {
};

// Merge the branch into integration (--no-ff) so it appears in the topology
crate::git_commands::git_merge::merge_no_ff(workdir, &local_name)?;
match crate::git_commands::git_merge::merge_no_ff(workdir, &git_dir, &local_name)? {
MergeOutcome::Completed => {
msg::success(&format!("Woven `{}` into integration branch", local_name));
}
MergeOutcome::Conflicted => {
let state = LoomState {
command: "merge".to_string(),
rollback: Rollback::default(),
context: serde_json::to_value(MergeContext {
branch_name: local_name,
})?,
};
transaction::save(&git_dir, &state)?;
transaction::warn_conflict_paused("merge");
}
}

msg::success(&format!("Woven `{}` into integration branch", local_name));
Ok(())
}

/// Resume a `loom branch merge` after conflicts have been resolved.
pub fn after_continue(context: &serde_json::Value) -> anyhow::Result<()> {
let ctx: MergeContext =
serde_json::from_value(context.clone()).context("Failed to parse merge resume context")?;
msg::success(&format!(
"Woven `{}` into integration branch",
ctx.branch_name
));
Ok(())
}

Expand Down Expand Up @@ -87,7 +120,7 @@ fn pick_branch(repo: &Repository, info: &git::RepoInfo, include_remote: bool) ->

// Remote branches without a local counterpart
if include_remote {
let local_names: Vec<String> = repo
let local_names: std::collections::HashSet<String> = repo
.branches(Some(BranchType::Local))?
.filter_map(|b| b.ok())
.filter_map(|(b, _)| b.name().ok().flatten().map(|n| n.to_string()))
Expand All @@ -107,8 +140,7 @@ fn pick_branch(repo: &Repository, info: &git::RepoInfo, include_remote: bool) ->
continue;
}
// Skip if a local branch with the same short name exists
let short_name = name.split('/').skip(1).collect::<Vec<_>>().join("/");
if !local_names.contains(&short_name) {
if !local_names.contains(&git::upstream_local_branch(name)) {
items.push(name.to_string());
}
}
Expand Down
126 changes: 126 additions & 0 deletions src/branch/merge_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,129 @@ fn merge_nonexistent_branch_errors() {
test_repo.in_dir(|| super::merge::run(Some("nonexistent-branch".to_string()), false));
assert!(result.is_err(), "merging nonexistent branch should error");
}

/// When a merge conflicts, the operation pauses: state file saved, HEAD unchanged.
/// Resolving and running continue completes the merge and weaves the branch.
#[test]
fn merge_conflict_continue_weaves_branch() {
let test_repo = TestRepo::new_with_remote();

// Both branches modify the same file differently → merge conflict.
let base_oid = test_repo.find_remote_branch_target("origin/main");
test_repo.create_branch_at("feature-conflict", &base_oid.to_string());

test_repo.write_file("shared.txt", "integration");
test_repo.stage_files(&["shared.txt"]);
test_repo.commit_staged("Integration change");

test_repo.switch_branch("feature-conflict");
test_repo.write_file("shared.txt", "feature");
test_repo.stage_files(&["shared.txt"]);
test_repo.commit_staged("Feature change");
test_repo.switch_branch("integration");

let pre_merge_head = test_repo.head_oid();

// Merge pauses — must not return Err.
let result =
test_repo.in_dir(|| super::merge::run(Some("feature-conflict".to_string()), false));
assert!(
result.is_ok(),
"merge should return Ok on conflict: {:?}",
result
);

let state_path = test_repo.repo.path().join("loom/state.json");
assert!(
state_path.exists(),
"state file must exist when merge is paused"
);
assert_eq!(
test_repo.head_oid(),
pre_merge_head,
"HEAD must not move on conflict"
);

// Resolve conflict and continue.
test_repo.write_file("shared.txt", "resolved");
test_repo.stage_files(&["shared.txt"]);
crate::transaction::continue_cmd(&test_repo.workdir(), test_repo.repo.path()).unwrap();

assert!(
!state_path.exists(),
"state file must be deleted after continue"
);
assert_eq!(
test_repo.head_commit().parent_count(),
2,
"HEAD must be a merge commit after continue"
);

let info = git::gather_repo_info(&test_repo.repo, false, 1).unwrap();
let branch_names: Vec<&str> = info.branches.iter().map(|b| b.name.as_str()).collect();
assert!(
branch_names.contains(&"feature-conflict"),
"branch must be woven after continue, got: {:?}",
branch_names
);
}

/// Aborting a paused merge restores HEAD to its pre-merge state and leaves the
/// branch unwoven.
#[test]
fn merge_conflict_abort_restores_state() {
let test_repo = TestRepo::new_with_remote();

let base_oid = test_repo.find_remote_branch_target("origin/main");
test_repo.create_branch_at("feature-abort", &base_oid.to_string());

test_repo.write_file("shared.txt", "integration");
test_repo.stage_files(&["shared.txt"]);
test_repo.commit_staged("Integration change");

test_repo.switch_branch("feature-abort");
test_repo.write_file("shared.txt", "feature");
test_repo.stage_files(&["shared.txt"]);
test_repo.commit_staged("Feature change");
test_repo.switch_branch("integration");

let pre_merge_head = test_repo.head_oid();

let result = test_repo.in_dir(|| super::merge::run(Some("feature-abort".to_string()), false));
assert!(
result.is_ok(),
"merge should return Ok on conflict: {:?}",
result
);

let state_path = test_repo.repo.path().join("loom/state.json");
assert!(
state_path.exists(),
"state file must exist when merge is paused"
);

crate::transaction::abort_cmd(&test_repo.workdir(), test_repo.repo.path()).unwrap();

assert!(
!state_path.exists(),
"state file must be deleted after abort"
);
assert_eq!(
test_repo.head_oid(),
pre_merge_head,
"HEAD must be restored after abort"
);
assert_eq!(
test_repo.head_commit().parent_count(),
1,
"HEAD must not be a merge commit after abort"
);

let info = git::gather_repo_info(&test_repo.repo, false, 1).unwrap();
let branch_names: Vec<&str> = info.branches.iter().map(|b| b.name.as_str()).collect();
assert!(
!branch_names.contains(&"feature-abort"),
"branch must NOT be woven after abort, got: {:?}",
branch_names
);
}
50 changes: 47 additions & 3 deletions src/git_commands/git_merge.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,51 @@
use std::path::Path;

use anyhow::Result;

/// Outcome of a merge operation.
pub enum MergeOutcome {
Completed,
Conflicted,
}

/// Merge a branch into the current branch, forcing a merge commit.
///
/// Wraps `git merge --no-ff <branch> --no-edit`.
/// Use this when a fast-forward merge would skip creating the merge topology.
pub fn merge_no_ff(workdir: &std::path::Path, branch: &str) -> anyhow::Result<()> {
super::run_git(workdir, &["merge", "--no-ff", branch, "--no-edit"])
/// Returns `Conflicted` if the merge stopped due to conflicts,
/// `Completed` on success, or `Err` on any other failure.
pub fn merge_no_ff(workdir: &Path, git_dir: &Path, branch: &str) -> Result<MergeOutcome> {
run_merge_cmd(workdir, git_dir, &["merge", "--no-ff", branch, "--no-edit"])
}

/// Continue an in-progress merge (equivalent to `git merge --continue`).
///
/// Note: `--continue` does not accept extra flags like `--no-edit`. The merge
/// commit message is taken from `MERGE_MSG` without opening an editor.
pub fn continue_merge(workdir: &Path, git_dir: &Path) -> Result<MergeOutcome> {
run_merge_cmd(workdir, git_dir, &["merge", "--continue"])
}

fn run_merge_cmd(workdir: &Path, git_dir: &Path, args: &[&str]) -> Result<MergeOutcome> {
match super::run_git(workdir, args) {
Ok(()) => Ok(MergeOutcome::Completed),
Err(e) => {
if is_in_progress(git_dir) {
Ok(MergeOutcome::Conflicted)
} else {
Err(e)
}
}
}
}

/// Abort an in-progress merge.
pub fn abort(workdir: &Path) -> Result<()> {
super::run_git(workdir, &["merge", "--abort"])
}

/// Check whether a merge is currently in progress.
///
/// Detects the presence of `MERGE_HEAD`, which git creates when a merge is paused.
pub fn is_in_progress(git_dir: &Path) -> bool {
git_dir.join("MERGE_HEAD").exists()
}
17 changes: 16 additions & 1 deletion src/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ impl TestRepo {
let mut config = repo.config().unwrap();
config.set_str("user.name", "Test").unwrap();
config.set_str("user.email", "test@test.com").unwrap();
// Prevent git from opening an interactive editor in tests (e.g. for
// `git merge --continue` which is equivalent to `git commit`).
config.set_str("core.editor", "true").unwrap();
}

/// Get the signature used for commits.
Expand Down Expand Up @@ -610,7 +613,19 @@ impl TestRepo {

/// Merge a branch into the current branch with --no-ff.
pub fn merge_no_ff(&self, branch: &str) {
crate::git_commands::git_merge::merge_no_ff(self.workdir().as_path(), branch).unwrap();
let outcome = crate::git_commands::git_merge::merge_no_ff(
self.workdir().as_path(),
self.repo.path(),
branch,
)
.unwrap();
assert!(
matches!(
outcome,
crate::git_commands::git_merge::MergeOutcome::Completed
),
"merge_no_ff: expected Completed, got Conflicted"
);
}

/// Stage files in the working directory.
Expand Down
13 changes: 12 additions & 1 deletion src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use anyhow::{Context, Result, bail};
use serde::{Deserialize, Serialize};

use crate::git_commands::{self, git_branch, git_commit, git_rebase};
use crate::git_commands::{self, git_branch, git_commit, git_merge, git_rebase};

/// Persistent state saved when a loom command is paused due to a rebase conflict.
#[derive(Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -151,6 +151,14 @@ pub fn continue_cmd(workdir: &Path, git_dir: &Path) -> Result<()> {
}
git_rebase::RebaseOutcome::Completed => {}
}
} else if git_merge::is_in_progress(git_dir) {
match git_merge::continue_merge(workdir, git_dir)? {
git_merge::MergeOutcome::Conflicted => {
crate::msg::warn("Conflicts remain — resolve them and run `loom continue` again");
return Ok(());
}
git_merge::MergeOutcome::Completed => {}
}
}

dispatch_after_continue(workdir, &state)?;
Expand All @@ -171,6 +179,8 @@ pub fn abort_cmd(workdir: &Path, git_dir: &Path) -> Result<()> {

if git_rebase::is_in_progress(git_dir) {
let _ = git_rebase::abort(workdir);
} else if git_merge::is_in_progress(git_dir) {
let _ = git_merge::abort(workdir);
}

state.rollback.apply_abort(workdir)?;
Expand All @@ -192,6 +202,7 @@ fn dispatch_after_continue(workdir: &Path, state: &LoomState) -> Result<()> {
"drop" => crate::drop::after_continue(workdir, &state.context),
"fold" => crate::fold::after_continue(workdir, &state.context),
"swap" => crate::swap::after_continue(workdir, &state.context),
"merge" => crate::branch::merge::after_continue(&state.context),
other => bail!("Unknown command '{}' in loom state file", other),
}
}
Expand Down
Loading
Loading