Merge pull request #5174 from gitbutlerapp/revwalk-simplify-first-parent

When revwalking only get the commits from the first parent
This commit is contained in:
Kiril Videlov 2024-10-16 18:51:51 +02:00 committed by GitHub
commit 666c167e5e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 158 additions and 86 deletions

View File

@ -321,14 +321,14 @@ pub(crate) fn target_to_base_branch(ctx: &CommandContext, target: &Target) -> Re
let (number_commits_ahead, number_commits_behind) = repo.graph_ahead_behind(target.sha, oid)?;
let diverged_ahead = repo
.log(target.sha, LogUntil::Take(number_commits_ahead))
.log(target.sha, LogUntil::Take(number_commits_ahead), false)
.context("failed to get fork point")?
.iter()
.map(|commit| commit.id())
.collect::<Vec<_>>();
let diverged_behind = repo
.log(oid, LogUntil::Take(number_commits_behind))
.log(oid, LogUntil::Take(number_commits_behind), false)
.context("failed to get fork point")?
.iter()
.map(|commit| commit.id())
@ -339,7 +339,7 @@ pub(crate) fn target_to_base_branch(ctx: &CommandContext, target: &Target) -> Re
// gather a list of commits between oid and target.sha
let upstream_commits = repo
.log(oid, LogUntil::Commit(target.sha))
.log(oid, LogUntil::Commit(target.sha), false)
.context("failed to get upstream commits")?
.iter()
.map(commit_to_remote_commit)
@ -347,7 +347,7 @@ pub(crate) fn target_to_base_branch(ctx: &CommandContext, target: &Target) -> Re
// get some recent commits
let recent_commits = repo
.log(target.sha, LogUntil::Take(20))
.log(target.sha, LogUntil::Take(20), false)
.context("failed to get recent commits")?
.iter()
.map(commit_to_remote_commit)

View File

