Implement tunable preservation of remote branches
diff mbox series

Message ID 20191016065305.100695-1-ruscur@russell.cc
State Superseded
Headers show
Series
  • Implement tunable preservation of remote branches
Related show

Commit Message

Russell Currey Oct. 16, 2019, 6:53 a.m. UTC
A new parameter, branch_preserve_policy, allows users to specify whether
they want to preserve all remote branches, just full series or
standalone patches, or none (default, current behaviour).  In addition,
users can specify a separate remote to push to, allowing branches to be
preserved while still deleting them from the main remote used for
testing.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 docs/configuration.md   | 18 ++++++++++++++++--
 examples/openpower.toml |  2 ++
 src/main.rs             | 26 ++++++++++++++++++++++++--
 src/settings.rs         | 27 +++++++++++++++++++++++++++
 4 files changed, 69 insertions(+), 4 deletions(-)

Comments

Andrew Donnellan Oct. 28, 2019, 11:09 p.m. UTC | #1
On 16/10/19 5:53 pm, Russell Currey wrote:
> A new parameter, branch_preserve_policy, allows users to specify whether
> they want to preserve all remote branches, just full series or
> standalone patches, or none (default, current behaviour).  In addition,
> users can specify a separate remote to push to, allowing branches to be
> preserved while still deleting them from the main remote used for
> testing.
> 
> Signed-off-by: Russell Currey <ruscur@russell.cc>

LGTM, can you check the v2 rebase I've just sent?

Patch
diff mbox series

diff --git a/docs/configuration.md b/docs/configuration.md
index 4402693..aba3a84 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -144,7 +144,21 @@  Example:
 
 - `remote_uri`: the URI of the remote
 
-- `push_results`: whether test results should be pushed to Patchwork for this project
+- `push_results`: whether test results should be pushed to Patchwork for this
+  project
+
+- `branch_preserve_policy`: set the policy for how snowpatch will handle
+  branches on git remotes after tests have run.
+
+  "ALL": preserves all branches on the remote.
+  "SERIES": only preserves full series or standalone patches.
+  "NONE": deletes branches on the remote after testing.
+
+  (Optional, defaults to NONE)
+
+- `branch_preserve_remote`: only valid if `branch_preserve_policy` is not NONE.
+  Specify the name of a git remote that will only be used for branch
+  preservation.  If set, branches will be deleted on the main remote. (Optional)
 
 Individual jobs contain the following:
 
@@ -169,4 +183,4 @@  Individual jobs contain the following:
 - `warn_on_fail`: if true, this job will return a warning rather than a failure
   if it fails (Optional, defaults to false)
 
-- Any further parameters will be passed to Jenkins as build parameters
\ No newline at end of file
+- Any further parameters will be passed to Jenkins as build parameters
diff --git a/examples/openpower.toml b/examples/openpower.toml
index 5933145..f67d8f5 100644
--- a/examples/openpower.toml
+++ b/examples/openpower.toml
@@ -42,6 +42,8 @@  token = "33333333333333333333333333333333"
     remote_name = "github"
     remote_uri = "git@github.com:ruscur/skiboot.git"
     push_results = false
+    branch_preserve_policy = "SERIES" # defaults to NONE
+    branch_preserve_remote = "gitlab" # branch to push to, but not delete from
 
         [[projects.skiboot.jobs]]
         job = "skiboot-compile-test-snowpatch"
diff --git a/src/main.rs b/src/main.rs
index 809ff18..d92000c 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -65,7 +65,7 @@  mod jenkins;
 use jenkins::JenkinsBackend;
 
 mod settings;
-use settings::{Config, Job, Project};
+use settings::{BranchPreservePolicy, Config, Job, Project};
 
 mod git;
 
