Skip to content

Commit

Permalink
Adjust integration check to also recognize by matching tree.
Browse files Browse the repository at this point in the history
This is relevant when all commits are equal by tree, but seem changed
due to the added GitButler headers.

For added safety, we also compare by commit message, date and authors,
basically everything that isn't the headers.
  • Loading branch information
Byron committed Aug 30, 2024
1 parent 4cafc90 commit 4d495cb
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 25 deletions.
11 changes: 9 additions & 2 deletions apps/desktop/src/lib/vbranches/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,19 @@ export class DetailedCommit {
conflicted!: boolean;
// Set if a GitButler branch reference pointing to this commit exists. In the format of "refs/remotes/origin/my-branch"
remoteRef?: string | undefined;
// Identifies the remote commit id from which this local commit was copied. The backend figured this out by comparing
// author, commit and message.
copiedFromRemoteId?: string;

prev?: DetailedCommit;
next?: DetailedCommit;

get status(): CommitStatus {
if (this.isIntegrated) return 'integrated';
if (this.isRemote && (!this.relatedTo || this.id === this.relatedTo.id))
if (
(this.isRemote && (!this.relatedTo || this.id === this.relatedTo.id)) ||
(this.copiedFromRemoteId && this.relatedTo && this.copiedFromRemoteId === this.relatedTo.id)
)
return 'localAndRemote';
return 'local';
}
Expand Down Expand Up @@ -240,9 +246,10 @@ export class Commit {

export type AnyCommit = DetailedCommit | Commit;

export function commitCompare(left: AnyCommit, right: AnyCommit): boolean {
export function commitCompare(left: AnyCommit, right: DetailedCommit): boolean {
if (left.id === right.id) return true;
if (left.changeId && right.changeId && left.changeId === right.changeId) return true;
if (right.copiedFromRemoteId && right.copiedFromRemoteId === left.id) return true;
return false;
}

Expand Down
7 changes: 7 additions & 0 deletions crates/gitbutler-branch-actions/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ pub struct VirtualBranchCommit {
pub change_id: Option<String>,
pub is_signed: bool,
pub conflicted: bool,
/// The id of the remote commit from which this one was copied, as identified by
/// having equal author, committer, and commit message.
/// This is used by the frontend similar to the `change_id` to group matching commits.
#[serde(with = "gitbutler_serde::oid_opt")]
pub copied_from_remote_id: Option<git2::Oid>,
pub remote_ref: Option<ReferenceName>,
}

Expand All @@ -45,6 +50,7 @@ pub(crate) fn commit_to_vbranch_commit(
commit: &git2::Commit,
is_integrated: bool,
is_remote: bool,
copied_from_remote_id: Option<git2::Oid>,
) -> Result<VirtualBranchCommit> {
let timestamp = u128::try_from(commit.time().seconds())?;
let message = commit.message_bstr().to_owned();
Expand Down Expand Up @@ -81,6 +87,7 @@ pub(crate) fn commit_to_vbranch_commit(
change_id: commit.change_id(),
is_signed: commit.is_signed(),
conflicted: commit.is_conflicted(),
copied_from_remote_id,
remote_ref,
};

Expand Down
108 changes: 85 additions & 23 deletions crates/gitbutler-branch-actions/src/virtual.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
use std::{
borrow::Cow,
collections::HashMap,
path::{Path, PathBuf},
vec,
};

use crate::{
branch_manager::BranchManagerExt,
commit::{commit_to_vbranch_commit, VirtualBranchCommit},
Expand All @@ -17,7 +10,7 @@ use crate::{
Get, VirtualBranchesExt,
};
use anyhow::{anyhow, bail, Context, Result};
use bstr::ByteSlice;
use bstr::{BString, ByteSlice};
use git2_hooks::HookResult;
use gitbutler_branch::{
dedup, dedup_fmt, reconcile_claims, signature, Branch, BranchId, BranchOwnershipClaims,
Expand All @@ -38,6 +31,13 @@ use gitbutler_repo::{
};
use gitbutler_time::time::now_since_unix_epoch_ms;
use serde::Serialize;
use std::collections::HashSet;
use std::{
borrow::Cow,
collections::HashMap,
path::{Path, PathBuf},
vec,
};
use tracing::instrument;

// this struct is a mapping to the view `Branch` type in Typescript
Expand Down Expand Up @@ -313,19 +313,34 @@ pub fn list_virtual_branches_cached(
))?;

// find upstream commits if we found an upstream reference
let mut pushed_commits = HashMap::new();
if let Some(upstream) = &upstram_branch_commit {
let merge_base =
repo.merge_base(upstream.id(), default_target.sha)
.context(format!(
"failed to find merge base between {} and {}",
upstream.id(),
default_target.sha
))?;
for oid in ctx.l(upstream.id(), LogUntil::Commit(merge_base))? {
pushed_commits.insert(oid, true);
}
}
let (remote_commit_ids, remote_commit_data) = upstram_branch_commit
.as_ref()
.map(
|upstream| -> Result<(HashSet<git2::Oid>, HashMap<CommitData, git2::Oid>)> {
let merge_base =
repo.merge_base(upstream.id(), default_target.sha)
.context(format!(
"failed to find merge base between {} and {}",
upstream.id(),
default_target.sha
))?;
let remote_commit_ids =
HashSet::from_iter(ctx.l(upstream.id(), LogUntil::Commit(merge_base))?);
let remote_commit_data: HashMap<_, _> = remote_commit_ids
.iter()
.copied()
.filter_map(|id| repo.find_commit(id).ok())
.filter_map(|commit| {
CommitData::try_from(&commit)
.ok()
.map(|key| (key, commit.id()))
})
.collect();
Ok((remote_commit_ids, remote_commit_data))
},
)
.transpose()?
.unwrap_or_default();

let mut is_integrated = false;
let mut is_remote = false;
Expand All @@ -346,15 +361,28 @@ pub fn list_virtual_branches_cached(
is_remote = if is_remote {
is_remote
} else {
pushed_commits.contains_key(&commit.id())
// This can only work once we have pushed our commits to the remote.
// Otherwise, even local commits created from a remote commit will look different.
remote_commit_ids.contains(&commit.id())
};

// only check for integration if we haven't already found an integration
if !is_integrated {
is_integrated = check_commit.is_integrated(commit)?
};

commit_to_vbranch_commit(ctx, &branch, commit, is_integrated, is_remote)
let copied_from_remote_id = CommitData::try_from(commit)
.ok()
.and_then(|data| remote_commit_data.get(&data).copied());

commit_to_vbranch_commit(
ctx,
&branch,
commit,
is_integrated,
is_remote,
copied_from_remote_id,
)
})
.collect::<Result<Vec<_>>>()?
};
Expand Down Expand Up @@ -427,6 +455,40 @@ pub fn list_virtual_branches_cached(
Ok((branches, status.skipped_files))
}

/// The commit-data we can use for comparison to see which remote-commit was used to craete
/// a local commit from.
/// Note that trees can't be used for comparison as these are typically rebased.
#[derive(Hash, Eq, PartialEq)]
struct CommitData {
message: BString,
author: gix::actor::Signature,
committer: gix::actor::Signature,
}

impl TryFrom<&git2::Commit<'_>> for CommitData {
type Error = anyhow::Error;

fn try_from(commit: &git2::Commit<'_>) -> std::result::Result<Self, Self::Error> {
Ok(CommitData {
message: commit.message_raw_bytes().into(),
author: git2_signature_to_gix_signature(commit.author()),
committer: git2_signature_to_gix_signature(commit.committer()),
})
}
}

fn git2_signature_to_gix_signature(input: git2::Signature<'_>) -> gix::actor::Signature {
gix::actor::Signature {
name: input.name_bytes().into(),
email: input.email_bytes().into(),
time: gix::date::Time {
seconds: input.when().seconds(),
offset: input.when().offset_minutes() * 60,
sign: input.when().offset_minutes().into(),
},
}
}

fn branches_with_large_files_abridged(mut branches: Vec<VirtualBranch>) -> Vec<VirtualBranch> {
for branch in &mut branches {
for file in &mut branch.files {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,11 @@ mod applied_branch {
assert_eq!(branches[0].files.len(), 1);
assert_eq!(branches[0].commits.len(), 1);
assert!(!branches[0].commits[0].is_remote);
assert!(
branches[0].commits[0].copied_from_remote_id.is_some(),
"it's copied, which displays things differently in the \
UI which knows what remote commit it relates to"
);
assert!(!branches[0].commits[0].is_integrated);
}
}
Expand Down

0 comments on commit 4d495cb

Please sign in to comment.