Skip to content

Commit 81bd7ef

Browse files
committed
Adjust integration check to also recognize by matching tree.
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.
1 parent bdab3e2 commit 81bd7ef

File tree

4 files changed

+106
-25
lines changed

4 files changed

+106
-25
lines changed

apps/desktop/src/lib/vbranches/types.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,19 @@ export class DetailedCommit {
179179
conflicted!: boolean;
180180
// Set if a GitButler branch reference pointing to this commit exists. In the format of "refs/remotes/origin/my-branch"
181181
remoteRef?: string | undefined;
182+
// Identifies the remote commit id from which this local commit was copied. The backend figured this out by comparing
183+
// author, commit and message.
184+
copiedFromRemoteId?: string;
182185

183186
prev?: DetailedCommit;
184187
next?: DetailedCommit;
185188

186189
get status(): CommitStatus {
187190
if (this.isIntegrated) return 'integrated';
188-
if (this.isRemote && (!this.relatedTo || this.id === this.relatedTo.id))
191+
if (
192+
(this.isRemote && (!this.relatedTo || this.id === this.relatedTo.id)) ||
193+
(this.copiedFromRemoteId && this.relatedTo && this.copiedFromRemoteId === this.relatedTo.id)
194+
)
189195
return 'localAndRemote';
190196
return 'local';
191197
}
@@ -240,9 +246,10 @@ export class Commit {
240246

241247
export type AnyCommit = DetailedCommit | Commit;
242248

243-
export function commitCompare(left: AnyCommit, right: AnyCommit): boolean {
249+
export function commitCompare(left: AnyCommit, right: DetailedCommit): boolean {
244250
if (left.id === right.id) return true;
245251
if (left.changeId && right.changeId && left.changeId === right.changeId) return true;
252+
if (right.copiedFromRemoteId && right.copiedFromRemoteId === left.id) return true;
246253
return false;
247254
}
248255

crates/gitbutler-branch-actions/src/commit.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ pub struct VirtualBranchCommit {
3636
pub change_id: Option<String>,
3737
pub is_signed: bool,
3838
pub conflicted: bool,
39+
/// The id of the remote commit from which this one was copied, as identified by
40+
/// having equal author, committer, and commit message.
41+
/// This is used by the frontend similar to the `change_id` to group matching commits.
42+
#[serde(with = "gitbutler_serde::oid_opt")]
43+
pub copied_from_remote_id: Option<git2::Oid>,
3944
pub remote_ref: Option<ReferenceName>,
4045
}
4146

@@ -45,6 +50,7 @@ pub(crate) fn commit_to_vbranch_commit(
4550
commit: &git2::Commit,
4651
is_integrated: bool,
4752
is_remote: bool,
53+
copied_from_remote_id: Option<git2::Oid>,
4854
) -> Result<VirtualBranchCommit> {
4955
let timestamp = u128::try_from(commit.time().seconds())?;
5056
let message = commit.message_bstr().to_owned();
@@ -81,6 +87,7 @@ pub(crate) fn commit_to_vbranch_commit(
8187
change_id: commit.change_id(),
8288
is_signed: commit.is_signed(),
8389
conflicted: commit.is_conflicted(),
90+
copied_from_remote_id,
8491
remote_ref,
8592
};
8693

crates/gitbutler-branch-actions/src/virtual.rs

Lines changed: 85 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
use std::{
2-
borrow::Cow,
3-
collections::HashMap,
4-
path::{Path, PathBuf},
5-
vec,
6-
};
7-
81
use crate::{
92
branch_manager::BranchManagerExt,
103
commit::{commit_to_vbranch_commit, VirtualBranchCommit},
@@ -17,7 +10,7 @@ use crate::{
1710
Get, VirtualBranchesExt,
1811
};
1912
use anyhow::{anyhow, bail, Context, Result};
20-
use bstr::ByteSlice;
13+
use bstr::{BString, ByteSlice};
2114
use git2_hooks::HookResult;
2215
use gitbutler_branch::{
2316
dedup, dedup_fmt, reconcile_claims, signature, Branch, BranchId, BranchOwnershipClaims,
@@ -38,6 +31,13 @@ use gitbutler_repo::{
3831
};
3932
use gitbutler_time::time::now_since_unix_epoch_ms;
4033
use serde::Serialize;
34+
use std::collections::HashSet;
35+
use std::{
36+
borrow::Cow,
37+
collections::HashMap,
38+
path::{Path, PathBuf},
39+
vec,
40+
};
4141
use tracing::instrument;
4242

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

315315
// find upstream commits if we found an upstream reference
316-
let mut pushed_commits = HashMap::new();
317-
if let Some(upstream) = &upstram_branch_commit {
318-
let merge_base =
319-
repo.merge_base(upstream.id(), default_target.sha)
320-
.context(format!(
321-
"failed to find merge base between {} and {}",
322-
upstream.id(),
323-
default_target.sha
324-
))?;
325-
for oid in ctx.l(upstream.id(), LogUntil::Commit(merge_base))? {
326-
pushed_commits.insert(oid, true);
327-
}
328-
}
316+
let (remote_commit_ids, remote_commit_data) = upstram_branch_commit
317+
.as_ref()
318+
.map(
319+
|upstream| -> Result<(HashSet<git2::Oid>, HashMap<CommitData, git2::Oid>)> {
320+
let merge_base =
321+
repo.merge_base(upstream.id(), default_target.sha)
322+
.context(format!(
323+
"failed to find merge base between {} and {}",
324+
upstream.id(),
325+
default_target.sha
326+
))?;
327+
let remote_commit_ids =
328+
HashSet::from_iter(ctx.l(upstream.id(), LogUntil::Commit(merge_base))?);
329+
let remote_commit_data: HashMap<_, _> = remote_commit_ids
330+
.iter()
331+
.copied()
332+
.filter_map(|id| repo.find_commit(id).ok())
333+
.filter_map(|commit| {
334+
CommitData::try_from(&commit)
335+
.ok()
336+
.map(|key| (key, commit.id()))
337+
})
338+
.collect();
339+
Ok((remote_commit_ids, remote_commit_data))
340+
},
341+
)
342+
.transpose()?
343+
.unwrap_or_default();
329344

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

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

357-
commit_to_vbranch_commit(ctx, &branch, commit, is_integrated, is_remote)
374+
let copied_from_remote_id = CommitData::try_from(commit)
375+
.ok()
376+
.and_then(|data| remote_commit_data.get(&data).copied());
377+
378+
commit_to_vbranch_commit(
379+
ctx,
380+
&branch,
381+
commit,
382+
is_integrated,
383+
is_remote,
384+
copied_from_remote_id,
385+
)
358386
})
359387
.collect::<Result<Vec<_>>>()?
360388
};
@@ -427,6 +455,40 @@ pub fn list_virtual_branches_cached(
427455
Ok((branches, status.skipped_files))
428456
}
429457

458+
/// The commit-data we can use for comparison to see which remote-commit was used to craete
459+
/// a local commit from.
460+
/// Note that trees can't be used for comparison as these are typically rebased.
461+
#[derive(Hash, Eq, PartialEq)]
462+
struct CommitData {
463+
message: BString,
464+
author: gix::actor::Signature,
465+
committer: gix::actor::Signature,
466+
}
467+
468+
impl TryFrom<&git2::Commit<'_>> for CommitData {
469+
type Error = anyhow::Error;
470+
471+
fn try_from(commit: &git2::Commit<'_>) -> std::result::Result<Self, Self::Error> {
472+
Ok(CommitData {
473+
message: commit.message_raw_bytes().into(),
474+
author: git2_signature_to_gix_signature(commit.author()),
475+
committer: git2_signature_to_gix_signature(commit.committer()),
476+
})
477+
}
478+
}
479+
480+
fn git2_signature_to_gix_signature(input: git2::Signature<'_>) -> gix::actor::Signature {
481+
gix::actor::Signature {
482+
name: input.name_bytes().into(),
483+
email: input.email_bytes().into(),
484+
time: gix::date::Time {
485+
seconds: input.when().seconds(),
486+
offset: input.when().offset_minutes() * 60,
487+
sign: input.when().offset_minutes().into(),
488+
},
489+
}
490+
}
491+
430492
fn branches_with_large_files_abridged(mut branches: Vec<VirtualBranch>) -> Vec<VirtualBranch> {
431493
for branch in &mut branches {
432494
for file in &mut branch.files {

crates/gitbutler-branch-actions/tests/virtual_branches/update_base_branch.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,11 @@ mod applied_branch {
415415
assert_eq!(branches[0].files.len(), 1);
416416
assert_eq!(branches[0].commits.len(), 1);
417417
assert!(!branches[0].commits[0].is_remote);
418+
assert!(
419+
branches[0].commits[0].copied_from_remote_id.is_some(),
420+
"it's copied, which displays things differently in the \
421+
UI which knows what remote commit it relates to"
422+
);
418423
assert!(!branches[0].commits[0].is_integrated);
419424
}
420425
}

0 commit comments

Comments
 (0)