patch_sync: Commit features

This adds several important version control features:
* Changes branches on set up.
* Cleans up any changes made, even if upload fails.
* Sends commits for review (enabled via CLI).
* Enables CQ for CrOS.

BUG=b:209493133
TEST=cargo test

Change-Id: I8ab2650aae301c08fd80358162a285e46d44e3e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3379479
Reviewed-by: George Burgess <gbiv@chromium.org>
Commit-Queue: Jordan Abrahams-Whitehead <ajordanr@google.com>
Tested-by: Jordan Abrahams-Whitehead <ajordanr@google.com>

2 files changed, 114 insertions, 28 deletions

diff --git a/llvm_tools/patch_sync/src/main.rs b/llvm_tools/patch_sync/src/main.rs
index f2d70edb..cec523bb 100644
--- a/llvm_tools/patch_sync/src/main.rs
+++ b/llvm_tools/patch_sync/src/main.rs
@@ -2,6 +2,7 @@ mod patch_parsing;
 mod version_control;
 
 use anyhow::{Context, Result};
+use std::borrow::ToOwned;
 use std::path::PathBuf;
 use structopt::StructOpt;
 
@@ -14,8 +15,10 @@ fn main() -> Result<()> {
         } => show_subcmd(cros_checkout_path, android_checkout_path, sync),
         Opt::Transpose {
             cros_checkout_path,
+            cros_reviewers,
             old_cros_ref,
             android_checkout_path,
+            android_reviewers,
             old_android_ref,
             sync,
             verbose,
@@ -23,8 +26,13 @@ fn main() -> Result<()> {
             no_commit,
         } => transpose_subcmd(TransposeOpt {
             cros_checkout_path,
+            cros_reviewers: cros_reviewers.split(',').map(ToOwned::to_owned).collect(),
             old_cros_ref,
             android_checkout_path,
+            android_reviewers: android_reviewers
+                .split(',')
+                .map(ToOwned::to_owned)
+                .collect(),
             old_android_ref,
             sync,
             verbose,
@@ -67,6 +75,8 @@ struct TransposeOpt {
     verbose: bool,
     dry_run: bool,
     no_commit: bool,
+    cros_reviewers: Vec<String>,
+    android_reviewers: Vec<String>,
 }
 
 fn transpose_subcmd(args: TransposeOpt) -> Result<()> {
@@ -128,11 +138,11 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> {
     // Note we want to check if the android patches are empty for CrOS, and
     // vice versa. This is a little counterintuitive.
     if !new_android_patches.is_empty() {
-        ctx.cros_repo_upload()
+        ctx.cros_repo_upload(&args.cros_reviewers)
             .context("uploading chromiumos changes")?;
     }
     if !new_cros_patches.is_empty() {
-        ctx.android_repo_upload()
+        ctx.android_repo_upload(&args.android_reviewers)
             .context("uploading android changes")?;
     }
     Ok(())
@@ -158,6 +168,11 @@ enum Opt {
         #[structopt(long = "cros-checkout", parse(from_os_str))]
         cros_checkout_path: PathBuf,
 
+        /// Emails to send review requests to during Chromium OS upload.
+        /// Comma separated.
+        #[structopt(long = "cros-rev")]
+        cros_reviewers: String,
+
         /// Git ref (e.g. hash) for the ChromiumOS overlay to use as the base.
         #[structopt(long = "overlay-base-ref")]
         old_cros_ref: String,
@@ -166,6 +181,11 @@ enum Opt {
         #[structopt(long = "aosp-checkout", parse(from_os_str))]
         android_checkout_path: PathBuf,
 
+        /// Emails to send review requests to during Android upload.
+        /// Comma separated.
+        #[structopt(long = "aosp-rev")]
+        android_reviewers: String,
+
         /// Git ref (e.g. hash) for the llvm_android repo to use as the base.
         #[structopt(long = "aosp-base-ref")]
         old_android_ref: String,
@@ -179,12 +199,11 @@ enum Opt {
         verbose: bool,
 
         /// Do not change any files. Useful in combination with `--verbose`
-        /// Implies `--no-commit` and `--no-upload`.
+        /// Implies `--no-commit`.
         #[structopt(long)]
         dry_run: bool,
 
-        /// Do not commit any changes made.
-        /// Implies `--no-upload`.
+        /// Do not commit or upload any changes made.
         #[structopt(long)]
         no_commit: bool,
     },
diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs
index 3621a909..a19c16e5 100644
--- a/llvm_tools/patch_sync/src/version_control.rs
+++ b/llvm_tools/patch_sync/src/version_control.rs
@@ -8,6 +8,9 @@ use std::process::{Command, Output};
 const CHROMIUMOS_OVERLAY_REL_PATH: &str = "src/third_party/chromiumos-overlay";
 const ANDROID_LLVM_REL_PATH: &str = "toolchain/llvm_android";
 
+const CROS_MAIN_BRANCH: &str = "main";
+const ANDROID_MAIN_BRANCH: &str = "master"; // nocheck
+
 /// Context struct to keep track of both Chromium OS and Android checkouts.
 #[derive(Debug)]
 pub struct RepoSetupContext {
@@ -20,13 +23,23 @@ pub struct RepoSetupContext {
 impl RepoSetupContext {
     pub fn setup(&self) -> Result<()> {
         if self.sync_before {
+            {
+                let crpp = self.cros_patches_path();
+                let cros_git = crpp.parent().unwrap();
+                git_cd_cmd(cros_git, ["checkout", CROS_MAIN_BRANCH])?;
+            }
+            {
+                let anpp = self.android_patches_path();
+                let android_git = anpp.parent().unwrap();
+                git_cd_cmd(android_git, ["checkout", ANDROID_MAIN_BRANCH])?;
+            }
             repo_cd_cmd(&self.cros_checkout, &["sync", CHROMIUMOS_OVERLAY_REL_PATH])?;
             repo_cd_cmd(&self.android_checkout, &["sync", ANDROID_LLVM_REL_PATH])?;
         }
         Ok(())
     }
 
-    pub fn cros_repo_upload(&self) -> Result<()> {
+    pub fn cros_repo_upload<S: AsRef<str>>(&self, reviewers: &[S]) -> Result<()> {
         let llvm_dir = self
             .cros_checkout
             .join(&CHROMIUMOS_OVERLAY_REL_PATH)
@@ -37,36 +50,48 @@ impl RepoSetupContext {
             llvm_dir.display()
         );
         Self::rev_bump_llvm(&llvm_dir)?;
+        let mut extra_args = vec!["--label=Commit-Queue+1"];
+        for reviewer in reviewers {
+            extra_args.push("--re");
+            extra_args.push(reviewer.as_ref());
+        }
         Self::repo_upload(
             &self.cros_checkout,
+            CROS_MAIN_BRANCH,
             CHROMIUMOS_OVERLAY_REL_PATH,
-            &Self::build_commit_msg("android", "chromiumos", "BUG=None\nTEST=CQ"),
+            &Self::build_commit_msg(
+                "llvm: Synchronize patches from android",
+                "android",
+                "chromiumos",
+                "BUG=None\nTEST=CQ",
+            ),
+            extra_args,
         )
     }
 
-    pub fn android_repo_upload(&self) -> Result<()> {
+    pub fn android_repo_upload<S: AsRef<str>>(&self, reviewers: &[S]) -> Result<()> {
+        let mut extra_args = Vec::new();
+        // TODO(ajordanr): Presubmit ready can only be enabled if we
+        // have the permissions.
+        // extra_args.push("--label=Presubmit-Ready+1");
+        for reviewer in reviewers {
+            extra_args.push("--re");
+            extra_args.push(reviewer.as_ref());
+        }
         Self::repo_upload(
             &self.android_checkout,
+            ANDROID_MAIN_BRANCH,
             ANDROID_LLVM_REL_PATH,
-            &Self::build_commit_msg("chromiumos", "android", "Test: N/A"),
+            &Self::build_commit_msg(
+                "Synchronize patches from chromiumos",
+                "chromiumos",
+                "android",
+                "Test: N/A",
+            ),
+            extra_args,
         )
     }
 
-    fn repo_upload(path: &Path, git_wd: &str, commit_msg: &str) -> Result<()> {
-        // TODO(ajordanr): Need to clean up if there's any failures during upload.
-        let git_path = &path.join(&git_wd);
-        ensure!(
-            git_path.is_dir(),
-            "git_path {} is not a directory",
-            git_path.display()
-        );
-        repo_cd_cmd(path, &["start", "patch_sync_branch", git_wd])?;
-        git_cd_cmd(git_path, &["add", "."])?;
-        git_cd_cmd(git_path, &["commit", "-m", commit_msg])?;
-        repo_cd_cmd(path, &["upload", "-y", "--verify", git_wd])?;
-        Ok(())
-    }
-
     pub fn android_patches_path(&self) -> PathBuf {
         self.android_checkout
             .join(&ANDROID_LLVM_REL_PATH)
@@ -79,6 +104,46 @@ impl RepoSetupContext {
             .join("sys-devel/llvm/files/PATCHES.json")
     }
 
+    fn repo_upload<'a, I: IntoIterator<Item = &'a str>>(
+        path: &Path,
+        base_branch: &str,
+        git_wd: &'a str,
+        commit_msg: &str,
+        extra_flags: I,
+    ) -> Result<()> {
+        let git_path = &path.join(&git_wd);
+        ensure!(
+            git_path.is_dir(),
+            "git_path {} is not a directory",
+            git_path.display()
+        );
+        let branch_name = "__patch_sync_tmp_branch";
+        repo_cd_cmd(path, &["start", branch_name, git_wd])?;
+        let res: Result<()> = {
+            let base_args = ["upload", "--br", branch_name, "-y", "--verify"];
+            let new_args = base_args
+                .iter()
+                .copied()
+                .chain(extra_flags)
+                .chain(["--", git_wd]);
+            git_cd_cmd(git_path, &["add", "."])
+                .and_then(|_| git_cd_cmd(git_path, &["commit", "-m", commit_msg]))
+                .and_then(|_| repo_cd_cmd(path, new_args))
+        };
+        Self::cleanup_branch(git_path, base_branch, branch_name)
+            .with_context(|| format!("cleaning up branch {}", branch_name))?;
+        res
+    }
+
+    /// Clean up the git repo after we're done with it.
+    fn cleanup_branch(git_path: &Path, base_branch: &str, rm_branch: &str) -> Result<()> {
+        git_cd_cmd(git_path, ["restore", "."])?;
+        git_cd_cmd(git_path, ["clean", "-fd"])?;
+        git_cd_cmd(git_path, ["checkout", base_branch])?;
+        git_cd_cmd(git_path, ["branch", "-D", rm_branch])?;
+        Ok(())
+    }
+
     /// Increment LLVM's revision number
     fn rev_bump_llvm(llvm_dir: &Path) -> Result<PathBuf> {
         let ebuild = find_ebuild(llvm_dir)
@@ -146,11 +211,13 @@ impl RepoSetupContext {
     }
 
     /// Create the commit message
-    fn build_commit_msg(from: &str, to: &str, footer: &str) -> String {
+    fn build_commit_msg(subj: &str, from: &str, to: &str, footer: &str) -> String {
         format!(
-            "[patch_sync] Synchronize patches from {}\n\n\
-        Copies new PATCHES.json changes from {} to {}\n\n{}",
-            from, from, to, footer
+            "[patch_sync] {}\n\n\
+Copies new PATCHES.json changes from {} to {}.\n
+For questions about this job, contact chromeos-toolchain@google.com\n\n
+{}",
+            subj, from, to, footer
         )
     }
 }