@@ -194,12 +194,16 @@  fn test_patch(
     hefty_tests: bool,
 ) -> Vec<TestResult> {
     let repo = project.get_repo().unwrap();
+    let preserve_policy = project
+        .branch_preserve_policy
+        .unwrap_or(BranchPreservePolicy::None);
     let mut results: Vec<TestResult> = Vec::new();
     if !path.is_file() {
         return results;
     }
     let tag = utils::sanitise_path(path.file_name().unwrap().to_str().unwrap());
     let mut remote = repo.find_remote(&project.remote_name).unwrap();
+    let preserve_remote = &project.branch_preserve_remote;
 
     let mut push_callbacks = RemoteCallbacks::new();
     push_callbacks.credentials(|_, _, _| git::cred_from_settings(&settings.git));
@@ -234,6 +238,18 @@  fn test_patch(
 
         if output.is_ok() {
             git::push_to_remote(&mut remote, &branch, false, &mut push_opts).unwrap();
+            if preserve_remote.is_some()
+                && (preserve_policy == BranchPreservePolicy::All
+                    || (preserve_policy == BranchPreservePolicy::Series && hefty_tests))
+            {
+                git::push_to_remote(
+                    &mut repo.find_remote(preserve_remote.as_ref().unwrap()).unwrap(),
+                    &branch,
+                    false,
+                    &mut push_opts,
+                )
+                .unwrap();
+            }
         }
 
         git::checkout_branch(&repo, &branch_name);
@@ -282,6 +298,7 @@  fn test_patch(
         let project = project.clone();
         let client = client.clone();
         let test_all_branches = project.test_all_branches.unwrap_or(true);
+
         let base = commit.id().to_string();
 
         // We've set up a remote branch, time to kick off tests
@@ -302,7 +319,12 @@  fn test_patch(
         results.append(&mut test.join().unwrap());
 
         // Delete the remote branch now it's not needed any more
-        git::push_to_remote(&mut remote, &branch, true, &mut push_opts).unwrap();
+        if preserve_remote.is_some()
+            || preserve_policy == BranchPreservePolicy::None
+            || (preserve_policy == BranchPreservePolicy::Series && !hefty_tests)
+        {
+            git::push_to_remote(&mut remote, &branch, true, &mut push_opts).unwrap();
+        }
 
         if !test_all_branches {
             break;
diff --git a/src/settings.rs b/src/settings.rs
index 5cfd13b..d373b12 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -29,6 +29,13 @@  use std::io::Read;
 
 // TODO: Give more informative error messages when we fail to parse.
 
+#[derive(Deserialize, Clone, PartialEq, Copy)]
+pub enum BranchPreservePolicy {
+    All,
+    Series,
+    None,
+}
+
 #[derive(Deserialize, Clone)]
 pub struct Git {
     pub user: String,
@@ -67,6 +74,10 @@  pub struct Project {
     pub remote_uri: String,
     pub jobs: Vec<Job>,
     pub push_results: bool,
+    #[serde(default)] // necessary for serde to treat as optional
+    #[serde(deserialize_with = "parse_preserve_policy")]
+    pub branch_preserve_policy: Option<BranchPreservePolicy>,
+    pub branch_preserve_remote: Option<String>,
     pub category: Option<String>,
 }
 
@@ -88,6 +99,22 @@  pub struct Job {
     pub parameters: BTreeMap<String, String>,
 }
 
+fn parse_preserve_policy<'de, D>(deserializer: D) -> Result<Option<BranchPreservePolicy>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let s = String::deserialize(deserializer)?;
+
+    match s.as_ref() {
+        "ALL" => Ok(Some(BranchPreservePolicy::All)),
+        "SERIES" => Ok(Some(BranchPreservePolicy::Series)),
+        "NONE" => Ok(Some(BranchPreservePolicy::None)),
+        _ => Err(serde::de::Error::custom(
+            "branch_preserve_policy not one of ALL, SERIES, NONE",
+        )),
+    }
+}
+
 impl<'de> Deserialize<'de> for Job {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where