Skip to content

Commit

Permalink
Fix bug in finding upstream commits
Browse files Browse the repository at this point in the history
We should be using the branch upstream instead of push remote name.
  • Loading branch information
mtsgrd committed Nov 20, 2024
1 parent 171c21a commit 2e263ab
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ impl BranchManager<'_> {
vb_state.find_by_source_refname_where_not_in_workspace(target)
{
branch.upstream_head = upstream_branch.is_some().then_some(head_commit.id());
branch.upstream = upstream_branch;
branch.upstream = upstream_branch; // Used as remote when listing commits.
branch.ownership = ownership;
branch.order = order;
branch.selected_for_changes = selected_for_changes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn integrate_upstream_commits_for_series(
.iter()
.find(|branch| branch.name == series_name)
.ok_or(anyhow!("Series not found"))?;
let upstream_reference = subject_branch.remote_reference(remote.as_str())?;
let upstream_reference = subject_branch.remote_reference(remote.as_str());
let remote_head = repo.find_reference(&upstream_reference)?.peel_to_commit()?;

let series_head = subject_branch.head_oid(&ctx.to_stack_context()?, &stack)?;
Expand All @@ -59,7 +59,7 @@ pub fn integrate_upstream_commits_for_series(
branch_tree: stack.tree,
branch_name: &subject_branch.name,
remote_head: remote_head.id(),
remote_branch_name: &subject_branch.remote_reference(&remote)?,
remote_branch_name: &subject_branch.remote_reference(&remote),
strategy,
};

Expand Down
2 changes: 1 addition & 1 deletion crates/gitbutler-branch-actions/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ fn stack_branch_to_api_branch(
// anyhow::bail!("Lets pretend this is a real error");
let remote = default_target.push_remote_name();
let upstream_reference = if stack_branch.pushed(remote.as_str(), repository)? {
stack_branch.remote_reference(remote.as_str()).ok()
Some(stack_branch.remote_reference(remote.as_str()))
} else {
None
};
Expand Down
17 changes: 8 additions & 9 deletions crates/gitbutler-stack/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ pub struct Stack {
/// If set, this means this virtual branch was originally created from `Some(branch)`.
/// It can be *any* branch.
pub source_refname: Option<Refname>,
/// The local tracking branch, holding the state of the remote.
/// Upstream tracking branch reference, added when creating a stack from a branch.
/// Used e.g. when listing commits from a fork.
pub upstream: Option<RemoteRefname>,
// upstream_head is the last commit on we've pushed to the upstream branch
#[serde(with = "gitbutler_serde::oid_opt", default)]
Expand Down Expand Up @@ -556,8 +557,7 @@ impl Stack {
.head;
let remote_name = branch_state(ctx).get_default_target()?.push_remote_name();
let upstream_refname =
RemoteRefname::from_str(&reference.remote_reference(remote_name.as_str())?)
.context("Failed to parse the remote reference for branch")?;
RemoteRefname::from_str(&reference.remote_reference(remote_name.as_str()))?;
Ok(PushDetails {
head: commit.id(),
remote_refname: upstream_refname,
Expand Down Expand Up @@ -925,13 +925,12 @@ fn local_reference_exists(repository: &gix::Repository, name: &str) -> Result<bo
fn remote_reference_exists(
repository: &gix::Repository,
state: &VirtualBranchesHandle,
reference: &StackBranch,
branch: &StackBranch,
) -> Result<bool> {
Ok(reference
.remote_reference(state.get_default_target()?.push_remote_name().as_str())
.and_then(|reference| local_reference_exists(repository, &reference))
.ok()
.unwrap_or(false))
local_reference_exists(
repository,
&branch.remote_reference(state.get_default_target()?.push_remote_name().as_str()),
)
}

#[cfg(test)]
Expand Down
26 changes: 17 additions & 9 deletions crates/gitbutler-stack/src/stack_branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,15 @@ impl StackBranch {
Ok(head_commit)
}
/// Returns a fully qualified reference with the supplied remote e.g. `refs/remotes/origin/base-branch-improvements`
pub fn remote_reference(&self, remote: &str) -> Result<String> {
Ok(format!("refs/remotes/{}/{}", remote, self.name))
pub fn remote_reference(&self, remote: &str) -> String {
format!("refs/remotes/{}/{}", remote, self.name)
}

/// Returns `true` if the reference is pushed to the provided remote
pub fn pushed(&self, remote: &str, repository: &git2::Repository) -> Result<bool> {
let remote_ref = self.remote_reference(remote)?; // todo: this should probably just return false
Ok(repository.find_reference(&remote_ref).is_ok())
Ok(repository
.find_reference(&self.remote_reference(remote))
.is_ok())
}

/// Returns the commits that are part of the branch.
Expand Down Expand Up @@ -133,13 +134,20 @@ impl StackBranch {

let default_target = stack_context.target();
let mut remote_patches: Vec<Commit<'_>> = vec![];
let remote_name = default_target.push_remote_name();
if self.pushed(&remote_name, repository).unwrap_or_default() {
let head_commit = repository
.find_reference(&self.remote_reference(&remote_name)?)?

// Use remote from upstream if available, otherwise default to push remote.
let remote = stack
.upstream
.clone()
.map(|ref_name| ref_name.remote().to_owned())
.unwrap_or(default_target.push_remote_name());

if self.pushed(&remote, repository).unwrap_or_default() {
let upstream_head = repository
.find_reference(self.remote_reference(&remote).as_str())?
.peel_to_commit()?;
repository
.log(head_commit.id(), LogUntil::Commit(previous_head), false)?
.log(upstream_head.id(), LogUntil::Commit(previous_head), false)?
.into_iter()
.rev()
.for_each(|c| {
Expand Down

0 comments on commit 2e263ab

Please sign in to comment.