@ -329,7 +329,8 @@ impl BranchManager<'_> {
// Do we need to rebase the branch on top of the default target?
if merge_base != default_target.sha {
let new_head = if branch.allow_rebasing {
let commits_to_rebase = repo.l(branch.head(), LogUntil::Commit(merge_base))?;
let commits_to_rebase =
repo.l(branch.head(), LogUntil::Commit(merge_base), false)?;
let head_oid = cherry_rebase_group(repo, default_target.sha, &commits_to_rebase)?;

View File

@ -149,11 +149,16 @@ fn order_commits_for_rebasing(
let merge_base =
repository.merge_base_octopussy(&[target_branch_head, branch_head, remote_head])?;
let target_branch_commits = repository.l(target_branch_head, LogUntil::Commit(merge_base))?;
let local_branch_commits = repository.l(branch_head, LogUntil::Commit(merge_base))?;
let target_branch_commits =
repository.l(target_branch_head, LogUntil::Commit(merge_base), false)?;
let local_branch_commits = repository.l(branch_head, LogUntil::Commit(merge_base), false)?;
let remote_local_merge_base = repository.merge_base(branch_head, remote_head)?;
let remote_commits = repository.l(remote_head, LogUntil::Commit(remote_local_merge_base))?;
let remote_commits = repository.l(
remote_head,
LogUntil::Commit(remote_local_merge_base),
false,
)?;
let (integrated_commits, filtered_remote_commits) =
remote_commits.into_iter().partition(|remote_commit| {
@ -225,7 +230,7 @@ mod test {
assert_eq!(
test_repository
.repository
.l(head, LogUntil::Commit(base_commit.id()))
.l(head, LogUntil::Commit(base_commit.id()), false)
.unwrap(),
vec![remote_y.id(), remote_x.id(), local_b.id(), local_a.id()],
);
@ -279,7 +284,7 @@ mod test {
let commits = test_repository
.repository
.log(head, LogUntil::Commit(base_commit.id()))
.log(head, LogUntil::Commit(base_commit.id()), false)
.unwrap();
assert_eq!(commits.len(), 4);
@ -358,7 +363,7 @@ mod test {
let commits = test_repository
.repository
.log(head, LogUntil::Commit(base_commit.id()))
.log(head, LogUntil::Commit(base_commit.id()), false)
.unwrap();
assert_eq!(commits.len(), 4);
@ -450,7 +455,7 @@ mod test {
let commits = test_repository
.repository
.log(head, LogUntil::Commit(base_commit.id()))
.log(head, LogUntil::Commit(base_commit.id()), false)
.unwrap();
assert_eq!(commits.len(), 3);

View File

@ -330,7 +330,11 @@ fn verify_head_is_clean(ctx: &CommandContext, perm: &mut WorktreeWritePermission
let commits = ctx
.repository()
.log(head_commit.id(), LogUntil::Commit(default_target.sha))
.log(
head_commit.id(),
LogUntil::Commit(default_target.sha),
false,
)
.context("failed to get log")?;
let workspace_index = commits

View File

@ -76,18 +76,21 @@ pub(crate) fn move_commit(
gitbutler_diff::diff_files_into_hunks(source_commit_diff).collect();
let is_source_locked = check_source_lock(source_branch_non_comitted_files, &source_commit_diff);
let mut ancestor_commits = ctx
.repository()
.log(source_commit_parent.id(), LogUntil::Commit(merge_base.id()))?;
let mut ancestor_commits = ctx.repository().log(
source_commit_parent.id(),
LogUntil::Commit(merge_base.id()),
false,
)?;
ancestor_commits.push(merge_base);
let ancestor_commits = ancestor_commits;
let mut descendant_commits = None;
if !is_head_commit {
descendant_commits = Some(
ctx.repository()
.log(source_branch.head(), LogUntil::Commit(commit_id))?,
);
descendant_commits = Some(ctx.repository().log(
source_branch.head(),
LogUntil::Commit(commit_id),
false,
)?);
}
let is_ancestor_locked =

View File

@ -169,7 +169,7 @@ pub(crate) fn branch_to_remote_branch_data(
.map(|sha| {
let ahead = ctx
.repository()
.log(sha, LogUntil::Commit(base))
.log(sha, LogUntil::Commit(base), false)
.context("failed to get ahead commits")?;
let name = Refname::try_from(branch).context("could not get branch name")?;

View File

@ -54,7 +54,7 @@ pub(crate) fn reorder_commit(
let mut branch = vb_state.get_branch_in_workspace(branch_id)?;
let original_commits = repository.l(branch.head(), LogUntil::Commit(merge_base))?;
let original_commits = repository.l(branch.head(), LogUntil::Commit(merge_base), false)?;
let ReorderResult {
tree,
head,
@ -441,7 +441,7 @@ mod test {
let commits: Vec<git2::Commit> = test_repository
.repository
.log(result.head, LogUntil::Commit(merge_base.id()))
.log(result.head, LogUntil::Commit(merge_base.id()), false)
.unwrap();
assert_eq!(
@ -482,7 +482,7 @@ mod test {
let commits: Vec<git2::Commit> = test_repository
.repository
.log(result.head, LogUntil::Commit(merge_base.id()))
.log(result.head, LogUntil::Commit(merge_base.id()), false)
.unwrap();
assert_eq!(commits.len(), 3);

View File

@ -56,8 +56,11 @@ fn inner_undo_commit(
return Ok(commit_to_remove.parent(0)?.id());
};
let commits_to_rebase =
repository.l(branch_head_commit, LogUntil::Commit(commit_to_remove.id()))?;
let commits_to_rebase = repository.l(
branch_head_commit,
LogUntil::Commit(commit_to_remove.id()),
false,
)?;
let new_head = cherry_rebase_group(
repository,

View File

@ -377,7 +377,7 @@ pub(crate) fn resolve_upstream_integration(
Ok(new_head.id())
}
BaseBranchResolutionApproach::Rebase => {
let commits = repo.l(old_target_id, LogUntil::Commit(fork_point))?;
let commits = repo.l(old_target_id, LogUntil::Commit(fork_point), false)?;
let new_head = cherry_rebase_group(repo, new_target_id, &commits)?;
Ok(new_head)
@ -457,8 +457,11 @@ fn compute_resolutions(
};
// Rebase virtual branches' commits
let virtual_branch_commits =
repository.l(virtual_branch.head(), LogUntil::Commit(lower_bound))?;
let virtual_branch_commits = repository.l(
virtual_branch.head(),
LogUntil::Commit(lower_bound),
false,
)?;
let new_head =
cherry_rebase_group(repository, new_target.id(), &virtual_branch_commits)?;
@ -757,7 +760,11 @@ mod test {
let commits_to_rebase = test_repository
.repository
.l(old_target.id(), LogUntil::Commit(initial_commit.id()))
.l(
old_target.id(),
LogUntil::Commit(initial_commit.id()),
false,
)
.unwrap();
let head_after_rebase = cherry_rebase_group(
&test_repository.repository,
@ -828,7 +835,11 @@ mod test {
let commits_to_rebase = test_repository
.repository
.l(old_target.id(), LogUntil::Commit(initial_commit.id()))
.l(
old_target.id(),
LogUntil::Commit(initial_commit.id()),
false,
)
.unwrap();
let head_after_rebase = cherry_rebase_group(
&test_repository.repository,

View File

@ -322,8 +322,11 @@ pub fn list_virtual_branches_cached(
upstream.id(),
default_target.sha
))?;
let remote_commit_ids =
HashSet::from_iter(repo.l(upstream.id(), LogUntil::Commit(merge_base))?);
let remote_commit_ids = HashSet::from_iter(repo.l(
upstream.id(),
LogUntil::Commit(merge_base),
false,
)?);
let remote_commit_data: HashMap<_, _> = remote_commit_ids
.iter()
.copied()
@ -344,7 +347,7 @@ pub fn list_virtual_branches_cached(
let mut is_remote = false;
// find all commits on head that are not on target.sha
let commits = repo.log(branch.head(), LogUntil::Commit(default_target.sha))?;
let commits = repo.log(branch.head(), LogUntil::Commit(default_target.sha), false)?;
let check_commit = IsCommitIntegrated::new(ctx, &default_target)?;
let vbranch_commits = {
let _span = tracing::debug_span!(
@ -793,7 +796,7 @@ pub(crate) fn reset_branch(
if default_target.sha != target_commit_id
&& !ctx
.repository()
.l(branch.head(), LogUntil::Commit(default_target.sha))?
.l(branch.head(), LogUntil::Commit(default_target.sha), false)?
.contains(&target_commit_id)
{
bail!("commit {target_commit_id} not in the branch");
@ -1048,9 +1051,9 @@ impl<'repo> IsCommitIntegrated<'repo> {
.maybe_find_branch_by_refname(&target.branch.clone().into())?
.ok_or(anyhow!("failed to get branch"))?;
let remote_head = remote_branch.get().peel_to_commit()?;
let upstream_commits = ctx
.repository()
.l(remote_head.id(), LogUntil::Commit(target.sha))?;
let upstream_commits =
ctx.repository()
.l(remote_head.id(), LogUntil::Commit(target.sha), false)?;
let inmemory_repo = ctx.repository().in_memory_repo()?;
Ok(Self {
repo: ctx.repository(),
@ -1180,9 +1183,11 @@ pub(crate) fn move_commit_file(
.context("failed to find commit")?;
// find all the commits upstream from the target "to" commit
let mut upstream_commits = ctx
.repository()
.l(target_branch.head(), LogUntil::Commit(amend_commit.id()))?;
let mut upstream_commits = ctx.repository().l(
target_branch.head(),
LogUntil::Commit(amend_commit.id()),
false,
)?;
// get a list of all the diffs across all the virtual branches
let base_file_diffs = gitbutler_diff::workdir(ctx.repository(), default_target.sha)
@ -1310,13 +1315,15 @@ pub(crate) fn move_commit_file(
// ok, now we need to identify which the new "to" commit is in the rebased history
// so we'll take a list of the upstream oids and find it simply based on location
// (since the order should not have changed in our simple rebase)
let old_upstream_commit_oids = ctx
.repository()
.l(target_branch.head(), LogUntil::Commit(default_target.sha))?;
let old_upstream_commit_oids = ctx.repository().l(
target_branch.head(),
LogUntil::Commit(default_target.sha),
false,
)?;
let new_upstream_commit_oids = ctx
.repository()
.l(new_head, LogUntil::Commit(default_target.sha))?;
let new_upstream_commit_oids =
ctx.repository()
.l(new_head, LogUntil::Commit(default_target.sha), false)?;
// find to_commit_oid offset in upstream_commits vector
let to_commit_offset = old_upstream_commit_oids
@ -1336,9 +1343,9 @@ pub(crate) fn move_commit_file(
.context("failed to find commit")?;
// reset the concept of what the upstream commits are to be the rebased ones
upstream_commits = ctx
.repository()
.l(new_head, LogUntil::Commit(amend_commit.id()))?;
upstream_commits =
ctx.repository()
.l(new_head, LogUntil::Commit(amend_commit.id()), false)?;
}
// ok, now we will apply the moved changes to the "to" commit.
@ -1427,7 +1434,11 @@ pub(crate) fn amend(
if ctx
.repository()
.l(target_branch.head(), LogUntil::Commit(default_target.sha))?
.l(
target_branch.head(),
LogUntil::Commit(default_target.sha),
false,
)?
.is_empty()
{
bail!("branch has no commits - there is nothing to amend to");
@ -1493,9 +1504,11 @@ pub(crate) fn amend(
.context("failed to create commit")?;
// now rebase upstream commits, if needed
let upstream_commits = ctx
.repository()
.l(target_branch.head(), LogUntil::Commit(amend_commit.id()))?;
let upstream_commits = ctx.repository().l(
target_branch.head(),
LogUntil::Commit(amend_commit.id()),
false,
)?;
// if there are no upstream commits, we're done
if upstream_commits.is_empty() {
target_branch.set_stack_head(ctx, commit_oid, None)?;
@ -1575,9 +1588,9 @@ pub(crate) fn squash(ctx: &CommandContext, branch_id: StackId, commit_id: git2::
let vb_state = ctx.project().virtual_branches();
let mut branch = vb_state.get_branch_in_workspace(branch_id)?;
let default_target = vb_state.get_default_target()?;
let branch_commit_oids = ctx
.repository()
.l(branch.head(), LogUntil::Commit(default_target.sha))?;
let branch_commit_oids =
ctx.repository()
.l(branch.head(), LogUntil::Commit(default_target.sha), false)?;
if !branch_commit_oids.contains(&commit_id) {
bail!("commit {commit_id} not in the branch")
@ -1600,7 +1613,7 @@ pub(crate) fn squash(ctx: &CommandContext, branch_id: StackId, commit_id: git2::
|| Ok(vec![]),
|upstream_head| {
ctx.repository()
.l(upstream_head, LogUntil::Commit(default_target.sha))
.l(upstream_head, LogUntil::Commit(default_target.sha), false)
},
)?;
@ -1675,9 +1688,9 @@ pub(crate) fn update_commit_message(
let default_target = vb_state.get_default_target()?;
let mut branch = vb_state.get_branch_in_workspace(branch_id)?;
let branch_commit_oids = ctx
.repository()
.l(branch.head(), LogUntil::Commit(default_target.sha))?;
let branch_commit_oids =
ctx.repository()
.l(branch.head(), LogUntil::Commit(default_target.sha), false)?;
if !branch_commit_oids.contains(&commit_id) {
bail!("commit {commit_id} not in the branch");
@ -1687,7 +1700,7 @@ pub(crate) fn update_commit_message(
|| Ok(vec![]),
|upstream_head| {
ctx.repository()
.l(upstream_head, LogUntil::Commit(default_target.sha))
.l(upstream_head, LogUntil::Commit(default_target.sha), false)
},
)?;

View File

@ -1009,7 +1009,9 @@ fn merge_vbranch_upstream_conflict() -> Result<()> {
);
assert_eq!(branch1.files.len(), 0);
assert_eq!(branch1.commits.len(), 4); // Local commit + Remote commit + Merge commit + Conflicted uncommited changes
assert_eq!(branch1.commits.len(), 3); // Local commit + Remote commit + Merge commit + Conflicted uncommited changes
assert_eq!(branch1.series[0].patches.len(), 3);
assert_eq!(branch1.series[0].upstream_patches.len(), 0);
assert!(!branch1.conflicted);
Ok(())

View File

@ -27,9 +27,9 @@ pub fn cherry_rebase(
from_commit_oid: git2::Oid,
) -> Result<Option<git2::Oid>> {
// get a list of the commits to rebase
let ids_to_rebase = ctx
.repository()
.l(from_commit_oid, LogUntil::Commit(to_commit_oid))?;
let ids_to_rebase =
ctx.repository()
.l(from_commit_oid, LogUntil::Commit(to_commit_oid), false)?;
if ids_to_rebase.is_empty() {
return Ok(None);
@ -458,7 +458,7 @@ mod test {
let commits: Vec<git2::Commit> = test_repository
.repository
.log(commit.id(), LogUntil::End)
.log(commit.id(), LogUntil::End, false)
.unwrap();
assert!(commits.into_iter().all(|commit| !commit.is_conflicted()));
@ -596,7 +596,7 @@ mod test {
let commits: Vec<git2::Commit> = test_repository
.repository
.log(commit.id(), LogUntil::Commit(d.id()))
.log(commit.id(), LogUntil::Commit(d.id()), false)
.unwrap();
assert!(commits.iter().all(|commit| commit.is_conflicted()));

View File

@ -124,7 +124,7 @@ impl RepoActionsExt for CommandContext {
// returns the number of commits between the first oid to the second oid
fn distance(&self, from: git2::Oid, to: git2::Oid) -> Result<u32> {
let oids = self.repository().l(from, LogUntil::Commit(to))?;
let oids = self.repository().l(from, LogUntil::Commit(to), false)?;
Ok(oids.len().try_into()?)
}

View File

@ -34,9 +34,15 @@ pub trait RepositoryExt {
/// gets merged.
fn merge_base_octopussy(&self, ids: &[git2::Oid]) -> Result<git2::Oid>;
fn signatures(&self) -> Result<(git2::Signature, git2::Signature)>;
fn l(&self, from: git2::Oid, to: LogUntil) -> Result<Vec<git2::Oid>>;
fn l(&self, from: git2::Oid, to: LogUntil, include_all_parents: bool)
-> Result<Vec<git2::Oid>>;
fn list_commits(&self, from: git2::Oid, to: git2::Oid) -> Result<Vec<git2::Commit>>;
fn log(&self, from: git2::Oid, to: LogUntil) -> Result<Vec<git2::Commit>>;
fn log(
&self,
from: git2::Oid,
to: LogUntil,
include_all_parents: bool,
) -> Result<Vec<git2::Commit>>;
/// Return `HEAD^{commit}` - ideal for obtaining the integration branch commit in open-workspace mode
/// when it's clear that it's representing the current state.
///
@ -485,10 +491,20 @@ impl RepositoryExt for git2::Repository {
}
// returns a list of commit oids from the first oid to the second oid
fn l(&self, from: git2::Oid, to: LogUntil) -> Result<Vec<git2::Oid>> {
// if `include_all_parents` is true it will include commits from all sides of merge commits,
// otherwise, only the first parent of each commit is considered
fn l(
&self,
from: git2::Oid,
to: LogUntil,
include_all_parents: bool,
) -> Result<Vec<git2::Oid>> {
match to {
LogUntil::Commit(oid) => {
let mut revwalk = self.revwalk().context("failed to create revwalk")?;
if !include_all_parents {
revwalk.simplify_first_parent()?;
}
revwalk
.push(from)
.context(format!("failed to push {}", from))?;
@ -501,6 +517,9 @@ impl RepositoryExt for git2::Repository {
}
LogUntil::Take(n) => {
let mut revwalk = self.revwalk().context("failed to create revwalk")?;
if !include_all_parents {
revwalk.simplify_first_parent()?;
}
revwalk
.push(from)
.context(format!("failed to push {}", from))?;
@ -511,6 +530,9 @@ impl RepositoryExt for git2::Repository {
}
LogUntil::When(cond) => {
let mut revwalk = self.revwalk().context("failed to create revwalk")?;
if !include_all_parents {
revwalk.simplify_first_parent()?;
}
revwalk
.push(from)
.context(format!("failed to push {}", from))?;
@ -529,6 +551,9 @@ impl RepositoryExt for git2::Repository {
}
LogUntil::End => {
let mut revwalk = self.revwalk().context("failed to create revwalk")?;
if !include_all_parents {
revwalk.simplify_first_parent()?;
}
revwalk
.push(from)
.context(format!("failed to push {}", from))?;
@ -542,15 +567,20 @@ impl RepositoryExt for git2::Repository {
fn list_commits(&self, from: git2::Oid, to: git2::Oid) -> Result<Vec<git2::Commit>> {
Ok(self
.l(from, LogUntil::Commit(to))?
.l(from, LogUntil::Commit(to), false)?
.into_iter()
.map(|oid| self.find_commit(oid))
.collect::<Result<Vec<_>, _>>()?)
}
// returns a list of commits from the first oid to the second oid
fn log(&self, from: git2::Oid, to: LogUntil) -> Result<Vec<git2::Commit>> {
self.l(from, to)?
fn log(
&self,
from: git2::Oid,
to: LogUntil,
include_all_parents: bool,
) -> Result<Vec<git2::Commit>> {
self.l(from, to, include_all_parents)?
.into_iter()
.map(|oid| self.find_commit(oid))
.collect::<Result<Vec<_>, _>>()

View File

@ -466,7 +466,7 @@ impl StackExt for Stack {
let head_commit =
commit_by_oid_or_change_id(&head.target, ctx, self.head(), &default_target)?.id();
let local_patches = repo
.log(head_commit, LogUntil::Commit(previous_head))?
.log(head_commit, LogUntil::Commit(previous_head), false)?
.iter()
.rev() // oldest commit first
.map(|c| match c.change_id() {
@ -483,7 +483,7 @@ impl StackExt for Stack {
.find_reference(&head.remote_reference(remote_name)?)?
.peel_to_commit()?;
let merge_base = repo.merge_base(head_commit.id(), default_target.sha)?;
repo.log(head_commit.id(), LogUntil::Commit(merge_base))?
repo.log(head_commit.id(), LogUntil::Commit(merge_base), false)?
.iter()
.rev()
.for_each(|c| {
@ -627,7 +627,7 @@ fn validate_target(
.merge_base(stack_head, default_target.sha)?;
let mut stack_commits = ctx
.repository()
.log(stack_head, LogUntil::Commit(merge_base))?
.log(stack_head, LogUntil::Commit(merge_base), false)?
.iter()
.map(|c| c.id())
.collect_vec();
@ -664,7 +664,7 @@ fn stack_patches(
.merge_base(stack_head, default_target.sha)?;
let mut patches = ctx
.repository()
.log(stack_head, LogUntil::Commit(merge_base))?
.log(stack_head, LogUntil::Commit(merge_base), false)?
.iter()
.map(|c| match c.change_id() {
Some(change_id) => CommitOrChangeId::ChangeId(change_id.to_string()),
@ -744,7 +744,7 @@ fn commit_by_branch_id_and_change_id<'a>(
vec![ctx.repository().find_commit(stack_head)?]
} else {
ctx.repository()
.log(stack_head, LogUntil::Commit(merge_base))?
.log(stack_head, LogUntil::Commit(merge_base), false)?
};
let commit = commits
.iter()

View File

@ -1103,13 +1103,13 @@ fn test_ctx(ctx: &CommandContext) -> Result<TestContext> {
let branch = branches.iter().find(|b| b.name == "virtual").unwrap();
let other_branch = branches.iter().find(|b| b.name != "virtual").unwrap();
let target = handle.get_default_target()?;
let mut branch_commits = ctx
.repository()
.log(branch.head(), LogUntil::Commit(target.sha))?;
let mut branch_commits =
ctx.repository()
.log(branch.head(), LogUntil::Commit(target.sha), false)?;
branch_commits.reverse();
let mut other_commits = ctx
.repository()
.log(other_branch.head(), LogUntil::Commit(target.sha))?;
let mut other_commits =
ctx.repository()
.log(other_branch.head(), LogUntil::Commit(target.sha), false)?;
other_commits.reverse();
Ok(TestContext {
branch: branch.clone(),