[snowpatch] [PATCH] main, git: Rework creation of patch branch

[Andrew Donnellan]

When we update our remote tracking branch after the remote branch has been
rebased, using git pull will result in a merge commit being created, which
is not the behaviour we want.

Instead, fetch the remote, and create our new patch application branch
directly, without checking out a remote tracking branch and pulling. This
is a lot cleaner. Add a few helper functions to facilitate this.

This requires a new project config option, base_remote_name, to specify the
name of the remote from which we are fetching base branches. If not
specified, it will default to the same remote to which we are pushing. This
is a breaking change and will require existing users to update their
configuration.

Signed-off-by: Andrew Donnellan <ajd at linux.ibm.com>
---
 docs/configuration.md   |  7 ++++--
 examples/openpower.toml |  2 ++
 src/git.rs              | 50 ++++++++++++++++++++++++++---------------
 src/main.rs             | 39 ++++++++++++++++++--------------
 src/settings.rs         |  1 +
 5 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/docs/configuration.md b/docs/configuration.md
index 4402693cab99..d4c383fefe9e 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -111,6 +111,7 @@ Example:
     # test_all_branches defaults to true
     remote_name = "github"
     remote_uri = "git at github.com:ruscur/linux.git"
+    base_remote_name = "origin"
     push_results = false
 
         [[projects.linuxppc-dev.jobs]]
@@ -132,8 +133,7 @@ Example:
 
 - `repository`: path to local clone of git repository
 
-- `branches`: a list of base branches (as defined by the local git repository)
-  that patches should be tested against
+- `branches`: a list of base branches that patches should be tested against
 
 - `test_all_branches`: if true, each patch will be tested against all base
   branches. If false, a patch will only be tested against the first base branch
@@ -144,6 +144,9 @@ Example:
 
 - `remote_uri`: the URI of the remote
 
+- `base_remote_name`: the name of the remote where base branches are retrieved
+  from (Optional, defaults to value of remote_name)
+
 - `push_results`: whether test results should be pushed to Patchwork for this project
 
 Individual jobs contain the following:
diff --git a/examples/openpower.toml b/examples/openpower.toml
index 59331455fc68..d72d47ad7c69 100644
--- a/examples/openpower.toml
+++ b/examples/openpower.toml
@@ -41,6 +41,7 @@ token = "33333333333333333333333333333333"
     test_all_branches = false
     remote_name = "github"
     remote_uri = "git at github.com:ruscur/skiboot.git"
+    base_remote_name = "origin"
     push_results = false
 
         [[projects.skiboot.jobs]]
@@ -61,6 +62,7 @@ token = "33333333333333333333333333333333"
     # test_all_branches defaults to true
     remote_name = "github"
     remote_uri = "git at github.com:ruscur/linux.git"
+    base_remote_name = "origin"
     push_results = false
 
         [[projects.linuxppc-dev.jobs]]
diff --git a/src/git.rs b/src/git.rs
index ad4bb781d23b..3ac75f2991e5 100644
--- a/src/git.rs
+++ b/src/git.rs
@@ -15,7 +15,9 @@
 //
 
 use git2::build::CheckoutBuilder;
-use git2::{Branch, Commit, Cred, Error, PushOptions, Remote, Repository};
+use git2::{
+    Branch, BranchType, Commit, Cred, Error, FetchOptions, PushOptions, Remote, Repository,
+};
 
 use std::path::Path;
 use std::process::{Command, Output};
@@ -47,25 +49,37 @@ pub fn push_to_remote(
     remote.push(refspecs, Some(&mut opts))
 }
 
-// TODO: rewrite this to use git2-rs, I think it's impossible currently
-pub fn pull(repo: &Repository) -> Result<Output, &'static str> {
-    let workdir = repo.workdir().unwrap(); // TODO: support bare repositories
+pub fn fetch_branch(
+    remote: &mut Remote,
+    branch: &str,
+    opts: Option<&mut FetchOptions>,
+) -> Result<(), Error> {
+    remote.fetch(&[branch], opts, None)
+}
 
-    let output = Command::new("git")
-        .arg("pull") // pull the cool kid's way
-        .current_dir(&workdir) // in the repo's working directory
-        .output() // run synchronously
-        .unwrap(); // TODO
+pub fn create_branch<'a>(
+    repo: &'a Repository,
+    branch_name: &str,
+    base_remote: &Remote,
+    target_ref: &str,
+) -> Result<Branch<'a>, Error> {
+    let base_branch = repo.find_branch(
+        &format!("{}/{}", base_remote.name().unwrap(), target_ref),
+        BranchType::Remote,
+    )?;
+    let commit = base_branch.get().peel_to_commit()?;
+
+    let _ = delete_branch(repo, branch_name);
+    repo.branch(branch_name, &commit, true)
+}
 
