diff options
-rw-r--r-- | llvm_tools/patch_sync/src/main.rs | 65 | ||||
-rw-r--r-- | llvm_tools/patch_sync/src/patch_parsing.rs | 166 |
2 files changed, 214 insertions, 17 deletions
diff --git a/llvm_tools/patch_sync/src/main.rs b/llvm_tools/patch_sync/src/main.rs index 2141721a..c3c9a61b 100644 --- a/llvm_tools/patch_sync/src/main.rs +++ b/llvm_tools/patch_sync/src/main.rs @@ -13,7 +13,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use structopt::StructOpt; -use patch_parsing::{filter_patches_by_platform, PatchCollection, PatchDictSchema}; +use patch_parsing::{filter_patches_by_platform, PatchCollection, PatchDictSchema, VersionRange}; use version_control::RepoSetupContext; fn main() -> Result<()> { @@ -137,13 +137,21 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> { let android_patches_path = ctx.android_patches_path(); // Get new Patches ------------------------------------------------------- - let (cur_cros_collection, new_cros_patches) = patch_parsing::new_patches( + let patch_parsing::PatchTemporalDiff { + cur_collection: cur_cros_collection, + new_patches: new_cros_patches, + version_updates: cros_version_updates, + } = 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( + let patch_parsing::PatchTemporalDiff { + cur_collection: cur_android_collection, + new_patches: new_android_patches, + version_updates: android_version_updates, + } = patch_parsing::new_patches( &android_patches_path, &ctx.old_android_patch_contents(&args.old_android_ref)?, "android", @@ -176,9 +184,17 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> { } }); + // Need to filter version updates to only existing patches to the other platform. + let cros_version_updates = + filter_version_changes(cros_version_updates, &cur_android_collection); + let android_version_updates = + filter_version_changes(android_version_updates, &cur_cros_collection); + if args.verbose { display_patches("New patches from ChromiumOS", &new_cros_patches); + display_version_updates("Version updates from ChromiumOS", &cros_version_updates); display_patches("New patches from Android", &new_android_patches); + display_version_updates("Version updates from Android", &android_version_updates); } if args.dry_run { @@ -192,9 +208,11 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> { ModifyOpt { new_cros_patches, cur_cros_collection, + cros_version_updates, cros_reviewers: args.cros_reviewers, new_android_patches, cur_android_collection, + android_version_updates, android_reviewers: args.android_reviewers, }, ) @@ -203,9 +221,11 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> { struct ModifyOpt { new_cros_patches: PatchCollection, cur_cros_collection: PatchCollection, + cros_version_updates: Vec<(String, Option<VersionRange>)>, cros_reviewers: Vec<String>, new_android_patches: PatchCollection, cur_android_collection: PatchCollection, + android_version_updates: Vec<(String, Option<VersionRange>)>, android_reviewers: Vec<String>, } @@ -217,11 +237,16 @@ fn modify_repos(ctx: &RepoSetupContext, no_commit: bool, opt: ModifyOpt) -> Resu // Transpose Patches ----------------------------------------------------- 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() { + // Apply any version ranges and new patches, then write out. + if !opt.new_cros_patches.is_empty() || !opt.cros_version_updates.is_empty() { + cur_android_collection = + cur_android_collection.update_version_ranges(&opt.cros_version_updates); opt.new_cros_patches .transpose_write(&mut cur_android_collection)?; } - if !opt.new_android_patches.is_empty() { + if !opt.new_android_patches.is_empty() || !opt.android_version_updates.is_empty() { + cur_cros_collection = + cur_cros_collection.update_version_ranges(&opt.android_version_updates); opt.new_android_patches .transpose_write(&mut cur_cros_collection)?; } @@ -250,6 +275,25 @@ fn modify_repos(ctx: &RepoSetupContext, no_commit: bool, opt: ModifyOpt) -> Resu Ok(()) } +/// Filter version changes that can't apply to a given collection. +fn filter_version_changes<T>( + version_updates: T, + other_platform_collection: &PatchCollection, +) -> Vec<(String, Option<VersionRange>)> +where + T: IntoIterator<Item = (String, Option<VersionRange>)>, +{ + version_updates + .into_iter() + .filter(|(rel_patch_path, _)| { + other_platform_collection + .patches + .iter() + .any(|p| &p.rel_patch_path == rel_patch_path) + }) + .collect() +} + fn display_patches(prelude: &str, collection: &PatchCollection) { println!("{}", prelude); if collection.patches.is_empty() { @@ -259,6 +303,17 @@ fn display_patches(prelude: &str, collection: &PatchCollection) { println!("{}", collection); } +fn display_version_updates(prelude: &str, version_updates: &[(String, Option<VersionRange>)]) { + println!("{}", prelude); + if version_updates.is_empty() { + println!(" [No Version Changes]"); + return; + } + for (rel_patch_path, _) in version_updates { + println!("* {}", rel_patch_path); + } +} + #[derive(Debug, structopt::StructOpt)] #[structopt(name = "patch_sync", about = "A pipeline for syncing the patch code")] enum Opt { diff --git a/llvm_tools/patch_sync/src/patch_parsing.rs b/llvm_tools/patch_sync/src/patch_parsing.rs index 2f5d6d69..0f4cb741 100644 --- a/llvm_tools/patch_sync/src/patch_parsing.rs +++ b/llvm_tools/patch_sync/src/patch_parsing.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; /// JSON serde struct. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct PatchDictSchema { pub metadata: Option<BTreeMap<String, serde_json::Value>>, #[serde(default, skip_serializing_if = "BTreeSet::is_empty")] @@ -21,7 +21,7 @@ pub struct PatchDictSchema { pub version_range: Option<VersionRange>, } -#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct VersionRange { pub from: Option<u64>, pub until: Option<u64>, @@ -122,6 +122,61 @@ impl PatchCollection { ) } + /// Vec of every PatchDictSchema with differing + /// version ranges but the same rel_patch_paths. + fn version_range_diffs(&self, other: &Self) -> Vec<(String, Option<VersionRange>)> { + let other_map: BTreeMap<_, _> = other + .patches + .iter() + .map(|p| (p.rel_patch_path.clone(), p)) + .collect(); + self.patches + .iter() + .filter_map(|ours| match other_map.get(&ours.rel_patch_path) { + Some(theirs) => { + if ours.get_from_version() != theirs.get_from_version() + || ours.get_until_version() != theirs.get_until_version() + { + Some((ours.rel_patch_path.clone(), ours.version_range)) + } else { + None + } + } + _ => None, + }) + .collect() + } + + /// Given a vector of tuples with (rel_patch_path, Option<VersionRange>), replace + /// all version ranges in this collection with a matching one in the new_versions parameter. + pub fn update_version_ranges(&self, new_versions: &[(String, Option<VersionRange>)]) -> Self { + // new_versions should be really tiny (len() <= 2 for the most part), so + // the overhead of O(1) lookups is not worth it. + let get_updated_version = |rel_patch_path: &str| -> Option<Option<VersionRange>> { + // The first Option indicates whether we are updating it at all. + // The second Option indicates we can update it with None. + new_versions + .iter() + .find(|i| i.0 == rel_patch_path) + .map(|x| x.1) + }; + let cloned_patches = self + .patches + .iter() + .map(|p| match get_updated_version(&p.rel_patch_path) { + Some(version_range) => PatchDictSchema { + version_range, + ..p.clone() + }, + _ => p.clone(), + }) + .collect(); + Self { + workdir: self.workdir.clone(), + patches: cloned_patches, + } + } + fn union_helper( &self, other: &Self, @@ -255,25 +310,38 @@ impl std::fmt::Display for PatchCollection { } } +/// Represents information which changed between now and an old version of a PATCHES.json file. +pub struct PatchTemporalDiff { + pub cur_collection: PatchCollection, + pub new_patches: PatchCollection, + // Store version_updates as a vec, not a map, as it's likely to be very small (<=2), + // and the overhead of using a O(1) look up structure isn't worth it. + pub version_updates: Vec<(String, Option<VersionRange>)>, +} + /// 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)> { +) -> Result<PatchTemporalDiff> { + // Set up the current patch collection. 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)? - }; + + // Set up the old patch collection. + 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)); + + // Set up the differential values + let version_updates = cur_collection.version_range_diffs(&old_collection); + let new_patches: PatchCollection = 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()]); @@ -282,7 +350,11 @@ pub fn new_patches( ..p.to_owned() } }); - Ok((cur_collection, new_patches)) + Ok(PatchTemporalDiff { + cur_collection, + new_patches, + version_updates, + }) } /// Create a new collection with only the patches that apply to the @@ -446,4 +518,74 @@ mod test { assert_eq!(union.patches.len(), 1); assert_eq!(union.patches[0].platforms.len(), 0); } + + #[test] + fn test_version_differentials() { + let fixture = version_range_fixture(); + let diff = fixture[0].version_range_diffs(&fixture[1]); + assert_eq!(diff.len(), 1); + assert_eq!( + &diff, + &[( + "a".to_string(), + Some(VersionRange { + from: Some(0), + until: Some(1) + }) + )] + ); + let diff = fixture[1].version_range_diffs(&fixture[2]); + assert_eq!(diff.len(), 0); + } + + #[test] + fn test_version_updates() { + let fixture = version_range_fixture(); + let collection = fixture[0].update_version_ranges(&[("a".into(), None)]); + assert_eq!(collection.patches[0].version_range, None); + assert_eq!(collection.patches[1], fixture[1].patches[1]); + let new_version_range = Some(VersionRange { + from: Some(42), + until: Some(43), + }); + let collection = fixture[0].update_version_ranges(&[("a".into(), new_version_range)]); + assert_eq!(collection.patches[0].version_range, new_version_range); + assert_eq!(collection.patches[1], fixture[1].patches[1]); + } + + fn version_range_fixture() -> Vec<PatchCollection> { + let patch1 = PatchDictSchema { + rel_patch_path: "a".into(), + metadata: None, + platforms: Default::default(), + version_range: Some(VersionRange { + from: Some(0), + until: Some(1), + }), + }; + let patch1_updated = PatchDictSchema { + version_range: Some(VersionRange { + from: Some(0), + until: Some(3), + }), + ..patch1.clone() + }; + let patch2 = PatchDictSchema { + rel_patch_path: "b".into(), + ..patch1.clone() + }; + let collection1 = PatchCollection { + workdir: PathBuf::new(), + patches: vec![patch1, patch2.clone()], + }; + let collection2 = PatchCollection { + workdir: PathBuf::new(), + patches: vec![patch1_updated, patch2.clone()], + }; + let collection3 = PatchCollection { + workdir: PathBuf::new(), + patches: vec![patch2], + }; + vec![collection1, collection2, collection3] + } } |