Merge pull request #2488 from gitbutlerapp/set-base-branch-go-back-to-integration

Set base branch go back to integration

gitbutler-app/src/git/repository.rs

@@ -101,6 +101,12 @@ impl Repository {
             .map_err(Into::into)
     }
 
+    pub fn is_descendant_of(&self, a: Oid, b: Oid) -> Result<bool> {
+        self.0
+            .graph_descendant_of(a.into(), b.into())
+            .map_err(Into::into)
+    }
+
     pub fn merge_base(&self, one: Oid, two: Oid) -> Result<Oid> {
         self.0
             .merge_base(one.into(), two.into())
@@ -389,6 +395,12 @@ impl Repository {
         self.0.set_head(&refname.to_string()).map_err(Into::into)
     }
 
+    pub fn set_head_detached(&self, commitish: Oid) -> Result<()> {
+        self.0
+            .set_head_detached(commitish.into())
+            .map_err(Into::into)
+    }
+
     pub fn reference(
         &self,
         name: &Refname,

gitbutler-app/src/virtual_branches/base.rs

@@ -67,22 +67,17 @@ pub fn set_base_branch(
         Err(error) => Err(errors::SetBaseBranchError::Other(error.into())),
     }?;
 
-    let remote_name = repo
+    let remote = repo
-        .branch_remote_name(target_branch.refname().unwrap())
+        .find_remote(target_branch_ref.remote())
         .context(format!(
-            "failed to get remote name for branch {}",
+            "failed to find remote for branch {}",
-            target_branch.name().unwrap()
-        ))?;
-    let remote = repo.find_remote(&remote_name).context(format!(
-        "failed to find remote {} for branch {}",
-        remote_name,
             target_branch.name().unwrap()
         ))?;
     let remote_url = remote
         .url()
         .context(format!(
-            "failed to get remote url for remote {}",
+            "failed to get remote url for {}",
-            remote_name
+            target_branch_ref.remote()
         ))?
         .unwrap();
 
@@ -91,23 +86,35 @@ pub fn set_base_branch(
         target_branch.name().unwrap()
     ))?;
 
-    let head_ref = repo.head().context("Failed to get HEAD reference")?;
+    let current_head = repo.head().context("Failed to get HEAD reference")?;
-    let head_name: git::Refname = head_ref
+    let current_head_commit = current_head
-        .name()
-        .context("Failed to get HEAD reference name")?;
-    let head_commit = head_ref
         .peel_to_commit()
         .context("Failed to peel HEAD reference to commit")?;
 
     // calculate the commit as the merge-base between HEAD in project_repository and this target commit
     let commit_oid = repo
-        .merge_base(head_commit.id(), target_branch_head.id())
+        .merge_base(current_head_commit.id(), target_branch_head.id())
         .context(format!(
             "Failed to calculate merge base between {} and {}",
-            head_commit.id(),
+            current_head_commit.id(),
             target_branch_head.id()
         ))?;
 
+    // if default target was already set, and the new target is a descendant of the current head, then we want to
+    // keep the current target to avoid unnecessary rebases
+    let commit_oid = if let Some(current_target) = gb_repository.default_target()? {
+        if repo
+            .is_descendant_of(current_target.sha, commit_oid)
+            .context("failed to check if target branch is descendant of current head")?
+        {
+            current_target.sha
+        } else {
+            commit_oid
+        }
+    } else {
+        commit_oid
+    };
+
     let target = target::Target {
         branch: target_branch_ref.clone(),
         remote_url: remote_url.to_string(),
@@ -118,6 +125,9 @@ pub fn set_base_branch(
         target::Writer::new(gb_repository).context("failed to create target writer")?;
     target_writer.write_default(&target)?;
 
+    let head_name: git::Refname = current_head
+        .name()
+        .context("Failed to get HEAD reference name")?;
     if !head_name
         .to_string()
         .eq(&GITBUTLER_INTEGRATION_REFERENCE.to_string())
@@ -125,9 +135,8 @@ pub fn set_base_branch(
         // if there are any commits on the head branch or uncommitted changes in the working directory, we need to
         // put them into a virtual branch
 
-        let wd_diff = diff::workdir(repo, &head_commit.id())?;
+        let wd_diff = diff::workdir(repo, &current_head_commit.id())?;
-
+        if !wd_diff.is_empty() || current_head_commit.id() != target.sha {
-        if !wd_diff.is_empty() || head_commit.id() != commit_oid {
             let hunks_by_filepath =
                 super::virtual_hunks_by_filepath(&project_repository.project().path, &wd_diff);
 
@@ -183,10 +192,10 @@ pub fn set_base_branch(
                 upstream_head,
                 created_timestamp_ms: now_ms,
                 updated_timestamp_ms: now_ms,
-                head: head_commit.id(),
+                head: current_head_commit.id(),
                 tree: super::write_tree_onto_commit(
                     project_repository,
-                    head_commit.id(),
+                    current_head_commit.id(),
                     &wd_diff,
                 )?,
                 ownership,

gitbutler-app/tests/common.rs

@@ -85,7 +85,7 @@ impl TestProject {
     }
 
     /// git add -A
-    /// git reset --hard
+    /// git reset --hard <oid>
     pub fn reset_hard(&self, oid: Option<git::Oid>) {
         let mut index = self.local_repository.index().expect("failed to get index");
         index
@@ -93,14 +93,14 @@ impl TestProject {
             .expect("failed to add all");
         index.write().expect("failed to write index");
 
-        let commit = oid.map_or(
+        let head = self.local_repository.head().unwrap();
-            self.local_repository
+        let commit = oid.map_or(head.peel_to_commit().unwrap(), |oid| {
-                .head()
+            self.local_repository.find_commit(oid).unwrap()
-                .unwrap()
+        });
-                .peel_to_commit()
+
-                .unwrap(),
+        let head_ref = head.name().unwrap();
-            |oid| self.local_repository.find_commit(oid).unwrap(),
+        let head_ref = self.local_repository.find_reference(&head_ref).unwrap();
-        );
+
         self.local_repository
             .reset(&commit, git2::ResetType::Hard, None)
             .unwrap();
@@ -231,6 +231,18 @@ impl TestProject {
         self.local_repository.find_commit(oid)
     }
 
+    pub fn checkout_commit(&self, commit_oid: git::Oid) {
+        let commit = self.local_repository.find_commit(commit_oid).unwrap();
+        let commit_tree = commit.tree().unwrap();
+
+        self.local_repository.set_head_detached(commit_oid).unwrap();
+        self.local_repository
+            .checkout_tree(&commit_tree)
+            .force()
+            .checkout()
+            .unwrap();
+    }
+
     pub fn checkout(&self, branch: &git::LocalRefname) {
         let branch: git::Refname = branch.into();
         let tree = match self.local_repository.find_branch(&branch) {

gitbutler-app/tests/virtual_branches.rs

@@ -692,6 +692,8 @@ mod delete_virtual_branch {
 mod set_base_branch {
     use super::*;
 
+    use pretty_assertions::assert_eq;
+
     #[tokio::test]
     async fn success() {
         let Test {
@@ -728,6 +730,44 @@ mod set_base_branch {
             ));
         }
     }
+
+    #[tokio::test]
+    async fn go_back_to_integration() {
+        let Test {
+            repository,
+            project_id,
+            controller,
+            ..
+        } = Test::default();
+
+        std::fs::write(repository.path().join("file.txt"), "one").unwrap();
+        let oid_one = repository.commit_all("one");
+        std::fs::write(repository.path().join("file.txt"), "two").unwrap();
+        repository.commit_all("two");
+        repository.push();
+
+        println!("{}", repository.path().display());
+
+        let base = controller
+            .set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
+            .await
+            .unwrap();
+
+        let branches = controller.list_virtual_branches(&project_id).await.unwrap();
+        assert!(branches.is_empty());
+
+        repository.checkout_commit(oid_one);
+
+        let base_two = controller
+            .set_base_branch(&project_id, &"refs/remotes/origin/master".parse().unwrap())
+            .await
+            .unwrap();
+
+        let branches = controller.list_virtual_branches(&project_id).await.unwrap();
+        assert_eq!(branches.len(), 1);
+
+        assert_eq!(base_two, base);
+    }
 }
 
 mod unapply {