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 <gbiv@chromium.org>
Tested-by: Jordan Abrahams-Whitehead <ajordanr@google.com>
Commit-Queue: Jordan Abrahams-Whitehead <ajordanr@google.com>

3 files changed, 116 insertions, 54 deletions

diff --git a/llvm_tools/patch_sync/src/main.rs b/llvm_tools/patch_sync/src/main.rs
index cec523bb..bfcba584 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 patch_parsing::PatchCollection;
 use std::borrow::ToOwned;
 use std::path::PathBuf;
 use structopt::StructOpt;
@@ -55,11 +56,10 @@ fn show_subcmd(
     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)
+    let cur_cros_collection = 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 cur_android_collection = PatchCollection::parse_from_file(&android_patches_path)
+        .context("could not parse android PATCHES.json")?;
     let merged = cur_cros_collection.union(&cur_android_collection)?;
     println!("{}", merged.serialize_patches()?);
     Ok(())
@@ -89,33 +89,24 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> {
     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")?
-            .filter_patches(|p| p.platforms.contains("chromiumos"));
-    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)?
-    };
-
-    // Android Patches -------------------------------------------------------
-    let mut cur_android_collection =
-        patch_parsing::PatchCollection::parse_from_file(&android_patches_path)
-            .context("parsing android PATCHES.json")?
-            .filter_patches(|p| p.platforms.contains("android"));
-    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)?
-    };
+    // Get new Patches -------------------------------------------------------
+    let (mut 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 (mut 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")?;
+
+    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");
@@ -148,6 +139,15 @@ fn transpose_subcmd(args: TransposeOpt) -> Result<()> {
     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 {
diff --git a/llvm_tools/patch_sync/src/patch_parsing.rs b/llvm_tools/patch_sync/src/patch_parsing.rs
index c89d4eb2..1da2c2a6 100644
--- a/llvm_tools/patch_sync/src/patch_parsing.rs
+++ b/llvm_tools/patch_sync/src/patch_parsing.rs
@@ -53,7 +53,14 @@ impl PatchCollection {
         }
     }
 
-    #[allow(dead_code)]
+    /// 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()
@@ -196,11 +203,67 @@ 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))?
+        .filter_patches(|p| p.platforms.contains(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| PatchDictSchema {
+        platforms: BTreeSet::from(["android".to_string(), "chromiumos".to_string()])
+            .union(&p.platforms)
+            .cloned()
+            .collect(),
+
+        ..p.to_owned()
+    });
+    Ok((cur_collection, new_patches))
+}
+
 /// Get the hash from the patch file contents.
 ///
 /// Not every patch file actually contains its own hash,
@@ -228,7 +291,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)
 }
 
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<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>>(
         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<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!(
             "{}:{}",