From cb7c4ea4863b9cb3820c7cce688c6bf248e5fc77 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Tue, 22 Oct 2024 13:29:37 +0200 Subject: [PATCH] refactor stack api Make it not depend on RepoActionsExt --- crates/gitbutler-branch-actions/src/stack.rs | 10 +++++- crates/gitbutler-stack-api/src/stack_ext.rs | 37 +++++++++----------- crates/gitbutler-stack-api/tests/mod.rs | 28 +++++++++------ 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/crates/gitbutler-branch-actions/src/stack.rs b/crates/gitbutler-branch-actions/src/stack.rs index 250e22456..85a87bac2 100644 --- a/crates/gitbutler-branch-actions/src/stack.rs +++ b/crates/gitbutler-branch-actions/src/stack.rs @@ -6,6 +6,7 @@ use gitbutler_command_context::CommandContext; use gitbutler_commit::commit_ext::CommitExt; use gitbutler_patch_reference::{CommitOrChangeId, PatchReference}; use gitbutler_project::Project; +use gitbutler_repo::RepoActionsExt; use gitbutler_stack::{Stack, StackId, Target}; use gitbutler_stack_api::{ commit_by_oid_or_change_id, CommitsForId, PatchReferenceUpdate, StackExt, @@ -146,7 +147,14 @@ pub fn push_stack(project: &Project, branch_id: StackId, with_force: bool) -> Re // Nothing to push for this one continue; } - stack.push_series(ctx, series.head.name, with_force)?; + let push_details = stack.push_details(ctx, series.head.name)?; + ctx.push( + push_details.head, + &push_details.remote_refname, + with_force, + None, + Some(Some(stack.id)), + )? } Ok(()) } diff --git a/crates/gitbutler-stack-api/src/stack_ext.rs b/crates/gitbutler-stack-api/src/stack_ext.rs index ecd5c92f6..f31d6e6a2 100644 --- a/crates/gitbutler-stack-api/src/stack_ext.rs +++ b/crates/gitbutler-stack-api/src/stack_ext.rs @@ -12,7 +12,6 @@ use gitbutler_reference::normalize_branch_name; use gitbutler_reference::Refname; use gitbutler_reference::RemoteRefname; use gitbutler_repo::LogUntil; -use gitbutler_repo::RepoActionsExt; use gitbutler_repo::RepositoryExt; use gitbutler_stack::Stack; use gitbutler_stack::VirtualBranchesHandle; @@ -122,14 +121,9 @@ pub trait StackExt { tree: Option, ) -> Result<()>; - /// Pushes the reference (branch) to the Stack remote as derived from the default target. + /// Prepares push details according to the series to be pushed (picking out the correct sha and remote refname) /// This operation will error out if the target has no push remote configured. - fn push_series( - &self, - ctx: &CommandContext, - branch_name: String, - with_force: bool, - ) -> Result<()>; + fn push_details(&self, ctx: &CommandContext, branch_name: String) -> Result; /// Returns a list of all branches/series in the stack. /// This operation will compute the current list of local and remote commits that belong to each series. @@ -176,6 +170,15 @@ pub struct TargetUpdate { pub preceding_head: Option, } +/// Push details to be supplied to `RepoActionsExt`'s `push` method. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct PushDetails { + /// The commit that is being pushed. + pub head: git2::Oid, + /// A remote refname to push to. + pub remote_refname: RemoteRefname, +} + /// A Stack implementation for `gitbutler_branch::Branch` /// This operates via a list of PatchReferences (heads) that are an attribute of gitbutler_branch::Branch. /// In this context a (virtual) "Branch" is a stack of PatchReferences, each pointing to a commit (or change) within the stack. @@ -417,12 +420,7 @@ impl StackExt for Stack { state.set_branch(self.clone()) } - fn push_series( - &self, - ctx: &CommandContext, - branch_name: String, - with_force: bool, - ) -> Result<()> { + fn push_details(&self, ctx: &CommandContext, branch_name: String) -> Result { if !self.initialized() { return Err(anyhow!("Stack has not been initialized")); } @@ -442,13 +440,10 @@ impl StackExt for Stack { let upstream_refname = RemoteRefname::from_str(&reference.remote_reference(remote_name.as_str())?) .context("Failed to parse the remote reference for branch")?; - ctx.push( - commit.id(), - &upstream_refname, - with_force, - None, - Some(Some(self.id)), - ) + Ok(PushDetails { + head: commit.id(), + remote_refname: upstream_refname, + }) } fn list_series(&self, ctx: &CommandContext) -> Result> { diff --git a/crates/gitbutler-stack-api/tests/mod.rs b/crates/gitbutler-stack-api/tests/mod.rs index 43f7e3518..f630de914 100644 --- a/crates/gitbutler-stack-api/tests/mod.rs +++ b/crates/gitbutler-stack-api/tests/mod.rs @@ -3,7 +3,7 @@ use gitbutler_command_context::CommandContext; use gitbutler_commit::commit_ext::CommitExt; use gitbutler_patch_reference::{CommitOrChangeId, PatchReference}; use gitbutler_reference::RemoteRefname; -use gitbutler_repo::{LogUntil, RepositoryExt as _}; +use gitbutler_repo::{LogUntil, RepoActionsExt, RepositoryExt as _}; use gitbutler_stack::VirtualBranchesHandle; use gitbutler_stack_api::{PatchReferenceUpdate, StackExt, TargetUpdate}; use itertools::Itertools; @@ -592,9 +592,7 @@ fn push_series_success() -> Result<()> { target.push_remote_name = Some("origin".into()); state.set_default_target(target)?; - let result = test_ctx - .branch - .push_series(&ctx, "a-branch-2".into(), false); + let result = test_ctx.branch.push_details(&ctx, "a-branch-2".into()); assert!(result.is_ok()); Ok(()) } @@ -610,9 +608,14 @@ fn update_name_after_push() -> Result<()> { target.push_remote_name = Some("origin".into()); state.set_default_target(target)?; - let result = test_ctx - .branch - .push_series(&ctx, "a-branch-2".into(), false); + let push_details = test_ctx.branch.push_details(&ctx, "a-branch-2".into())?; + let result = ctx.push( + push_details.head, + &push_details.remote_refname, + false, + None, + Some(Some(test_ctx.branch.id)), + ); assert!(result.is_ok()); let result = test_ctx.branch.update_series( &ctx, @@ -1065,9 +1068,14 @@ fn set_legacy_refname_pushed() -> Result<()> { let mut target = state.get_default_target()?; target.push_remote_name = Some("origin".into()); state.set_default_target(target)?; - test_ctx - .branch - .push_series(&ctx, "a-branch-2".into(), false)?; + let push_details = test_ctx.branch.push_details(&ctx, "a-branch-2".into())?; + ctx.push( + push_details.head, + &push_details.remote_refname, + false, + None, + Some(Some(test_ctx.branch.id)), + )?; let initial_state = test_ctx.branch.clone(); test_ctx