Merge pull request #5362 from gitbutlerapp/kv-branch-1

Retain fully integrated braches on the stack but flag them as archived
This commit is contained in:
Kiril Videlov 2024-10-30 13:51:19 +01:00 committed by GitHub
commit 7be3e7a6e7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 82 additions and 34 deletions

View File

@ -18,13 +18,15 @@
branch.series.flatMap((s) => s.patches).some((patch) => patch.conflicted)
);
const nonArchivedSeries = $derived(branch.series.filter((s) => !s.archived));
const reorderDropzoneManagerFactory = getContext(ReorderDropzoneManagerFactory);
const reorderDropzoneManager = $derived(
reorderDropzoneManagerFactory.build(branch, [...branch.localCommits, ...branch.remoteCommits])
);
</script>
{#each branch.series as currentSeries, idx (currentSeries.name)}
{#each nonArchivedSeries as currentSeries, idx (currentSeries.name)}
{@const isTopSeries = idx === 0}
{#if !isTopSeries}
<StackSeriesDividerLine {currentSeries} />

View File

@ -450,6 +450,11 @@ export class PatchSeries {
* The list is empty if there is no review units, eg. no Pull Request has been created.
*/
forgeId?: ForgeIdentifier | undefined;
/**
* Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
* This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
*/
archived!: boolean;
get localCommits() {
return this.patches.filter((c) => c.status === 'local');

View File

@ -46,6 +46,7 @@ pub fn create_series(
name: req.name,
description: req.description,
forge_id: Default::default(),
archived: Default::default(),
},
req.preceding_head,
)
@ -283,6 +284,7 @@ pub(crate) fn stack_series(
patches,
upstream_patches,
forge_id: series.head.forge_id,
archived: series.head.archived,
});
}
api_series.reverse();

View File

@ -322,7 +322,7 @@ pub(crate) fn integrate_upstream(
};
branch.set_stack_head(command_context, *head, Some(*tree))?;
branch.prune_integrated_heads(command_context)?;
branch.archive_integrated_heads(command_context)?;
}
// checkout_branch_trees won't checkout anything if there are no

View File

@ -100,6 +100,9 @@ pub struct PatchSeries {
/// A list of identifiers for the review unit at possible forges (eg. Pull Request).
/// The list is empty if there is no review units, eg. no Pull Request has been created.
pub forge_id: Option<ForgeIdentifier>,
/// Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
/// This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
pub archived: bool,
}
#[derive(Debug, PartialEq, Clone, Serialize)]

View File

@ -21,6 +21,10 @@ pub struct PatchReference {
/// None if is no review unit, eg. no Pull Request has been created.
#[serde(default)]
pub forge_id: Option<ForgeIdentifier>,
/// Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
/// This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
#[serde(default)]
pub archived: bool,
}
/// Represents identifiers for the series at possible forges, eg. GitHub PR numbers.

View File

@ -28,6 +28,9 @@ pub struct Series<'a> {
/// The commit IDs of the remote commits that are part of this series, grouped by change id.
/// Since we don't have a change_id to commit_id index, this is used to determine
pub remote_commit_ids_by_change_id: HashMap<String, git2::Oid>,
/// Archived represents the state when series/branch has been integrated and is below the merge base of the branch.
/// This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
pub archived: bool,
}
impl Series<'_> {

View File

