diff --git a/crates/but-workspace/tests/workspace/commit_engine/refs_update.rs b/crates/but-workspace/tests/workspace/commit_engine/refs_update.rs index b6993c6446..c2917ac338 100644 --- a/crates/but-workspace/tests/workspace/commit_engine/refs_update.rs +++ b/crates/but-workspace/tests/workspace/commit_engine/refs_update.rs @@ -144,6 +144,7 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> { ("s1-b/first", repo.rev_parse_single("@~1")?.detach()), ("s1-b/init", repo.rev_parse_single("@~2")?.detach()), ], + &repo, ); vb.branches.insert(stack.id, stack); @@ -155,6 +156,7 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> { ("s2-b/first", repo.rev_parse_single("@~1")?.detach()), ("s2-b/init", repo.rev_parse_single("@~2")?.detach()), ], + &repo, ); vb.branches.insert(stack.id, stack); @@ -274,7 +276,12 @@ fn insert_commit_into_single_stack_with_signatures() -> anyhow::Result<()> { * ecd6722 (tag: first-commit, first-commit) init "); - let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", initial_commit_id)]); + let stack = stack_with_branches( + "s1", + head_commit_id, + [("s1-b/init", initial_commit_id)], + &repo, + ); vb.branches.insert(stack.id, stack); // Add 10 lines to the end. write_sequence(&repo, "file", [(30, None)])?; @@ -430,7 +437,12 @@ fn branch_tip_below_non_merge_workspace_commit() -> anyhow::Result<()> { * 4342edf (tag: first-commit) init "); - let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", initial_commit_id)]); + let stack = stack_with_branches( + "s1", + head_commit_id, + [("s1-b/init", initial_commit_id)], + &repo, + ); vb.branches.insert(stack.id, stack); write_sequence(&repo, "file", [(110, None)])?; @@ -529,7 +541,7 @@ fn insert_commits_into_workspace() -> anyhow::Result<()> { let mut vb = VirtualBranchesState::default(); let stack1_head = repo.rev_parse_single("merge^1")?.detach(); - let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", stack1_head)]); + let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", stack1_head)], &repo); vb.branches.insert(stack.id, stack); // another 10 to the end (HEAD range is 1-30). @@ -657,7 +669,7 @@ fn merge_commit_remains_unsigned_in_remerge() -> anyhow::Result<()> { let branch_a = repo.rev_parse_single("A")?.detach(); let mut vb = VirtualBranchesState::default(); - let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)]); + let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)], &repo); vb.branches.insert(stack.id, stack); // initial is 1-30, remove first 5 @@ -745,6 +757,7 @@ fn commit_on_top_of_branch_in_workspace() -> anyhow::Result<()> { // The order indicates which one actually is on top, even though they both point to the // same commit. [("s1-b/below-top", branch_a), ("s1-b/top", branch_a)], + &repo, ); vb.branches.insert(stack.id, stack); @@ -826,7 +839,7 @@ fn amend_on_top_of_branch_in_workspace() -> anyhow::Result<()> { let branch_a = repo.rev_parse_single("A")?.detach(); let mut vb = VirtualBranchesState::default(); - let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)]); + let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)], &repo); vb.branches.insert(stack.id, stack); // initial is 1-30, make a change that transfers correctly to A where it is 5-20. @@ -910,6 +923,7 @@ mod utils { name: &str, tip: gix::ObjectId, branches: impl IntoIterator, + repo: &gix::Repository, ) -> gitbutler_stack::Stack { gitbutler_stack::Stack { id: Default::default(), @@ -918,7 +932,8 @@ mod utils { head: tip.to_git2(), heads: branches .into_iter() - .map(|(name, target_id)| new_stack_branch(name, target_id)) + .map(|(name, target_id)| new_stack_branch(name, target_id, repo)) + .filter_map(Result::ok) .collect(), notes: String::new(), source_refname: None, @@ -936,11 +951,16 @@ mod utils { } } - fn new_stack_branch(name: &str, head: gix::ObjectId) -> gitbutler_stack::StackBranch { + fn new_stack_branch( + name: &str, + head: gix::ObjectId, + repo: &gix::Repository, + ) -> anyhow::Result { gitbutler_stack::StackBranch::new( CommitOrChangeId::CommitId(head.to_string()), name.into(), None, + repo, ) } diff --git a/crates/gitbutler-branch-actions/src/stack.rs b/crates/gitbutler-branch-actions/src/stack.rs index 8920d93d19..8b4e90ffac 100644 --- a/crates/gitbutler-branch-actions/src/stack.rs +++ b/crates/gitbutler-branch-actions/src/stack.rs @@ -45,11 +45,12 @@ pub fn create_series( assure_open_workspace_mode(ctx).context("Requires an open workspace mode")?; let mut stack = ctx.project().virtual_branches().get_stack(stack_id)?; let normalized_head_name = normalize_branch_name(&req.name)?; + let repo = ctx.gix_repository()?; // If target_patch is None, create a new head that points to the top of the stack (most recent patch) if let Some(target_patch) = req.target_patch { stack.add_series( ctx, - StackBranch::new(target_patch, normalized_head_name, req.description), + StackBranch::new(target_patch, normalized_head_name, req.description, &repo)?, req.preceding_head, ) } else { diff --git a/crates/gitbutler-stack/src/heads.rs b/crates/gitbutler-stack/src/heads.rs index 26f9a71f2c..ac825b517c 100644 --- a/crates/gitbutler-stack/src/heads.rs +++ b/crates/gitbutler-stack/src/heads.rs @@ -46,7 +46,7 @@ pub(crate) fn remove_head( /// Returns new, updated list of heads with the new head added in the correct position. /// If there are multiple heads pointing to the same patch, it uses `preceding_head` to disambiguate the order. // TODO: when there is a patch reference for a commit ID and a patch reference for a change ID, recognize if they are equivalent (i.e. point to the same commit) -pub(crate) fn add_head( +pub fn add_head( existing_heads: Vec, new_head: StackBranch, preceding_head: Option, @@ -138,41 +138,3 @@ pub(crate) fn add_head( } Ok(updated_heads) } - -#[cfg(test)] -mod test { - use super::*; - #[test] - fn add_head_with_archived_bottom_head() -> Result<()> { - let mut head_1_archived = StackBranch::new( - CommitOrChangeId::CommitId("328447a2-08aa-4c4d-a1bc-08d5cd82bcd4".to_string()), - "kv-branch-3".to_string(), - None, - ); - head_1_archived.archived = true; - let head_2 = StackBranch::new( - CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()), - "more-on-top".to_string(), - None, - ); - let existing_heads = vec![head_1_archived.clone(), head_2.clone()]; - let new_head = StackBranch::new( - CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()), - "abcd".to_string(), - None, - ); - let patches = vec![ - CommitOrChangeId::CommitId("92a89ae608d77ff75c1ce52ea9dccc0bccd577e9".to_string()), - CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()), - ]; - - let updated_heads = add_head( - existing_heads, - new_head.clone(), - Some(head_2.clone()), - patches, - )?; - assert_eq!(updated_heads, vec![head_1_archived, head_2, new_head]); - Ok(()) - } -} diff --git a/crates/gitbutler-stack/src/lib.rs b/crates/gitbutler-stack/src/lib.rs index 07f3d5c304..4dcffaa720 100644 --- a/crates/gitbutler-stack/src/lib.rs +++ b/crates/gitbutler-stack/src/lib.rs @@ -13,6 +13,7 @@ pub use state::{VirtualBranches as VirtualBranchesState, VirtualBranchesHandle}; pub use target::Target; mod heads; +pub use heads::add_head; pub use stack::{commit_by_oid_or_change_id, CommitsForId, PatchReferenceUpdate, TargetUpdate}; // This is here because CommitOrChangeId::ChangeId is deprecated, for some reason allow cant be done on the CommitOrChangeId struct diff --git a/crates/gitbutler-stack/src/stack.rs b/crates/gitbutler-stack/src/stack.rs index f8192185c9..924c92c47e 100644 --- a/crates/gitbutler-stack/src/stack.rs +++ b/crates/gitbutler-stack/src/stack.rs @@ -306,9 +306,10 @@ impl Stack { ) -> Result { let commit = ctx.repo().find_commit(self.head())?; let state = branch_state(ctx); + let repo = ctx.gix_repository()?; let name = Stack::next_available_name( - ctx, + &repo, &state, if let Some(refname) = self.upstream.as_ref() { refname.branch().to_string() @@ -319,14 +320,14 @@ impl Stack { allow_duplicate_refs, )?; - let reference = StackBranch::new(commit.into(), name, None); + let reference = StackBranch::new(commit.into(), name, None, &repo)?; validate_name(&reference, &state)?; Ok(reference) } fn next_available_name( - ctx: &CommandContext, + repo: &gix::Repository, state: &VirtualBranchesHandle, mut name: String, allow_duplicate_refs: bool, @@ -335,10 +336,9 @@ impl Stack { Ok(if allow_duplicate_refs { patch_reference_exists(state, name)? } else { - let repository = ctx.gix_repository()?; patch_reference_exists(state, name)? - || local_reference_exists(&repository, name)? - || remote_reference_exists(&repository, state, name)? + || local_reference_exists(repo, name)? + || remote_reference_exists(repo, state, name)? }) }; while is_duplicate(&name)? { @@ -402,7 +402,9 @@ impl Stack { let current_top_head = self.heads.last().ok_or(anyhow!( "Stack is in an invalid state - heads list is empty" ))?; - let new_head = StackBranch::new(current_top_head.head().to_owned(), name, description); + let repo = ctx.gix_repository()?; + let new_head = + StackBranch::new(current_top_head.head().to_owned(), name, description, &repo)?; self.add_series(ctx, new_head, Some(current_top_head.name().clone())) } diff --git a/crates/gitbutler-stack/src/stack_branch.rs b/crates/gitbutler-stack/src/stack_branch.rs index 5aec77cbfd..64a286f659 100644 --- a/crates/gitbutler-stack/src/stack_branch.rs +++ b/crates/gitbutler-stack/src/stack_branch.rs @@ -79,15 +79,22 @@ impl RepositoryExt for git2::Repository { } impl StackBranch { - pub fn new(head: CommitOrChangeId, name: String, description: Option) -> Self { - StackBranch { + pub fn new( + head: CommitOrChangeId, + name: String, + description: Option, + repo: &gix::Repository, + ) -> Result { + let branch = StackBranch { head, name, description, pr_number: None, archived: false, review_id: None, - } + }; + let _ = branch.set_real_reference(repo, &branch.head); + Ok(branch) } pub fn head(&self) -> &CommitOrChangeId { diff --git a/crates/gitbutler-stack/tests/mod.rs b/crates/gitbutler-stack/tests/mod.rs index 0321e3daf8..8eab16562e 100644 --- a/crates/gitbutler-stack/tests/mod.rs +++ b/crates/gitbutler-stack/tests/mod.rs @@ -19,7 +19,8 @@ fn add_series_success() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "asdf".into(), Some("my description".into()), - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference, None); assert!(result.is_ok()); assert_eq!(test_ctx.stack.heads.len(), 2); @@ -71,7 +72,8 @@ fn add_series_top_base() -> Result<()> { CommitOrChangeId::CommitId(merge_base.id().to_string()), "asdf".into(), Some("my description".into()), - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference, None); println!("{:?}", result); // Assert persisted @@ -95,7 +97,8 @@ fn add_multiple_series() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.last().unwrap().id().to_string()), "head_4".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx .stack .add_series(&ctx, head_4, Some(default_head.name().clone())); @@ -106,7 +109,8 @@ fn add_multiple_series() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.last().unwrap().id().to_string()), "head_2".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, head_2, None); assert!(result.is_ok()); assert_eq!( @@ -118,7 +122,8 @@ fn add_multiple_series() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.first().unwrap().id().to_string()), "head_1".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, head_1, None); assert!(result.is_ok()); @@ -142,7 +147,8 @@ fn add_series_invalid_name_fails() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()), "name with spaces".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference, None); assert_eq!(result.err().unwrap().to_string(), "Invalid branch name"); Ok(()) @@ -156,7 +162,8 @@ fn add_series_duplicate_name_fails() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "asdf".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert!(result.is_ok()); let result = test_ctx.stack.add_series(&ctx, reference, None); @@ -175,7 +182,8 @@ fn add_series_matching_git_ref_is_ok() -> Result<()> { test_ctx.commits[0].clone().into(), "existing-branch".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert!(result.is_ok()); // allow this Ok(()) @@ -189,7 +197,8 @@ fn add_series_including_refs_head_fails() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()), "refs/heads/my-branch".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert_eq!( result.err().unwrap().to_string(), @@ -206,7 +215,8 @@ fn add_series_target_commit_doesnt_exist() -> Result<()> { CommitOrChangeId::CommitId("30696678319e0fa3a20e54f22d47fc8cf1ceaade".into()), // does not exist "my-branch".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert!(result .err() @@ -224,7 +234,8 @@ fn add_series_target_change_id_doesnt_exist() -> Result<()> { CommitOrChangeId::CommitId("10696678319e0fa3a20e54f22d47fc8cf1ceaade".into()), // does not exist "my-branch".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert_eq!( result.err().unwrap().to_string(), @@ -242,7 +253,8 @@ fn add_series_target_commit_not_in_stack() -> Result<()> { CommitOrChangeId::CommitId(other_commit_id.clone()), // does not exist "my-branch".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert_eq!( result.err().unwrap().to_string(), @@ -295,7 +307,8 @@ fn remove_series_with_multiple_last_heads() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.last().unwrap().id().to_string()), "to_stay".into(), None, - ); + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, to_stay.clone(), None); assert!(result.is_ok()); assert_eq!(head_names(&test_ctx), vec!["to_stay", "a-branch-2"]); @@ -325,7 +338,8 @@ fn remove_series_no_orphan_commits() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.first().unwrap().id().to_string()), "to_stay".into(), None, - ); // references the oldest commit + &ctx.gix_repository()?, + )?; // references the oldest commit let result = test_ctx.stack.add_series(&ctx, to_stay.clone(), None); assert!(result.is_ok()); assert_eq!(head_names(&test_ctx), vec!["to_stay", "a-branch-2"]); @@ -504,7 +518,12 @@ fn update_series_target_success() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; let commit_0_change_id = CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()); - let series_1 = StackBranch::new(commit_0_change_id.clone(), "series_1".into(), None); + let series_1 = StackBranch::new( + commit_0_change_id.clone(), + "series_1".into(), + None, + &ctx.gix_repository()?, + )?; let result = test_ctx.stack.add_series(&ctx, series_1, None); assert!(result.is_ok()); assert_eq!(*test_ctx.stack.heads[0].head(), commit_0_change_id); @@ -604,7 +623,8 @@ fn list_series_two_heads_same_commit() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.last().unwrap().id().to_string()), "head_before".into(), None, - ); + &ctx.gix_repository()?, + )?; // add `head_before` before the initial head let result = test_ctx.stack.add_series(&ctx, head_before, None); assert!(result.is_ok()); @@ -647,7 +667,8 @@ fn list_series_two_heads_different_commit() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits.first().unwrap().id().to_string()), "head_before".into(), None, - ); + &ctx.gix_repository()?, + )?; let stack_context = ctx.to_stack_context()?; @@ -720,7 +741,8 @@ fn replace_head_single() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "from_head".into(), None, - ); + &ctx.gix_repository()?, + )?; test_ctx.stack.add_series(&ctx, from_head, None)?; // replace with previous head let result = test_ctx @@ -751,7 +773,8 @@ fn replace_head_single_with_merge_base() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "from_head".into(), None, - ); + &ctx.gix_repository()?, + )?; test_ctx.stack.add_series(&ctx, from_head, None)?; // replace with merge base let merge_base = ctx.repo().find_commit( @@ -786,7 +809,8 @@ fn replace_head_with_invalid_commit_error() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "from_head".into(), None, - ); + &ctx.gix_repository()?, + )?; test_ctx.stack.add_series(&ctx, from_head, None)?; let stack = test_ctx.stack.clone(); let result = @@ -812,7 +836,8 @@ fn replace_head_with_same_noop() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "from_head".into(), None, - ); + &ctx.gix_repository()?, + )?; test_ctx.stack.add_series(&ctx, from_head, None)?; let stack = test_ctx.stack.clone(); let result = test_ctx @@ -897,12 +922,14 @@ fn replace_head_multiple() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "from_head_1".into(), None, - ); + &ctx.gix_repository()?, + )?; let from_head_2 = StackBranch::new( CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "from_head_2".into(), None, - ); + &ctx.gix_repository()?, + )?; // both references point to the same commit test_ctx.stack.add_series(&ctx, from_head_1, None)?; test_ctx @@ -944,7 +971,8 @@ fn replace_head_top_of_stack_multiple() -> Result<()> { CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()), "extra_head".into(), None, - ); + &ctx.gix_repository()?, + )?; // an extra head just beneath the top of the stack test_ctx.stack.add_series(&ctx, extra_head, None)?; // replace top of stack the commit @@ -998,7 +1026,8 @@ fn archive_heads_success() -> Result<()> { test_ctx.other_commits.first().cloned().unwrap().into(), "foo".to_string(), None, - ), + &ctx.gix_repository()?, + )?, ); assert_eq!(test_ctx.stack.heads.len(), 2); test_ctx.stack.archive_integrated_heads(&ctx)?; @@ -1074,6 +1103,45 @@ fn set_pr_numberentifiers_series_not_found_fails() -> Result<()> { ); Ok(()) } + +#[test] +fn add_head_with_archived_bottom_head() -> Result<()> { + let (ctx, _temp_dir) = command_ctx("multiple-commits")?; + let mut head_1_archived = StackBranch::new( + CommitOrChangeId::CommitId("328447a2-08aa-4c4d-a1bc-08d5cd82bcd4".to_string()), + "kv-branch-3".to_string(), + None, + &ctx.gix_repository()?, + )?; + head_1_archived.archived = true; + let head_2 = StackBranch::new( + CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()), + "more-on-top".to_string(), + None, + &ctx.gix_repository()?, + )?; + let existing_heads = vec![head_1_archived.clone(), head_2.clone()]; + let new_head = StackBranch::new( + CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()), + "abcd".to_string(), + None, + &ctx.gix_repository()?, + )?; + let patches = vec![ + CommitOrChangeId::CommitId("92a89ae608d77ff75c1ce52ea9dccc0bccd577e9".to_string()), + CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()), + ]; + + let updated_heads = gitbutler_stack::add_head( + existing_heads, + new_head.clone(), + Some(head_2.clone()), + patches, + )?; + assert_eq!(updated_heads, vec![head_1_archived, head_2, new_head]); + Ok(()) +} + fn command_ctx(name: &str) -> Result<(CommandContext, TempDir)> { gitbutler_testsupport::writable::fixture("stacking.sh", name) }