From c5b1bd6c4bb4f3e753c25872b58f1b9a53f3165b Mon Sep 17 00:00:00 2001 From: Jordan R Abrahams Date: Wed, 12 Jan 2022 00:31:47 +0000 Subject: patch_sync: Add better command debugging This commit allows the git commands to print out what the command was that they failed at. This is pretty helpful in debugging what went wrong. We don't capture the repo output ever, so that can remained piped to the terminal. But we need to do some trickery for the git cmd. BUG=b:209493133 TEST=None Change-Id: I389257fef1e3bf394fb4013588df6c78e83b733a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3379478 Reviewed-by: George Burgess Reviewed-by: Michael Benfield Tested-by: Jordan Abrahams-Whitehead Commit-Queue: Jordan Abrahams-Whitehead --- llvm_tools/patch_sync/src/version_control.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'llvm_tools/patch_sync/src/version_control.rs') diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs index 3dc5aae9..3621a909 100644 --- a/llvm_tools/patch_sync/src/version_control.rs +++ b/llvm_tools/patch_sync/src/version_control.rs @@ -179,9 +179,16 @@ where I: IntoIterator, S: AsRef, { - let output = Command::new("git").current_dir(&pwd).args(args).output()?; + let mut command = Command::new("git"); + command.current_dir(&pwd).args(args); + let output = command.output()?; if !output.status.success() { - bail!("git command failed") + bail!( + "git command failed:\n {:?}\nstdout --\n{}\nstderr --\n{}", + command, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); } Ok(output) } @@ -191,9 +198,11 @@ where I: IntoIterator, S: AsRef, { - let status = Command::new("repo").current_dir(&pwd).args(args).status()?; + let mut command = Command::new("repo"); + command.current_dir(&pwd).args(args); + let status = command.status()?; if !status.success() { - bail!("repo command failed") + bail!("repo command failed:\n {:?} \n", command) } Ok(()) } -- cgit v1.2.3 From 500f764595891c07c8de9e6bfaa2b16d8628e561 Mon Sep 17 00:00:00 2001 From: Jordan R Abrahams Date: Wed, 12 Jan 2022 00:29:50 +0000 Subject: 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 Commit-Queue: Jordan Abrahams-Whitehead Tested-by: Jordan Abrahams-Whitehead --- llvm_tools/patch_sync/src/version_control.rs | 113 +++++++++++++++++++++------ 1 file changed, 90 insertions(+), 23 deletions(-) (limited to 'llvm_tools/patch_sync/src/version_control.rs') 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>(&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>(&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>( + 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 { 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 ) } } -- cgit v1.2.3 From b0d6ea52f9ebad2924b04f6a87da28b1256480b1 Mon Sep 17 00:00:00 2001 From: Jordan R Abrahams Date: Wed, 12 Jan 2022 23:38:30 +0000 Subject: patch_sync: Refactor shared main code This commit attempts to remove duplicated code between Android and CrOS, as the complexity between this duplicated code exceeded the scope of the main file. Also adds display_patches code. This is to pretty print what patches are getting applied and to where. Only enabled if --verbose is on. Also does some minor function cleanup in version_control.rs. BUG=b:209493133 TEST=cargo test TEST=patch_sync transpose --dry-run --verbose <...> Change-Id: I63410160ff5159f4c079ac1cc189674fe3fc02a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3379481 Reviewed-by: George Burgess Tested-by: Jordan Abrahams-Whitehead Commit-Queue: Jordan Abrahams-Whitehead --- llvm_tools/patch_sync/src/version_control.rs | 41 ++++++++++++++-------------- 1 file changed, 20 insertions(+), 21 deletions(-) (limited to 'llvm_tools/patch_sync/src/version_control.rs') diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs index a19c16e5..e6d32a58 100644 --- a/llvm_tools/patch_sync/src/version_control.rs +++ b/llvm_tools/patch_sync/src/version_control.rs @@ -92,18 +92,38 @@ impl RepoSetupContext { ) } + /// Get the Android path to the PATCHES.json file pub fn android_patches_path(&self) -> PathBuf { self.android_checkout .join(&ANDROID_LLVM_REL_PATH) .join("patches/PATCHES.json") } + /// Get the Chromium OS path to the PATCHES.json file pub fn cros_patches_path(&self) -> PathBuf { self.cros_checkout .join(&CHROMIUMOS_OVERLAY_REL_PATH) .join("sys-devel/llvm/files/PATCHES.json") } + /// Return the contents of the old PATCHES.json from Chromium OS + pub fn old_cros_patch_contents(&self, hash: &str) -> Result { + Self::old_file_contents( + hash, + &self.cros_checkout.join(CHROMIUMOS_OVERLAY_REL_PATH), + Path::new("sys-devel/llvm/files/PATCHES.json"), + ) + } + + /// Return the contents of the old PATCHES.json from android + pub fn old_android_patch_contents(&self, hash: &str) -> Result { + Self::old_file_contents( + hash, + &self.android_checkout.join(ANDROID_LLVM_REL_PATH), + Path::new("patches/PATCHES.json"), + ) + } + fn repo_upload<'a, I: IntoIterator>( path: &Path, base_branch: &str, @@ -173,28 +193,7 @@ impl RepoSetupContext { Ok(new_path) } - /// Return the contents of the old PATCHES.json from Chromium OS - #[allow(dead_code)] - pub fn old_cros_patch_contents(&self, hash: &str) -> Result { - Self::old_file_contents( - hash, - &self.cros_checkout.join(CHROMIUMOS_OVERLAY_REL_PATH), - Path::new("sys-devel/llvm/files/PATCHES.json"), - ) - } - - /// Return the contents of the old PATCHES.json from android - #[allow(dead_code)] - pub fn old_android_patch_contents(&self, hash: &str) -> Result { - Self::old_file_contents( - hash, - &self.android_checkout.join(ANDROID_LLVM_REL_PATH), - Path::new("patches/PATCHES.json"), - ) - } - /// Return the contents of an old file in git - #[allow(dead_code)] fn old_file_contents(hash: &str, pwd: &Path, file: &Path) -> Result { let git_ref = format!( "{}:{}", -- cgit v1.2.3 From d3fe9cbd37edaa1f6cda95e1866fa3bcf86c7eb2 Mon Sep 17 00:00:00 2001 From: Jordan R Abrahams Date: Wed, 12 Jan 2022 00:51:24 +0000 Subject: patch_sync: Bugfix for ebuild finding Previously, the find_ebuild function could return the original ebuild instead of the llvm symlink. This would lead to incorrect uprevs, depending on how the file system returned the order of the files. Adds a test case to prevent this from happening in the future. BUG=b:209493133 TEST=cargo test Change-Id: Ia47ecee2a9c5b6ce0559e316c5227f70dcf87833 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3379482 Reviewed-by: George Burgess Tested-by: Jordan Abrahams-Whitehead Commit-Queue: Jordan Abrahams-Whitehead --- llvm_tools/patch_sync/src/version_control.rs | 80 ++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 15 deletions(-) (limited to 'llvm_tools/patch_sync/src/version_control.rs') diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs index e6d32a58..b45177f7 100644 --- a/llvm_tools/patch_sync/src/version_control.rs +++ b/llvm_tools/patch_sync/src/version_control.rs @@ -223,20 +223,44 @@ For questions about this job, contact chromeos-toolchain@google.com\n\n /// Return the path of an ebuild located within the given directory. fn find_ebuild(dir: &Path) -> Result { - // TODO(ajordanr): Maybe use OnceCell for this regex? - let ebuild_matcher = Regex::new(r"(-r[0-9]+)?\.ebuild").unwrap(); - for entry in fs::read_dir(dir)? { - let path = entry?.path(); - if let Some(name) = path.file_name() { - if ebuild_matcher.is_match( - name.to_str() - .ok_or_else(|| anyhow!("converting filepath to UTF-8"))?, - ) { - return Ok(path); - } + // The logic here is that we create an iterator over all file paths to ebuilds + // with _pre in the name. Then we sort those ebuilds based on their revision numbers. + // Then we return the highest revisioned one. + + let ebuild_rev_matcher = Regex::new(r"-r([0-9]+)\.ebuild").unwrap(); + // For LLVM ebuilds, we only want to check for ebuilds that have this in their file name. + let per_heuristic = "_pre"; + // Get an iterator over all ebuilds with a _per in the file name. + let ebuild_candidates = fs::read_dir(dir)?.filter_map(|entry| { + let entry = entry.ok()?; + let path = entry.path(); + if path.extension()? != "ebuild" { + // Not an ebuild, ignore. + return None; } - } - bail!("could not find ebuild") + let stem = path.file_stem()?.to_str()?; + if stem.contains(per_heuristic) { + return Some(path); + } + None + }); + let try_parse_ebuild_rev = |path: PathBuf| -> Option<(u64, PathBuf)> { + let name = path.file_name()?; + if let Some(rev_match) = ebuild_rev_matcher.captures(name.to_str()?) { + let rev_str = rev_match.get(1)?; + let rev_num = rev_str.as_str().parse::().ok()?; + return Some((rev_num, path)); + } + // If it doesn't have a revision, then it's revision 0. + Some((0, path)) + }; + let mut sorted_candidates: Vec<_> = + ebuild_candidates.filter_map(try_parse_ebuild_rev).collect(); + sorted_candidates.sort_unstable_by_key(|x| x.0); + let highest_rev_ebuild = sorted_candidates + .pop() + .ok_or_else(|| anyhow!("could not find ebuild"))?; + Ok(highest_rev_ebuild.1) } /// Run a given git command from inside a specified git dir. @@ -294,7 +318,11 @@ mod test { File::create(&ebuild_path).expect("creating test ebuild file"); let new_ebuild_path = RepoSetupContext::rev_bump_llvm(&llvm_dir).expect("rev bumping the ebuild"); - assert!(new_ebuild_path.ends_with("llvm-13.0_pre433403_p20211019-r11.ebuild")); + assert!( + new_ebuild_path.ends_with("llvm-13.0_pre433403_p20211019-r11.ebuild"), + "{}", + new_ebuild_path.display() + ); fs::remove_file(new_ebuild_path).expect("removing renamed ebuild file"); } { @@ -304,9 +332,31 @@ mod test { File::create(&ebuild_path).expect("creating test ebuild file"); let new_ebuild_path = RepoSetupContext::rev_bump_llvm(&llvm_dir).expect("rev bumping the ebuild"); - assert!(new_ebuild_path.ends_with("llvm-13.0_pre433403_p20211019-r1.ebuild")); + assert!( + new_ebuild_path.ends_with("llvm-13.0_pre433403_p20211019-r1.ebuild"), + "{}", + new_ebuild_path.display() + ); fs::remove_file(new_ebuild_path).expect("removing renamed ebuild file"); } + { + // With both + let ebuild_name = "llvm-13.0_pre433403_p20211019.ebuild"; + let ebuild_path = llvm_dir.join(ebuild_name); + File::create(&ebuild_path).expect("creating test ebuild file"); + let ebuild_link_name = "llvm-13.0_pre433403_p20211019-r2.ebuild"; + let ebuild_link_path = llvm_dir.join(ebuild_link_name); + File::create(&ebuild_link_path).expect("creating test ebuild link file"); + let new_ebuild_path = + RepoSetupContext::rev_bump_llvm(&llvm_dir).expect("rev bumping the ebuild"); + assert!( + new_ebuild_path.ends_with("llvm-13.0_pre433403_p20211019-r3.ebuild"), + "{}", + new_ebuild_path.display() + ); + fs::remove_file(new_ebuild_path).expect("removing renamed ebuild link file"); + fs::remove_file(ebuild_path).expect("removing renamed ebuild file"); + } fs::remove_dir(&llvm_dir).expect("removing temp test dir"); } -- cgit v1.2.3 From 8b206a483bcf832f12854567ff0ac9e174aae75a Mon Sep 17 00:00:00 2001 From: Jordan R Abrahams Date: Fri, 21 Jan 2022 19:08:51 +0000 Subject: patch_sync: Conduct unconditional cleanup After any sort of modification, we have to conduct a full cleanup of the workspace. While it's still possible to have a panic cause no cleanup, this is an improvement over the previous work, where cleanup could fail during the transpose stage. Also add a --wip mode to prevent spamming people with emails during testing. BUG=b:209493133 TEST=Running patch_sync transpose locally Change-Id: I01e8a5897ec8eeed8f90c528c567b2ba55613b23 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3407914 Reviewed-by: George Burgess Tested-by: Jordan Abrahams-Whitehead Commit-Queue: Jordan Abrahams-Whitehead --- llvm_tools/patch_sync/src/version_control.rs | 79 ++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 23 deletions(-) (limited to 'llvm_tools/patch_sync/src/version_control.rs') diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs index b45177f7..dc43523e 100644 --- a/llvm_tools/patch_sync/src/version_control.rs +++ b/llvm_tools/patch_sync/src/version_control.rs @@ -10,6 +10,7 @@ const ANDROID_LLVM_REL_PATH: &str = "toolchain/llvm_android"; const CROS_MAIN_BRANCH: &str = "main"; const ANDROID_MAIN_BRANCH: &str = "master"; // nocheck +const WORK_BRANCH_NAME: &str = "__patch_sync_tmp"; /// Context struct to keep track of both Chromium OS and Android checkouts. #[derive(Debug)] @@ -18,6 +19,7 @@ pub struct RepoSetupContext { pub android_checkout: PathBuf, /// Run `repo sync` before doing any comparisons. pub sync_before: bool, + pub wip_mode: bool, } impl RepoSetupContext { @@ -55,9 +57,12 @@ impl RepoSetupContext { extra_args.push("--re"); extra_args.push(reviewer.as_ref()); } + if self.wip_mode { + extra_args.push("--wip"); + extra_args.push("--no-emails"); + } Self::repo_upload( &self.cros_checkout, - CROS_MAIN_BRANCH, CHROMIUMOS_OVERLAY_REL_PATH, &Self::build_commit_msg( "llvm: Synchronize patches from android", @@ -78,9 +83,12 @@ impl RepoSetupContext { extra_args.push("--re"); extra_args.push(reviewer.as_ref()); } + if self.wip_mode { + extra_args.push("--wip"); + extra_args.push("--no-emails"); + } Self::repo_upload( &self.android_checkout, - ANDROID_MAIN_BRANCH, ANDROID_LLVM_REL_PATH, &Self::build_commit_msg( "Synchronize patches from chromiumos", @@ -92,6 +100,30 @@ impl RepoSetupContext { ) } + fn cros_cleanup(&self) -> Result<()> { + let git_path = self.cros_checkout.join(CHROMIUMOS_OVERLAY_REL_PATH); + Self::cleanup_branch(&git_path, CROS_MAIN_BRANCH, WORK_BRANCH_NAME) + .with_context(|| format!("cleaning up branch {}", WORK_BRANCH_NAME))?; + Ok(()) + } + + fn android_cleanup(&self) -> Result<()> { + let git_path = self.android_checkout.join(ANDROID_LLVM_REL_PATH); + Self::cleanup_branch(&git_path, ANDROID_MAIN_BRANCH, WORK_BRANCH_NAME) + .with_context(|| format!("cleaning up branch {}", WORK_BRANCH_NAME))?; + Ok(()) + } + + /// Wrapper around cleanups to ensure both get run, even if errors appear. + pub fn cleanup(&self) { + if let Err(e) = self.cros_cleanup() { + eprintln!("Failed to clean up chromiumos, continuing: {}", e); + } + if let Err(e) = self.android_cleanup() { + eprintln!("Failed to clean up android, continuing: {}", e); + } + } + /// Get the Android path to the PATCHES.json file pub fn android_patches_path(&self) -> PathBuf { self.android_checkout @@ -125,34 +157,31 @@ impl RepoSetupContext { } fn repo_upload<'a, I: IntoIterator>( - path: &Path, - base_branch: &str, - git_wd: &'a str, + checkout_path: &Path, + subproject_git_wd: &'a str, commit_msg: &str, extra_flags: I, ) -> Result<()> { - let git_path = &path.join(&git_wd); + let git_path = &checkout_path.join(&subproject_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 + repo_cd_cmd( + checkout_path, + &["start", WORK_BRANCH_NAME, subproject_git_wd], + )?; + let base_args = ["upload", "--br", WORK_BRANCH_NAME, "-y", "--verify"]; + let new_args = base_args + .iter() + .copied() + .chain(extra_flags) + .chain(["--", subproject_git_wd]); + git_cd_cmd(git_path, &["add", "."]) + .and_then(|_| git_cd_cmd(git_path, &["commit", "-m", commit_msg])) + .and_then(|_| repo_cd_cmd(checkout_path, new_args))?; + Ok(()) } /// Clean up the git repo after we're done with it. @@ -160,7 +189,11 @@ impl RepoSetupContext { 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])?; + // It's acceptable to be able to not delete the branch. This may be + // because the branch does not exist, which is an expected result. + // Since this is a very common case, we won't report any failures related + // to this command failure as it'll pollute the stderr logs. + let _ = git_cd_cmd(git_path, ["branch", "-D", rm_branch]); Ok(()) } -- cgit v1.2.3 From 76d717633c0aa3371a3f9c28fdbf7092af81d9a0 Mon Sep 17 00:00:00 2001 From: Jordan R Abrahams Date: Wed, 9 Feb 2022 20:41:54 +0000 Subject: patch_sync: Toggle CQ for CrOS, Android Currently, patch sync will always run CQ for CrOS, but never for Android. This was because the chrotomation bot didn't have the proper permissions to run the needed Presubmit-Ready checks. Now the bot does in fact have permission to enable the Presubmit check on Android, but some users may not have this permission when running locally. This commit introduces the ability to toggle running the presubmit/CQ checks early through the --disable-cq flags (by default it's assumed that running these checks is desirable). BUG=None TEST=patch_sync transpose --disable-cq <...> TEST=patch_sync transpose <...> Change-Id: I6185b98aa4394ba22f8433541885a450855cfbb4 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3451037 Reviewed-by: George Burgess Commit-Queue: Jordan Abrahams-Whitehead Tested-by: Jordan Abrahams-Whitehead --- llvm_tools/patch_sync/src/version_control.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'llvm_tools/patch_sync/src/version_control.rs') diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs index dc43523e..e07d39d6 100644 --- a/llvm_tools/patch_sync/src/version_control.rs +++ b/llvm_tools/patch_sync/src/version_control.rs @@ -20,6 +20,7 @@ pub struct RepoSetupContext { /// Run `repo sync` before doing any comparisons. pub sync_before: bool, pub wip_mode: bool, + pub enable_cq: bool, } impl RepoSetupContext { @@ -52,7 +53,7 @@ impl RepoSetupContext { llvm_dir.display() ); Self::rev_bump_llvm(&llvm_dir)?; - let mut extra_args = vec!["--label=Commit-Queue+1"]; + let mut extra_args = Vec::new(); for reviewer in reviewers { extra_args.push("--re"); extra_args.push(reviewer.as_ref()); @@ -61,6 +62,9 @@ impl RepoSetupContext { extra_args.push("--wip"); extra_args.push("--no-emails"); } + if self.enable_cq { + extra_args.push("--label=Commit-Queue+1"); + } Self::repo_upload( &self.cros_checkout, CHROMIUMOS_OVERLAY_REL_PATH, @@ -76,9 +80,6 @@ impl RepoSetupContext { pub fn android_repo_upload>(&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()); @@ -87,6 +88,9 @@ impl RepoSetupContext { extra_args.push("--wip"); extra_args.push("--no-emails"); } + if self.enable_cq { + extra_args.push("--label=Presubmit-Ready+1"); + } Self::repo_upload( &self.android_checkout, ANDROID_LLVM_REL_PATH, -- cgit v1.2.3