diff options
Diffstat (limited to 'llvm_tools/patch_sync')
| -rw-r--r-- | llvm_tools/patch_sync/Cargo.lock | 9 | ||||
| -rw-r--r-- | llvm_tools/patch_sync/Cargo.toml | 3 | ||||
| -rw-r--r-- | llvm_tools/patch_sync/src/android_utils.rs | 62 | ||||
| -rw-r--r-- | llvm_tools/patch_sync/src/main.rs | 253 | ||||
| -rw-r--r-- | llvm_tools/patch_sync/src/patch_parsing.rs | 161 | ||||
| -rw-r--r-- | llvm_tools/patch_sync/src/version_control.rs | 282 |
6 files changed, 655 insertions, 115 deletions
diff --git a/llvm_tools/patch_sync/Cargo.lock b/llvm_tools/patch_sync/Cargo.lock index 63a9fcf8..1ad74a77 100644 --- a/llvm_tools/patch_sync/Cargo.lock +++ b/llvm_tools/patch_sync/Cargo.lock @@ -162,11 +162,12 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" [[package]] name = "patch_sync" -version = "0.1.0" +version = "1.1.0" dependencies = [ "anyhow", "rand", "regex", + "scopeguard", "serde", "serde_json", "sha2", @@ -286,6 +287,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73b4b750c782965c211b42f022f59af1fbceabdd026623714f104152f1ec149f" [[package]] +name = "scopeguard" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" + +[[package]] name = "serde" version = "1.0.132" source = "registry+https://github.com/rust-lang/crates.io-index" diff --git a/llvm_tools/patch_sync/Cargo.toml b/llvm_tools/patch_sync/Cargo.toml index 43082627..ed33d5ca 100644 --- a/llvm_tools/patch_sync/Cargo.toml +++ b/llvm_tools/patch_sync/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "patch_sync" -version = "0.1.0" +version = "1.1.0" authors = ["Jordan R Abrahams-Whitehead <ajordanr@google.com>"] edition = "2018" @@ -15,6 +15,7 @@ serde_json = "1.0" sha2 = "0.9" structopt = "0.3" time = "0.3" +scopeguard = "1.1.0" [dev-dependencies] rand = "0.8" diff --git a/llvm_tools/patch_sync/src/android_utils.rs b/llvm_tools/patch_sync/src/android_utils.rs new file mode 100644 index 00000000..77cb4b8a --- /dev/null +++ b/llvm_tools/patch_sync/src/android_utils.rs @@ -0,0 +1,62 @@ +use std::path::Path; +use std::process::Command; + +use anyhow::{bail, ensure, Result}; + +const LLVM_ANDROID_REL_PATH: &str = "toolchain/llvm_android"; + +/// Return the Android checkout's current llvm version. +/// +/// This uses android_version.get_svn_revision_number, a python function +/// that can't be executed directly. We spawn a Python3 program +/// to run it and get the result from that. +pub fn get_android_llvm_version(android_checkout: &Path) -> Result<String> { + let mut command = new_android_cmd(android_checkout, "python3")?; + command.args([ + "-c", + "import android_version; print(android_version.get_svn_revision_number(), end='')", + ]); + let output = command.output()?; + if !output.status.success() { + bail!( + "could not get android llvm version: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + let out_string = String::from_utf8(output.stdout)?.trim().to_string(); + Ok(out_string) +} + +/// Sort the Android patches using the cherrypick_cl.py Android utility. +/// +/// This assumes that: +/// 1. There exists a python script called cherrypick_cl.py +/// 2. That calling it with the given arguments sorts the PATCHES.json file. +/// 3. Calling it does nothing besides sorting the PATCHES.json file. +/// +/// We aren't doing our own sorting because we shouldn't have to update patch_sync along +/// with cherrypick_cl.py any time they change the __lt__ implementation. +pub fn sort_android_patches(android_checkout: &Path) -> Result<()> { + let mut command = new_android_cmd(android_checkout, "python3")?; + command.args(["cherrypick_cl.py", "--reason", "patch_sync sorting"]); + let output = command.output()?; + if !output.status.success() { + bail!( + "could not sort: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + Ok(()) +} + +fn new_android_cmd(android_checkout: &Path, cmd: &str) -> Result<Command> { + let mut command = Command::new(cmd); + let llvm_android_dir = android_checkout.join(LLVM_ANDROID_REL_PATH); + ensure!( + llvm_android_dir.is_dir(), + "can't make android command; {} is not a directory", + llvm_android_dir.display() + ); + command.current_dir(llvm_android_dir); + Ok(command) +} diff --git a/llvm_tools/patch_sync/src/main.rs b/llvm_tools/patch_sync/src/main.rs index 081ce01a..c244f1c0 100644 --- a/llvm_tools/patch_sync/src/main.rs +++ b/llvm_tools/patch_sync/src/main.rs @@ -1,63 +1,110 @@ +mod android_utils; mod patch_parsing; mod version_control; +use std::borrow::ToOwned; +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + use anyhow::{Context, Result}; -use std::path::PathBuf; use structopt::StructOpt; +use patch_parsing::{filter_patches_by_platform, PatchCollection, PatchDictSchema}; +use version_control::RepoSetupContext; + fn main() -> Result<()> { match Opt::from_args() { Opt::Show { cros_checkout_path, android_checkout_path, sync, - } => show_subcmd(cros_checkout_path, android_checkout_path, sync), + keep_unmerged, + } => show_subcmd(ShowOpt { + cros_checkout_path, + android_checkout_path, + sync, + keep_unmerged, + }), Opt::Transpose { cros_checkout_path, + cros_reviewers, old_cros_ref, android_checkout_path, + android_reviewers, old_android_ref, sync, verbose, dry_run, no_commit, + wip, + disable_cq, } => transpose_subcmd(TransposeOpt { cros_checkout_path, + cros_reviewers: cros_reviewers + .map(|r| r.split(',').map(ToOwned::to_owned).collect()) + .unwrap_or_default(), old_cros_ref, android_checkout_path, + android_reviewers: android_reviewers + .map(|r| r.split(',').map(ToOwned::to_owned).collect()) + .unwrap_or_default(), old_android_ref, sync, verbose, dry_run, no_commit, + wip, + disable_cq, }), } } -fn show_subcmd( +struct ShowOpt { cros_checkout_path: PathBuf, android_checkout_path: PathBuf, + keep_unmerged: bool, sync: bool, -) -> Result<()> { - let ctx = version_control::RepoSetupContext { +} + +fn show_subcmd(args: ShowOpt) -> Result<()> { + let ShowOpt { + cros_checkout_path, + android_checkout_path, + keep_unmerged, + sync, + } = args; + let ctx = RepoSetupContext { cros_checkout: cros_checkout_path, android_checkout: android_checkout_path, sync_before: sync, + wip_mode: true, // Has no effect, as we're not making changes + enable_cq: false, // Has no effect, as we're not uploading anything }; ctx.setup()?; - let cros_patches_path = ctx.cros_patches_path(); - let android_patches_path = ctx.android_patches_path(); - let cur_cros_collection = patch_parsing::PatchCollection::parse_from_file(&cros_patches_path) - .context("could not parse cros PATCHES.json")?; - let cur_android_collection = - patch_parsing::PatchCollection::parse_from_file(&android_patches_path) - .context("could not parse android PATCHES.json")?; + let make_collection = |platform: &str, patches_fp: &Path| -> Result<PatchCollection> { + let parsed_collection = PatchCollection::parse_from_file(patches_fp) + .with_context(|| format!("could not parse {} PATCHES.json", platform))?; + Ok(if keep_unmerged { + parsed_collection + } else { + filter_patches_by_platform(&parsed_collection, platform).map_patches(|p| { + // Need to do this platforms creation as Rust 1.55 cannot use "from". + let mut platforms = BTreeSet::new(); + platforms.insert(platform.to_string()); + PatchDictSchema { + platforms, + ..p.clone() + } + }) + }) + }; + let cur_cros_collection = make_collection("chromiumos", &ctx.cros_patches_path())?; + let cur_android_collection = make_collection("android", &ctx.android_patches_path())?; let merged = cur_cros_collection.union(&cur_android_collection)?; println!("{}", merged.serialize_patches()?); Ok(()) } -#[allow(dead_code)] struct TransposeOpt { cros_checkout_path: PathBuf, old_cros_ref: String, @@ -67,59 +114,147 @@ struct TransposeOpt { verbose: bool, dry_run: bool, no_commit: bool, + cros_reviewers: Vec<String>, + android_reviewers: Vec<String>, + wip: bool, + disable_cq: bool, } fn transpose_subcmd(args: TransposeOpt) -> Result<()> { - let ctx = version_control::RepoSetupContext { + let ctx = RepoSetupContext { cros_checkout: args.cros_checkout_path, android_checkout: args.android_checkout_path, sync_before: args.sync, + wip_mode: args.wip, + enable_cq: !args.disable_cq, }; ctx.setup()?; let cros_patches_path = ctx.cros_patches_path(); let android_patches_path = ctx.android_patches_path(); - // Chromium OS Patches ---------------------------------------------------- - let mut cur_cros_collection = - patch_parsing::PatchCollection::parse_from_file(&cros_patches_path) - .context("parsing cros PATCHES.json")?; - let new_cros_patches: patch_parsing::PatchCollection = { - let cros_old_patches_json = ctx.old_cros_patch_contents(&args.old_cros_ref)?; - let old_cros_collection = patch_parsing::PatchCollection::parse_from_str( - cros_patches_path.parent().unwrap().to_path_buf(), - &cros_old_patches_json, - )?; - cur_cros_collection.subtract(&old_cros_collection)? - }; + // Get new Patches ------------------------------------------------------- + let (cur_cros_collection, new_cros_patches) = patch_parsing::new_patches( + &cros_patches_path, + &ctx.old_cros_patch_contents(&args.old_cros_ref)?, + "chromiumos", + ) + .context("finding new patches for chromiumos")?; + let (cur_android_collection, new_android_patches) = patch_parsing::new_patches( + &android_patches_path, + &ctx.old_android_patch_contents(&args.old_android_ref)?, + "android", + ) + .context("finding new patches for android")?; + + // Have to ignore patches that are already at the destination, even if + // the patches are new. + let new_cros_patches = new_cros_patches.subtract(&cur_android_collection)?; + let new_android_patches = new_android_patches.subtract(&cur_cros_collection)?; - // Android Patches ------------------------------------------------------- - let mut cur_android_collection = - patch_parsing::PatchCollection::parse_from_file(&android_patches_path) - .context("parsing android PATCHES.json")?; - let new_android_patches: patch_parsing::PatchCollection = { - let android_old_patches_json = ctx.old_android_patch_contents(&args.old_android_ref)?; - let old_android_collection = patch_parsing::PatchCollection::parse_from_str( - android_patches_path.parent().unwrap().to_path_buf(), - &android_old_patches_json, - )?; - cur_android_collection.subtract(&old_android_collection)? + // Need to do an extra filtering step for Android, as AOSP doesn't + // want patches outside of the start/end bounds. + let android_llvm_version: u64 = { + let android_llvm_version_str = + android_utils::get_android_llvm_version(&ctx.android_checkout)?; + android_llvm_version_str.parse::<u64>().with_context(|| { + format!( + "converting llvm version to u64: '{}'", + android_llvm_version_str + ) + })? }; + let new_android_patches = new_android_patches.filter_patches(|p| { + match (p.get_start_version(), p.get_end_version()) { + (Some(start), Some(end)) => start <= android_llvm_version && android_llvm_version < end, + (Some(start), None) => start <= android_llvm_version, + (None, Some(end)) => android_llvm_version < end, + (None, None) => true, + } + }); + + if args.verbose { + display_patches("New patches from Chromium OS", &new_cros_patches); + display_patches("New patches from Android", &new_android_patches); + } + + if args.dry_run { + println!("--dry-run specified; skipping modifications"); + return Ok(()); + } + + modify_repos( + &ctx, + args.no_commit, + ModifyOpt { + new_cros_patches, + cur_cros_collection, + cros_reviewers: args.cros_reviewers, + new_android_patches, + cur_android_collection, + android_reviewers: args.android_reviewers, + }, + ) +} +struct ModifyOpt { + new_cros_patches: PatchCollection, + cur_cros_collection: PatchCollection, + cros_reviewers: Vec<String>, + new_android_patches: PatchCollection, + cur_android_collection: PatchCollection, + android_reviewers: Vec<String>, +} + +fn modify_repos(ctx: &RepoSetupContext, no_commit: bool, opt: ModifyOpt) -> Result<()> { + // Cleanup on scope exit. + scopeguard::defer! { + ctx.cleanup(); + } // Transpose Patches ----------------------------------------------------- - new_cros_patches.transpose_write(&mut cur_cros_collection)?; - new_android_patches.transpose_write(&mut cur_android_collection)?; + let mut cur_android_collection = opt.cur_android_collection; + let mut cur_cros_collection = opt.cur_cros_collection; + if !opt.new_cros_patches.is_empty() { + opt.new_cros_patches + .transpose_write(&mut cur_android_collection)?; + } + if !opt.new_android_patches.is_empty() { + opt.new_android_patches + .transpose_write(&mut cur_cros_collection)?; + } - if !args.no_commit { + if no_commit { + println!("--no-commit specified; not committing or uploading"); return Ok(()); } // Commit and upload for review ------------------------------------------ - ctx.cros_repo_upload() - .context("uploading chromiumos changes")?; - ctx.android_repo_upload() - .context("uploading android changes")?; + // Note we want to check if the android patches are empty for CrOS, and + // vice versa. This is a little counterintuitive. + if !opt.new_android_patches.is_empty() { + ctx.cros_repo_upload(&opt.cros_reviewers) + .context("uploading chromiumos changes")?; + } + if !opt.new_cros_patches.is_empty() { + if let Err(e) = android_utils::sort_android_patches(&ctx.android_checkout) { + eprintln!( + "Couldn't sort Android patches; continuing. Caused by: {}", + e + ); + } + ctx.android_repo_upload(&opt.android_reviewers) + .context("uploading android changes")?; + } Ok(()) } +fn display_patches(prelude: &str, collection: &PatchCollection) { + println!("{}", prelude); + if collection.patches.is_empty() { + println!(" [No Patches]"); + return; + } + println!("{}", collection); +} + #[derive(Debug, structopt::StructOpt)] #[structopt(name = "patch_sync", about = "A pipeline for syncing the patch code")] enum Opt { @@ -130,6 +265,12 @@ enum Opt { cros_checkout_path: PathBuf, #[structopt(parse(from_os_str))] android_checkout_path: PathBuf, + + /// Keep a patch's platform field even if it's not merged at that platform. + #[structopt(long)] + keep_unmerged: bool, + + /// Run repo sync before transposing. #[structopt(short, long)] sync: bool, }, @@ -140,6 +281,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: Option<String>, + /// Git ref (e.g. hash) for the ChromiumOS overlay to use as the base. #[structopt(long = "overlay-base-ref")] old_cros_ref: String, @@ -148,6 +294,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: Option<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, @@ -161,13 +312,21 @@ 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, + + /// Upload and send things for review, but mark as WIP and send no + /// emails. + #[structopt(long)] + wip: bool, + + /// Don't run CQ if set. Only has an effect if uploading. + #[structopt(long)] + disable_cq: bool, }, } diff --git a/llvm_tools/patch_sync/src/patch_parsing.rs b/llvm_tools/patch_sync/src/patch_parsing.rs index 733451ae..124f0d6f 100644 --- a/llvm_tools/patch_sync/src/patch_parsing.rs +++ b/llvm_tools/patch_sync/src/patch_parsing.rs @@ -8,13 +8,43 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; /// JSON serde struct. +// FIXME(b/221489531): Remove when we clear out start_version and +// end_version. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PatchDictSchema { - pub rel_patch_path: String, - pub start_version: Option<u64>, + /// [deprecated(since = "1.1", note = "Use version_range")] + #[serde(skip_serializing_if = "Option::is_none")] pub end_version: Option<u64>, - pub platforms: BTreeSet<String>, pub metadata: Option<BTreeMap<String, serde_json::Value>>, + #[serde(default, skip_serializing_if = "BTreeSet::is_empty")] + pub platforms: BTreeSet<String>, + pub rel_patch_path: String, + /// [deprecated(since = "1.1", note = "Use version_range")] + #[serde(skip_serializing_if = "Option::is_none")] + pub start_version: Option<u64>, + pub version_range: Option<VersionRange>, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +pub struct VersionRange { + pub from: Option<u64>, + pub until: Option<u64>, +} + +// FIXME(b/221489531): Remove when we clear out start_version and +// end_version. +impl PatchDictSchema { + pub fn get_start_version(&self) -> Option<u64> { + self.version_range + .map(|x| x.from) + .unwrap_or(self.start_version) + } + + pub fn get_end_version(&self) -> Option<u64> { + self.version_range + .map(|x| x.until) + .unwrap_or(self.end_version) + } } /// Struct to keep track of patches and their relative paths. @@ -44,14 +74,29 @@ impl PatchCollection { }) } - #[allow(dead_code)] + /// Copy this collection with patches filtered by given criterion. + pub fn filter_patches(&self, f: impl FnMut(&PatchDictSchema) -> bool) -> Self { + Self { + patches: self.patches.iter().cloned().filter(f).collect(), + workdir: self.workdir.clone(), + } + } + + /// Map over the patches. + pub fn map_patches(&self, f: impl FnMut(&PatchDictSchema) -> PatchDictSchema) -> Self { + Self { + patches: self.patches.iter().map(f).collect(), + workdir: self.workdir.clone(), + } + } + /// Return true if the collection is tracking any patches. pub fn is_empty(&self) -> bool { self.patches.is_empty() } /// Compute the set-set subtraction, returning a new `PatchCollection` which - /// keeps the minuend's wordir. + /// keeps the minuend's workdir. pub fn subtract(&self, subtrahend: &Self) -> Result<Self> { let mut new_patches = Vec::new(); // This is O(n^2) when it could be much faster, but n is always going to be less @@ -121,6 +166,7 @@ impl PatchCollection { end_version: p.end_version, platforms: new_platforms, metadata: p.metadata.clone(), + version_range: p.version_range, }); // iii. *merged = true; @@ -187,11 +233,77 @@ impl PatchCollection { Ok(std::str::from_utf8(&serialization_buffer)?.to_string()) } + /// Return whether a given patch actually exists on the file system. + pub fn patch_exists(&self, patch: &PatchDictSchema) -> bool { + self.workdir.join(&patch.rel_patch_path).exists() + } + fn hash_from_rel_patch(&self, patch: &PatchDictSchema) -> Result<String> { hash_from_patch_path(&self.workdir.join(&patch.rel_patch_path)) } } +impl std::fmt::Display for PatchCollection { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + for (i, p) in self.patches.iter().enumerate() { + let title = p + .metadata + .as_ref() + .and_then(|x| x.get("title")) + .and_then(serde_json::Value::as_str) + .unwrap_or("[No Title]"); + let path = self.workdir.join(&p.rel_patch_path); + writeln!(f, "* {}", title)?; + if i == self.patches.len() - 1 { + write!(f, " {}", path.display())?; + } else { + writeln!(f, " {}", path.display())?; + } + } + Ok(()) + } +} + +/// Generate a PatchCollection incorporating only the diff between current patches and old patch +/// contents. +pub fn new_patches( + patches_path: &Path, + old_patch_contents: &str, + platform: &str, +) -> Result<(PatchCollection, PatchCollection)> { + let cur_collection = PatchCollection::parse_from_file(patches_path) + .with_context(|| format!("parsing {} PATCHES.json", platform))?; + let cur_collection = filter_patches_by_platform(&cur_collection, platform); + let cur_collection = cur_collection.filter_patches(|p| cur_collection.patch_exists(p)); + let new_patches: PatchCollection = { + let old_collection = PatchCollection::parse_from_str( + patches_path.parent().unwrap().to_path_buf(), + old_patch_contents, + )?; + let old_collection = old_collection.filter_patches(|p| old_collection.patch_exists(p)); + cur_collection.subtract(&old_collection)? + }; + let new_patches = new_patches.map_patches(|p| { + let mut platforms = BTreeSet::new(); + platforms.extend(["android".to_string(), "chromiumos".to_string()]); + PatchDictSchema { + platforms: platforms.union(&p.platforms).cloned().collect(), + ..p.to_owned() + } + }); + Ok((cur_collection, new_patches)) +} + +/// Create a new collection with only the patches that apply to the +/// given platform. +/// +/// If there's no platform listed, the patch should still apply if the patch file exists. +pub fn filter_patches_by_platform(collection: &PatchCollection, platform: &str) -> PatchCollection { + collection.filter_patches(|p| { + p.platforms.contains(platform) || (p.platforms.is_empty() && collection.patch_exists(p)) + }) +} + /// Get the hash from the patch file contents. /// /// Not every patch file actually contains its own hash, @@ -219,7 +331,7 @@ fn hash_from_patch(patch_contents: impl Read) -> Result<String> { } fn hash_from_patch_path(patch: &Path) -> Result<String> { - let f = File::open(patch)?; + let f = File::open(patch).with_context(|| format!("opening patch file {}", patch.display()))?; hash_from_patch(f) } @@ -240,6 +352,7 @@ fn copy_create_parents(from: &Path, to: &Path) -> Result<()> { #[cfg(test)] mod test { + use super::*; /// Test we can extract the hash from patch files. @@ -275,6 +388,10 @@ mod test { rel_patch_path: "a".into(), metadata: None, platforms: BTreeSet::from(["x".into()]), + version_range: Some(VersionRange { + from: Some(0), + until: Some(1), + }), }; let patch2 = PatchDictSchema { rel_patch_path: "b".into(), @@ -310,4 +427,36 @@ mod test { vec!["x", "y"] ); } + + #[test] + fn test_union_empties() { + let patch1 = PatchDictSchema { + start_version: Some(0), + end_version: Some(1), + rel_patch_path: "a".into(), + metadata: None, + platforms: Default::default(), + version_range: Some(VersionRange { + from: Some(0), + until: Some(1), + }), + }; + let collection1 = PatchCollection { + workdir: PathBuf::new(), + patches: vec![patch1.clone()], + }; + let collection2 = PatchCollection { + workdir: PathBuf::new(), + patches: vec![patch1], + }; + let union = collection1 + .union_helper( + &collection2, + |p| Ok(p.rel_patch_path.to_string()), + |p| Ok(p.rel_patch_path.to_string()), + ) + .expect("could not create union"); + assert_eq!(union.patches.len(), 1); + assert_eq!(union.patches[0].platforms.len(), 0); + } } diff --git a/llvm_tools/patch_sync/src/version_control.rs b/llvm_tools/patch_sync/src/version_control.rs index 3dc5aae9..e07d39d6 100644 --- a/llvm_tools/patch_sync/src/version_control.rs +++ b/llvm_tools/patch_sync/src/version_control.rs @@ -8,6 +8,10 @@ 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 +const WORK_BRANCH_NAME: &str = "__patch_sync_tmp"; + /// Context struct to keep track of both Chromium OS and Android checkouts. #[derive(Debug)] pub struct RepoSetupContext { @@ -15,18 +19,30 @@ pub struct RepoSetupContext { pub android_checkout: PathBuf, /// Run `repo sync` before doing any comparisons. pub sync_before: bool, + pub wip_mode: bool, + pub enable_cq: bool, } 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,48 +53,154 @@ impl RepoSetupContext { llvm_dir.display() ); Self::rev_bump_llvm(&llvm_dir)?; + let mut extra_args = Vec::new(); + for reviewer in reviewers { + extra_args.push("--re"); + extra_args.push(reviewer.as_ref()); + } + if self.wip_mode { + 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, - &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(); + for reviewer in reviewers { + extra_args.push("--re"); + extra_args.push(reviewer.as_ref()); + } + if self.wip_mode { + 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, - &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])?; + 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 .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<String> { + 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<String> { + Self::old_file_contents( + hash, + &self.android_checkout.join(ANDROID_LLVM_REL_PATH), + Path::new("patches/PATCHES.json"), + ) + } + + fn repo_upload<'a, I: IntoIterator<Item = &'a str>>( + checkout_path: &Path, + subproject_git_wd: &'a str, + commit_msg: &str, + extra_flags: I, + ) -> Result<()> { + let git_path = &checkout_path.join(&subproject_git_wd); + ensure!( + git_path.is_dir(), + "git_path {} is not a directory", + git_path.display() + ); + 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. + 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])?; + // 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(()) + } + /// Increment LLVM's revision number fn rev_bump_llvm(llvm_dir: &Path) -> Result<PathBuf> { let ebuild = find_ebuild(llvm_dir) @@ -108,28 +230,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<String> { - 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<String> { - 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<String> { let git_ref = format!( "{}:{}", @@ -146,31 +247,57 @@ 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 ) } } /// Return the path of an ebuild located within the given directory. fn find_ebuild(dir: &Path) -> Result<PathBuf> { - // 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::<u64>().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. @@ -179,9 +306,16 @@ where I: IntoIterator<Item = S>, S: AsRef<OsStr>, { - 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 +325,11 @@ where I: IntoIterator<Item = S>, S: AsRef<OsStr>, { - 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(()) } @@ -219,7 +355,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"); } { @@ -229,9 +369,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"); } |