Make stack initialization more forgiving

This commit is contained in:
Kiril Videlov 2024-10-02 19:28:29 +02:00
parent f699b72768
commit 89960f6bfc
5 changed files with 55 additions and 41 deletions

2
Cargo.lock generated
View File

@ -2187,6 +2187,7 @@ dependencies = [
"gitbutler-reference",
"gitbutler-repo",
"gitbutler-serde",
"gitbutler-stack",
"gitbutler-testsupport",
"gitbutler-time",
"gitbutler-url",
@ -2552,6 +2553,7 @@ dependencies = [
"gitbutler-testsupport",
"gix",
"itertools 0.13.0",
"rand 0.8.5",
"serde",
"tempfile",
]

View File

@ -27,6 +27,7 @@ gitbutler-diff.workspace = true
gitbutler-operating-modes.workspace = true
gitbutler-cherry-pick.workspace = true
gitbutler-oxidize.workspace = true
gitbutler-stack.workspace = true
serde = { workspace = true, features = ["std"] }
bstr.workspace = true
diffy = "0.4.0"

View File

@ -8,6 +8,7 @@ publish = false
[dependencies]
anyhow = "1.0.86"
itertools = "0.13"
rand = "0.8.5"
serde = { workspace = true, features = ["std"] }
git2.workspace = true
gix.workspace = true

View File

@ -40,7 +40,7 @@ pub trait Stack {
/// Errors out if the stack has already been initialized.
///
/// This operation mutates the gitbutler::Branch.heads list and updates the state in `virtual_branches.toml`
fn init(&mut self, ctx: &CommandContext) -> Result<()>;
fn initialize(&mut self, ctx: &CommandContext) -> Result<()>;
/// Adds a new "Branch" to the Stack.
/// This is in fact just creating a new GitButler patch reference (head) and associates it with the stack.
@ -131,12 +131,12 @@ impl Stack for Branch {
fn initialized(&self) -> bool {
!self.heads.is_empty()
}
fn init(&mut self, ctx: &CommandContext) -> Result<()> {
fn initialize(&mut self, ctx: &CommandContext) -> Result<()> {
if self.initialized() {
return Err(anyhow!("Stack already initialized"));
return Ok(());
}
let commit = ctx.repository().find_commit(self.head)?;
let reference = PatchReference {
let mut reference = PatchReference {
target: if let Some(change_id) = commit.change_id() {
CommitOrChangeId::ChangeId(change_id.to_string())
} else {
@ -146,6 +146,13 @@ impl Stack for Branch {
description: None,
};
let state = branch_state(ctx);
if reference_exists(ctx, &reference.name)?
|| patch_reference_exists(&state, &reference.name)?
{
// TODO: do something better here
let prefix = rand::random::<u32>().to_string();
reference.name = format!("{}-{}", &reference.name, prefix);
}
validate_name(&reference, ctx, &state)?;
self.heads = vec![reference];
state.set_branch(self.clone())
@ -406,12 +413,7 @@ fn validate_name(
}
}
// assert that there are no existing patch references with this name
if state
.list_all_branches()?
.iter()
.flat_map(|b| b.heads.iter())
.any(|r| r.name == reference.name)
{
if patch_reference_exists(state, &reference.name)? {
return Err(anyhow!(
"A patch reference with the name {} exists",
&reference.name
@ -469,3 +471,11 @@ fn reference_exists(ctx: &CommandContext, name: &str) -> Result<bool> {
let gix_repo = ctx.gix_repository()?;
Ok(gix_repo.find_reference(name_partial(name.into())?).is_ok())
}
fn patch_reference_exists(state: &VirtualBranchesHandle, name: &str) -> Result<bool> {
Ok(state
.list_all_branches()?
.iter()
.flat_map(|b| b.heads.iter())
.any(|r| r.name == name))
}

View File

@ -15,7 +15,7 @@ fn init_success() -> Result<()> {
let mut branch = test_ctx.branch;
assert!(!branch.initialized());
assert_eq!(branch.heads.len(), 0);
let result = branch.init(&ctx);
let result = branch.initialize(&ctx);
assert!(result.is_ok());
assert!(branch.initialized());
assert_eq!(branch.heads.len(), 1);
@ -35,14 +35,14 @@ fn init_success() -> Result<()> {
}
#[test]
fn init_already_initialized_fails() -> Result<()> {
fn init_already_initialized_noop() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let test_ctx = test_ctx(&ctx)?;
let mut branch = test_ctx.branch;
let result = branch.init(&ctx);
let result = branch.initialize(&ctx);
assert!(result.is_ok());
let result = branch.init(&ctx);
assert!(result.is_err());
let result = branch.initialize(&ctx);
assert!(result.is_ok()); // noop
Ok(())
}
@ -50,7 +50,7 @@ fn init_already_initialized_fails() -> Result<()> {
fn add_series_success() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "asdf".into(),
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
@ -76,7 +76,7 @@ fn add_series_success() -> Result<()> {
fn add_multiple_series() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
assert_eq!(test_ctx.branch.heads.len(), 1);
assert_eq!(head_names(&test_ctx), vec!["virtual"]); // defalts to stack name
@ -121,7 +121,7 @@ fn add_multiple_series() -> Result<()> {
fn add_series_commitid_when_changeid_available() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "asdf".into(),
target: CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()),
@ -159,7 +159,7 @@ fn add_series_uninitialized_fails() -> Result<()> {
fn add_series_invalid_name_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "name with spaces".into(),
target: CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()),
@ -174,7 +174,7 @@ fn add_series_invalid_name_fails() -> Result<()> {
fn add_series_duplicate_name_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "asdf".into(),
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
@ -194,7 +194,7 @@ fn add_series_duplicate_name_fails() -> Result<()> {
fn add_series_matching_git_ref_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "existing-branch".into(),
target: CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()),
@ -212,7 +212,7 @@ fn add_series_matching_git_ref_fails() -> Result<()> {
fn add_series_including_refs_head_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "refs/heads/my-branch".into(),
target: CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()),
@ -230,7 +230,7 @@ fn add_series_including_refs_head_fails() -> Result<()> {
fn add_series_target_commit_doesnt_exist() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "my-branch".into(),
target: CommitOrChangeId::CommitId("30696678319e0fa3a20e54f22d47fc8cf1ceaade".into()), // does not exist
@ -249,7 +249,7 @@ fn add_series_target_commit_doesnt_exist() -> Result<()> {
fn add_series_target_change_id_doesnt_exist() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let reference = PatchReference {
name: "my-branch".into(),
target: CommitOrChangeId::ChangeId("does-not-exist".into()), // does not exist
@ -267,7 +267,7 @@ fn add_series_target_change_id_doesnt_exist() -> Result<()> {
fn add_series_target_commit_not_in_stack() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let other_commit_id = test_ctx.other_commits.last().unwrap().id().to_string();
let reference = PatchReference {
name: "my-branch".into(),
@ -301,7 +301,7 @@ fn remove_series_uninitialized_fails() -> Result<()> {
fn remove_series_last_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let result = test_ctx
.branch
.remove_series(&ctx, test_ctx.branch.heads[0].name.clone());
@ -316,7 +316,7 @@ fn remove_series_last_fails() -> Result<()> {
fn remove_series_nonexistent_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let result = test_ctx
.branch
.remove_series(&ctx, "does-not-exist".to_string());
@ -331,7 +331,7 @@ fn remove_series_nonexistent_fails() -> Result<()> {
fn remove_series_with_multiple_last_heads() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
assert_eq!(test_ctx.branch.heads.len(), 1);
assert_eq!(head_names(&test_ctx), vec!["virtual"]); // defalts to stack name
@ -362,7 +362,7 @@ fn remove_series_with_multiple_last_heads() -> Result<()> {
fn remove_series_no_orphan_commits() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
assert_eq!(test_ctx.branch.heads.len(), 1);
assert_eq!(head_names(&test_ctx), vec!["virtual"]); // defalts to stack name
@ -412,7 +412,7 @@ fn update_series_uninitialized_fails() -> Result<()> {
fn update_series_noop_does_nothing() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let heads_before = test_ctx.branch.heads.clone();
let noop_update = PatchReferenceUpdate::default();
let result = test_ctx
@ -427,7 +427,7 @@ fn update_series_noop_does_nothing() -> Result<()> {
fn update_series_name_fails_validation() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let update = PatchReferenceUpdate {
name: Some("invalid name".into()),
target_update: None,
@ -444,7 +444,7 @@ fn update_series_name_fails_validation() -> Result<()> {
fn update_series_name_success() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let update = PatchReferenceUpdate {
name: Some("new-name".into()),
target_update: None,
@ -467,7 +467,7 @@ fn update_series_name_success() -> Result<()> {
fn update_series_set_description() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let update = PatchReferenceUpdate {
name: None,
target_update: None,
@ -493,7 +493,7 @@ fn update_series_set_description() -> Result<()> {
fn update_series_target_fails_commit_not_in_stack() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let other_commit_id = test_ctx.other_commits.last().unwrap().id().to_string();
let update = PatchReferenceUpdate {
name: None,
@ -520,7 +520,7 @@ fn update_series_target_fails_commit_not_in_stack() -> Result<()> {
fn update_series_target_orphan_commit_fails() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let initial_state = test_ctx.branch.heads.clone();
let first_commit_change_id = test_ctx.commits.first().unwrap().change_id().unwrap();
let update = PatchReferenceUpdate {
@ -547,7 +547,7 @@ fn update_series_target_orphan_commit_fails() -> Result<()> {
fn update_series_target_success() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let commit_0_change_id = CommitOrChangeId::ChangeId(test_ctx.commits[0].change_id().unwrap());
let series_1 = PatchReference {
name: "series_1".into(),
@ -595,7 +595,7 @@ fn push_series_uninitialized_fails() -> Result<()> {
fn push_series_no_remote() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let result = test_ctx.branch.push_series(&ctx, "virtual".into(), false);
assert_eq!(
result.err().unwrap().to_string(),
@ -608,7 +608,7 @@ fn push_series_no_remote() -> Result<()> {
fn push_series_success() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let state = VirtualBranchesHandle::new(ctx.project().gb_dir());
let mut target = state.get_default_target()?;
@ -636,7 +636,7 @@ fn list_series_uninitialized() -> Result<()> {
fn list_series_default_head() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let result = test_ctx.branch.list_series(&ctx);
assert!(result.is_ok());
let result = result.unwrap();
@ -656,7 +656,7 @@ fn list_series_default_head() -> Result<()> {
fn list_series_two_heads_same_commit() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let head_before = PatchReference {
name: "head_before".into(),
target: CommitOrChangeId::ChangeId(test_ctx.commits.last().unwrap().change_id().unwrap()),
@ -690,7 +690,7 @@ fn list_series_two_heads_same_commit() -> Result<()> {
fn list_series_two_heads_different_commit() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.init(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let head_before = PatchReference {
name: "head_before".into(),
// point to the first commit