patch_sync: Sync version range changes

At present, patch_sync intentionally does not
copy over version changes to existing patches, it only
copies new patches to the other repositories.

This commit changes this behaviour at the request of the
Android toolchain team, so that now version range
changes are also copied over (but nothing else).

BUG=b:237030928
TEST=cargo test
TEST=patch_sync locally with version range changes

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

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]
+    }
 }