@ -231,6 +231,7 @@ impl Stack {
},
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let state = branch_state(ctx);
@ -305,6 +306,7 @@ impl Stack {
name,
description,
forge_id: Default::default(),
archived: Default::default(),
};
self.add_series(ctx, new_head, Some(current_top_head.name.clone()))
}
@ -438,20 +440,18 @@ impl Stack {
}
/// Removes any heads that are refering to commits that are no longer between the stack head and the merge base
pub fn prune_integrated_heads(&mut self, ctx: &CommandContext) -> Result<()> {
pub fn archive_integrated_heads(&mut self, ctx: &CommandContext) -> Result<()> {
if !self.initialized() {
return Err(anyhow!("Stack has not been initialized"));
}
self.updated_timestamp_ms = gitbutler_time::time::now_ms();
let state = branch_state(ctx);
let commit_ids = stack_patches(ctx, &state, self.head(), true)?;
let new_heads = self
.heads
.iter()
.filter(|h| commit_ids.contains(&h.target))
.cloned()
.collect_vec();
self.heads = new_heads;
for head in self.heads.iter_mut() {
if !commit_ids.contains(&head.target) {
head.archived = true;
}
}
state.set_branch(self.clone())
}
@ -498,19 +498,19 @@ impl Stack {
let mut previous_head = repo.merge_base(self.head(), default_target.sha)?;
for head in self.heads.clone() {
let head_commit =
match commit_by_oid_or_change_id(&head.target, repo, self.head(), merge_base) {
Ok(commits_for_id) => commits_for_id.head.id(),
Err(e) => {
// The series may have been integrated
tracing::warn!(
"Failed to find commit with commit_or_change_id: {} for head: {}, {}",
head.target,
head.name,
e
);
continue;
}
};
commit_by_oid_or_change_id(&head.target, repo, self.head(), merge_base);
if head.archived || head_commit.is_err() {
all_series.push(Series {
head: head.clone(),
local_commits: vec![],
remote_commits: vec![],
upstream_only_commits: vec![],
remote_commit_ids_by_change_id: HashMap::new(),
archived: head.archived,
});
continue;
}
let head_commit = head_commit?.head.id();
let mut local_patches = vec![];
for commit in repo
@ -559,6 +559,7 @@ impl Stack {
remote_commits: remote_patches,
upstream_only_commits: upstream_only,
remote_commit_ids_by_change_id,
archived: head.archived,
});
previous_head = head_commit;
}

View File

@ -61,6 +61,7 @@ fn add_series_success() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: Some("my description".into()),
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference, None);
assert!(result.is_ok());
@ -116,6 +117,7 @@ fn add_series_top_base() -> Result<()> {
target: CommitOrChangeId::CommitId(merge_base.id().to_string()),
description: Some("my description".into()),
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference, None);
println!("{:?}", result);
@ -142,6 +144,7 @@ fn add_multiple_series() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.last().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx
.branch
@ -154,6 +157,7 @@ fn add_multiple_series() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.last().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, head_2, None);
assert!(result.is_ok());
@ -167,6 +171,7 @@ fn add_multiple_series() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.first().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, head_1, None);
@ -176,9 +181,9 @@ fn add_multiple_series() -> Result<()> {
vec!["head_1", "head_2", "a-branch-2", "head_4"]
);
// prune is noop
// archive is noop
let before_prune = test_ctx.branch.heads.clone();
test_ctx.branch.prune_integrated_heads(&ctx)?;
test_ctx.branch.archive_integrated_heads(&ctx)?;
assert_eq!(before_prune, test_ctx.branch.heads);
Ok(())
}
@ -193,6 +198,7 @@ fn add_series_commit_id_when_change_id_available() -> Result<()> {
target: CommitOrChangeId::CommitId(test_ctx.commits[1].id().to_string()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference, None);
assert_eq!(
@ -215,6 +221,7 @@ fn add_series_invalid_name_fails() -> Result<()> {
target: CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference, None);
assert_eq!(result.err().unwrap().to_string(), "Invalid branch name");
@ -231,6 +238,7 @@ fn add_series_duplicate_name_fails() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference.clone(), None);
assert!(result.is_ok());
@ -252,6 +260,7 @@ fn add_series_matching_git_ref_is_ok() -> Result<()> {
target: test_ctx.commits[0].clone().into(),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference.clone(), None);
assert!(result.is_ok()); // allow this
@ -268,6 +277,7 @@ fn add_series_including_refs_head_fails() -> Result<()> {
target: CommitOrChangeId::CommitId(test_ctx.commits[0].id().to_string()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference.clone(), None);
assert_eq!(
@ -287,6 +297,7 @@ fn add_series_target_commit_doesnt_exist() -> Result<()> {
target: CommitOrChangeId::CommitId("30696678319e0fa3a20e54f22d47fc8cf1ceaade".into()), // does not exist
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference.clone(), None);
assert!(result
@ -307,6 +318,7 @@ fn add_series_target_change_id_doesnt_exist() -> Result<()> {
target: CommitOrChangeId::ChangeId("does-not-exist".into()), // does not exist
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference.clone(), None);
assert_eq!(
@ -327,6 +339,7 @@ fn add_series_target_commit_not_in_stack() -> Result<()> {
target: CommitOrChangeId::CommitId(other_commit_id.clone()), // does not exist
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, reference.clone(), None);
assert_eq!(
@ -384,6 +397,7 @@ fn remove_series_with_multiple_last_heads() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.last().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, to_stay.clone(), None);
assert!(result.is_ok());
@ -416,6 +430,7 @@ fn remove_series_no_orphan_commits() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.first().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
}; // references the oldest commit
let result = test_ctx.branch.add_series(&ctx, to_stay.clone(), None);
assert!(result.is_ok());
@ -579,6 +594,7 @@ fn update_series_target_success() -> Result<()> {
target: commit_0_change_id.clone(),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let result = test_ctx.branch.add_series(&ctx, series_1, None);
assert!(result.is_ok());
@ -681,6 +697,7 @@ fn list_series_two_heads_same_commit() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.last().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
// add `head_before` before the initial head
let result = test_ctx.branch.add_series(&ctx, head_before, None);
@ -717,6 +734,7 @@ fn list_series_two_heads_different_commit() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits.first().unwrap().change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
// add `head_before` before the initial head
let result = test_ctx.branch.add_series(&ctx, head_before, None);
@ -782,6 +800,7 @@ fn replace_head_single() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
test_ctx.branch.add_series(&ctx, from_head, None)?;
// replace with previous head
@ -814,6 +833,7 @@ fn replace_head_single_with_merge_base() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
test_ctx.branch.add_series(&ctx, from_head, None)?;
// replace with merge base
@ -850,6 +870,7 @@ fn replace_head_with_invalid_commit_error() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
test_ctx.branch.add_series(&ctx, from_head, None)?;
let stack = test_ctx.branch.clone();
@ -877,6 +898,7 @@ fn replace_head_with_same_noop() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
test_ctx.branch.add_series(&ctx, from_head, None)?;
let stack = test_ctx.branch.clone();
@ -963,12 +985,14 @@ fn replace_head_multiple() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
let from_head_2 = PatchReference {
name: "from_head_2".into(),
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
// both references point to the same commit
test_ctx.branch.add_series(&ctx, from_head_1, None)?;
@ -1009,6 +1033,7 @@ fn replace_head_top_of_stack_multiple() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
// an extra head just beneath the top of the stack
test_ctx.branch.add_series(&ctx, extra_head, None)?;
@ -1075,6 +1100,7 @@ fn set_legacy_refname_multiple_heads() -> Result<()> {
target: CommitOrChangeId::ChangeId(test_ctx.commits[1].change_id().unwrap()),
description: None,
forge_id: Default::default(),
archived: Default::default(),
};
// an extra head just beneath the top of the stack
test_ctx.branch.add_series(&ctx, extra_head, None)?;
@ -1117,12 +1143,12 @@ fn set_legacy_refname_pushed() -> Result<()> {
}
#[test]
fn prune_heads_noop() -> Result<()> {
fn archive_heads_noop() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let initial_state = test_ctx.branch.heads.clone();
test_ctx.branch.prune_integrated_heads(&ctx)?;
test_ctx.branch.archive_integrated_heads(&ctx)?;
assert_eq!(initial_state, test_ctx.branch.heads);
// Assert persisted
assert_eq!(
@ -1133,11 +1159,10 @@ fn prune_heads_noop() -> Result<()> {
}
#[test]
fn prune_heads_success() -> Result<()> {
fn archive_heads_success() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
let initial_state = test_ctx.branch.heads.clone();
// adding a commit that is not in the stack
test_ctx.branch.heads.insert(
0,
@ -1146,12 +1171,14 @@ fn prune_heads_success() -> Result<()> {
name: "foo".to_string(),
description: None,
forge_id: Default::default(),
archived: Default::default(),
},
);
assert_eq!(test_ctx.branch.heads.len(), 2);
test_ctx.branch.prune_integrated_heads(&ctx)?;
assert_eq!(test_ctx.branch.heads.len(), 1);
assert_eq!(initial_state, test_ctx.branch.heads);
test_ctx.branch.archive_integrated_heads(&ctx)?;
assert_eq!(test_ctx.branch.heads.len(), 2);
assert!(test_ctx.branch.heads[0].archived);
assert!(!test_ctx.branch.heads[1].archived);
// Assert persisted
assert_eq!(
test_ctx.branch,
@ -1161,7 +1188,7 @@ fn prune_heads_success() -> Result<()> {
}
#[test]
fn does_not_prune_head_on_merge_base() -> Result<()> {
fn does_not_archive_head_on_merge_base() -> Result<()> {
let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
let mut test_ctx = test_ctx(&ctx)?;
test_ctx.branch.initialize(&ctx)?;
@ -1176,11 +1203,12 @@ fn does_not_prune_head_on_merge_base() -> Result<()> {
name: "bottom".to_string(),
description: None,
forge_id: Default::default(),
archived: Default::default(),
},
None,
)?;
let initial_state = test_ctx.branch.heads.clone();
test_ctx.branch.prune_integrated_heads(&ctx)?;
test_ctx.branch.archive_integrated_heads(&ctx)?;
assert_eq!(initial_state, test_ctx.branch.heads);
// Assert persisted
assert_eq!(