-    if output.status.success() {
-        debug!(
-            "Pull: {}",
-            String::from_utf8(output.clone().stdout).unwrap()
-        );
-        Ok(output)
-    } else {
-        Err("Error: couldn't pull changes")
+pub fn delete_branch(repo: &Repository, branch_name: &str) -> Result<(), Error> {
+    let mut branch = repo.find_branch(branch_name, BranchType::Local)?;
+    if branch.is_head() {
+        debug!("Detaching HEAD");
+        repo.set_head_detached(repo.head()?.target().unwrap())?;
     }
+    branch.delete()
 }
 
 pub fn checkout_branch(repo: &Repository, branch: &str) {
@@ -87,7 +101,7 @@ pub fn checkout_branch(repo: &Repository, branch: &str) {
         .output()
         .unwrap();
 
-    repo.set_head(&format!("{}/{}", GIT_REF_BASE, &branch))
+    repo.set_head(&branch)
         .unwrap_or_else(|err| panic!("Couldn't set HEAD: {}", err));
     repo.checkout_head(Some(&mut CheckoutBuilder::new().force()))
         .unwrap_or_else(|err| panic!("Couldn't checkout HEAD: {}", err));
diff --git a/src/main.rs b/src/main.rs
index c3c128faeb6f..ea09b07969ba 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -34,7 +34,7 @@ extern crate base64;
 extern crate serde_json;
 extern crate toml;
 
-use git2::{BranchType, PushOptions, RemoteCallbacks};
+use git2::{FetchOptions, PushOptions, RemoteCallbacks};
 
 use reqwest::{Client, Proxy};
 
@@ -200,31 +200,38 @@ fn test_patch(
     }
     let tag = utils::sanitise_path(path.file_name().unwrap().to_str().unwrap());
     let mut remote = repo.find_remote(&project.remote_name).unwrap();
+    let mut base_remote = repo
+        .find_remote(match &project.base_remote_name {
+            Some(name) => &name,
+            _ => &project.remote_name,
+        })
+        .unwrap();
 
     let mut push_callbacks = RemoteCallbacks::new();
     push_callbacks.credentials(|_, _, _| git::cred_from_settings(&settings.git));
-
     let mut push_opts = PushOptions::new();
     push_opts.remote_callbacks(push_callbacks);
+    let mut fetch_callbacks = RemoteCallbacks::new();
+    fetch_callbacks.credentials(|_, _, _| git::cred_from_settings(&settings.git));
+    let mut fetch_opts = FetchOptions::new();
+    fetch_opts.remote_callbacks(fetch_callbacks);
 
     let mut successfully_applied = false;
     for branch_name in project.branches.clone() {
         let tag = format!("{}_{}", tag, branch_name);
         info!("Configuring local branch for {}.", tag);
-        debug!("Switching to base branch {}...", branch_name);
-        git::checkout_branch(&repo, &branch_name);
 
-        // Make sure we're up to date
-        git::pull(&repo).unwrap();
+        debug!("Updating base branch {}...", branch_name);
+        git::fetch_branch(&mut base_remote, &branch_name, Some(&mut fetch_opts))
+            .unwrap_or_else(|err| panic!("Couldn't fetch branch: {}", err));
 
         debug!("Creating a new branch...");
-        let commit = git::get_latest_commit(&repo);
-        let mut branch = repo.branch(&tag, &commit, true).unwrap();
+        let branch = git::create_branch(&repo, &tag, &base_remote, &branch_name)
+            .unwrap_or_else(|err| panic!("Couldn't create branch: {}", err));
+
         debug!("Switching to branch...");
-        repo.set_head(branch.get().name().unwrap())
-            .unwrap_or_else(|err| panic!("Couldn't set HEAD: {}", err));
-        repo.checkout_head(None)
-            .unwrap_or_else(|err| panic!("Couldn't checkout HEAD: {}", err));
+        git::checkout_branch(&repo, branch.get().name().unwrap());
+        let commit = git::get_latest_commit(&repo);
         debug!(
             "Repo is now at head {}",
             repo.head().unwrap().name().unwrap()
@@ -236,11 +243,9 @@ fn test_patch(
             git::push_to_remote(&mut remote, &branch, false, &mut push_opts).unwrap();
         }
 
-        git::checkout_branch(&repo, &branch_name);
-        // we need to find the branch again since its head has moved
-        branch = repo.find_branch(&tag, BranchType::Local).unwrap();
-        branch.delete().unwrap();
-        debug!("Repo is back to {}", repo.head().unwrap().name().unwrap());
+        git::delete_branch(&repo, &tag).unwrap_or_else(|err| {
+            panic!("Couldn't delete branch: {}", err);
+        });
 
         match output {
             Ok(_) => {
diff --git a/src/settings.rs b/src/settings.rs
index 128f2030e467..40ac67cccf58 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -65,6 +65,7 @@ pub struct Project {
     pub test_all_branches: Option<bool>,
     pub remote_name: String,
     pub remote_uri: String,
+    pub base_remote_name: Option<String>,
     pub jobs: Vec<Job>,
     pub push_results: bool,
     pub category: Option<String>,
-- 
2.20.1