From e8732f72e2e1cbdf7c3d84791d026e148b2328d9 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Mon, 14 Oct 2024 16:29:38 +0200 Subject: [PATCH 1/2] initialize stack with the same reference as the legacy reference --- crates/gitbutler-stack-api/src/stack_ext.rs | 36 ++++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/crates/gitbutler-stack-api/src/stack_ext.rs b/crates/gitbutler-stack-api/src/stack_ext.rs index 5da97556a..9f5233df4 100644 --- a/crates/gitbutler-stack-api/src/stack_ext.rs +++ b/crates/gitbutler-stack-api/src/stack_ext.rs @@ -233,7 +233,11 @@ impl StackExt for Stack { } else { CommitOrChangeId::CommitId(commit.id().to_string()) }, - name: normalize_branch_name(&self.name)?, + name: if let Some(refname) = self.upstream.as_ref() { + refname.branch().to_string() + } else { + normalize_branch_name(&self.name)? + }, description: None, }; let state = branch_state(ctx); @@ -256,7 +260,7 @@ impl StackExt for Stack { let prefix = rand::random::().to_string(); reference.name = format!("{}-{}", &reference.name, prefix); } - validate_name(&reference, ctx, &state)?; + validate_name(&reference, ctx, &state, self.upstream.clone())?; self.heads = vec![reference]; state.set_branch(self.clone()) } @@ -279,7 +283,7 @@ impl StackExt for Stack { }; let state = branch_state(ctx); let patches = stack_patches(ctx, &state, self.head(), true)?; - validate_name(&new_head, ctx, &state)?; + validate_name(&new_head, ctx, &state, None)?; validate_target(&new_head, ctx, self.head(), &state)?; let updated_heads = add_head(self.heads.clone(), new_head, preceding_head, patches)?; self.heads = updated_heads; @@ -406,7 +410,7 @@ impl StackExt for Stack { } } head.name = name; - validate_name(head, ctx, &state)?; + validate_name(head, ctx, &state, self.upstream.clone())?; } } @@ -651,7 +655,9 @@ fn validate_name( reference: &PatchReference, ctx: &CommandContext, state: &VirtualBranchesHandle, + legacy_branch_ref: Option, ) -> Result<()> { + let legacy_branch_ref = legacy_branch_ref.map(|r| r.branch().to_string()); if reference.name.starts_with("refs/heads") { return Err(anyhow!("Stack head name cannot start with 'refs/heads'")); } @@ -659,19 +665,25 @@ fn validate_name( name_partial(reference.name.as_str().into()).context("Invalid branch name")?; // assert that there is no local git reference with this name if reference_exists(ctx, &reference.name)? { - return Err(anyhow!( - "A git reference with the name {} exists", - &reference.name - )); + // Allow the reference overlap if it is the same as the legacy branch ref + if legacy_branch_ref != Some(reference.name.clone()) { + return Err(anyhow!( + "A git reference with the name {} exists", + &reference.name + )); + } } let default_target = state.get_default_target()?; // assert that there is no remote git reference with this name if let Some(remote_name) = default_target.push_remote_name { if reference_exists(ctx, &reference.remote_reference(remote_name.as_str())?)? { - return Err(anyhow!( - "A git reference with the name {} exists", - &reference.name - )); + // Allow the reference overlap if it is the same as the legacy branch ref + if legacy_branch_ref != Some(reference.name.clone()) { + return Err(anyhow!( + "A git reference with the name {} exists", + &reference.name + )); + } } } // assert that there are no existing patch references with this name From d1685885961f2b3d5ccf2b304f44f46af1abd35c Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Mon, 14 Oct 2024 20:49:49 +0200 Subject: [PATCH 2/2] change default stack series reference to match the virtual branch legacy one if set This is only applicable during the migration period - when a virtual branch has been pushed, and therefore there is a reference set. If that is the case, this will make the default series reference match that reference name --- .../gitbutler-branch-actions/src/virtual.rs | 12 +++- crates/gitbutler-stack-api/src/stack_ext.rs | 37 ++++++++++ crates/gitbutler-stack-api/tests/mod.rs | 72 +++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index fcf1f775c..cd8a689e2 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -292,7 +292,7 @@ pub fn list_virtual_branches_cached( let branches_span = tracing::debug_span!("handle branches", num_branches = status.branches.len()).entered(); - for (branch, mut files) in status.branches { + for (mut branch, mut files) in status.branches { let repo = ctx.repository(); update_conflict_markers(ctx, files.clone())?; @@ -423,7 +423,7 @@ pub fn list_virtual_branches_cached( // TODO: Error out here once this API is stable let series = match stack_series( ctx, - &branch, + &mut branch, &default_target, &check_commit, remote_commit_data, @@ -481,7 +481,7 @@ pub fn list_virtual_branches_cached( /// Newest first, oldest last in the list fn stack_series( ctx: &CommandContext, - branch: &Stack, + branch: &mut Stack, default_target: &Target, check_commit: &IsCommitIntegrated, remote_commit_data: HashMap, @@ -563,6 +563,12 @@ fn stack_series( } api_series.reverse(); + // This is done for compatibility with the legacy flow. + // After a couple of weeks we can get rid of this. + if let Err(e) = branch.set_legacy_compatible_stack_reference(ctx) { + tracing::warn!("failed to set legacy compatible stack reference: {:?}", e); + } + Ok((api_series, requires_force)) } diff --git a/crates/gitbutler-stack-api/src/stack_ext.rs b/crates/gitbutler-stack-api/src/stack_ext.rs index 9f5233df4..35c5e0e25 100644 --- a/crates/gitbutler-stack-api/src/stack_ext.rs +++ b/crates/gitbutler-stack-api/src/stack_ext.rs @@ -152,6 +152,8 @@ pub trait StackExt { from: &Commit<'_>, to: &Commit<'_>, ) -> Result<()>; + + fn set_legacy_compatible_stack_reference(&mut self, ctx: &CommandContext) -> Result<()>; } /// Request to update a PatchReference. @@ -573,6 +575,41 @@ impl StackExt for Stack { } Ok(()) } + + fn set_legacy_compatible_stack_reference(&mut self, ctx: &CommandContext) -> Result<()> { + // self.upstream is only set if this is a branch that was created & manipulated by the legacy flow + let legacy_refname = match self.upstream.clone().map(|r| r.branch().to_owned()) { + Some(legacy_refname) => legacy_refname, + None => return Ok(()), // noop + }; + // update the reference only if there is exactly one series in the stack + if self.heads.len() != 1 { + return Ok(()); // noop + } + let head = match self.heads.first() { + Some(head) => head, + None => return Ok(()), // noop + }; + if legacy_refname == head.name { + return Ok(()); // noop + } + let default_target = branch_state(ctx).get_default_target()?; + let update = PatchReferenceUpdate { + name: Some(legacy_refname), + ..Default::default() + }; + if let Some(remote_name) = default_target.push_remote_name.as_ref() { + // modify the stack reference only if it has not been pushed yet + if !head.pushed(remote_name, ctx).unwrap_or_default() { + // set the stack reference to the legacy refname + self.update_series(ctx, head.name.clone(), &update)?; + } + } else { + // if there is no push remote, just update the stack reference + self.update_series(ctx, head.name.clone(), &update)?; + } + Ok(()) + } } /// Validates that the commit in the reference target diff --git a/crates/gitbutler-stack-api/tests/mod.rs b/crates/gitbutler-stack-api/tests/mod.rs index 8cccc4d96..98b5070cc 100644 --- a/crates/gitbutler-stack-api/tests/mod.rs +++ b/crates/gitbutler-stack-api/tests/mod.rs @@ -2,6 +2,7 @@ use anyhow::Result; 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_stack::VirtualBranchesHandle; use gitbutler_stack_api::{PatchReferenceUpdate, StackExt, TargetUpdate}; @@ -1010,6 +1011,77 @@ fn replace_head_top_of_stack_multiple() -> Result<()> { Ok(()) } +#[test] +fn set_legacy_refname() -> Result<()> { + let (ctx, _temp_dir) = command_ctx("multiple-commits")?; + let mut test_ctx = test_ctx(&ctx)?; + let remote_branch: RemoteRefname = "refs/remotes/origin/my-branch".parse().unwrap(); + test_ctx.branch.upstream = Some(remote_branch.clone()); + test_ctx + .branch + .set_legacy_compatible_stack_reference(&ctx)?; + // reference name was updated + assert_eq!(test_ctx.branch.heads[0].name, "my-branch"); + Ok(()) +} + +#[test] +fn set_legacy_refname_no_upstream_set() -> Result<()> { + let (ctx, _temp_dir) = command_ctx("multiple-commits")?; + let mut test_ctx = test_ctx(&ctx)?; + let initial_state = test_ctx.branch.clone(); + test_ctx + .branch + .set_legacy_compatible_stack_reference(&ctx)?; + // no changes + assert_eq!(initial_state, test_ctx.branch); + Ok(()) +} + +#[test] +fn set_legacy_refname_multiple_heads() -> Result<()> { + let (ctx, _temp_dir) = command_ctx("multiple-commits")?; + let mut test_ctx = test_ctx(&ctx)?; + let remote_branch: RemoteRefname = "refs/remotes/origin/my-branch".parse().unwrap(); + test_ctx.branch.upstream = Some(remote_branch.clone()); + let extra_head = PatchReference { + name: "extra_head".into(), + target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()), + description: None, + }; + // an extra head just beneath the top of the stack + test_ctx.branch.add_series(&ctx, extra_head, None)?; + let initial_state = test_ctx.branch.clone(); + test_ctx + .branch + .set_legacy_compatible_stack_reference(&ctx)?; + // no changes + assert_eq!(initial_state, test_ctx.branch); + Ok(()) +} + +#[test] +fn set_legacy_refname_pushed() -> Result<()> { + let (ctx, _temp_dir) = command_ctx("multiple-commits")?; + let mut test_ctx = test_ctx(&ctx)?; + let remote_branch: RemoteRefname = "refs/remotes/origin/my-branch".parse().unwrap(); + test_ctx.branch.upstream = Some(remote_branch.clone()); + + let state = VirtualBranchesHandle::new(ctx.project().gb_dir()); + 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, "virtual".into(), false)?; + let initial_state = test_ctx.branch.clone(); + + test_ctx + .branch + .set_legacy_compatible_stack_reference(&ctx)?; + // no changes + assert_eq!(initial_state, test_ctx.branch); + Ok(()) +} + fn command_ctx(name: &str) -> Result<(CommandContext, TempDir)> { gitbutler_testsupport::writable::fixture("stacking.sh", name) }