patch_sync: Rework show cmd to display merged

At present, patch_sync show merges two PATCHES.json files by
merging their platform contents. However, because of timing,
review latency, or just patches being denied, it's possible that
one repo recommends a patch being applied to a certain platform,
and the other denies that same patch.

To resolve this conflict, this commit by default only considers
patches that exist in the PATCHES.json file at the present time.

The original behaviour can be enabled by turning on --keep-unmerged.

BUG=b:209493133
TEST=patch_sync show <...cros> <...android>
TEST=patch_sync transpose --dry-run -s <...>

Change-Id: I3bdf6c36b4dbfe26d4221191b5c22363a7f0dfe0
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3388390
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, 58 insertions, 15 deletions

diff --git a/llvm_tools/patch_sync/src/main.rs b/llvm_tools/patch_sync/src/main.rs
index 9616716c..8c1eff1c 100644
--- a/llvm_tools/patch_sync/src/main.rs
+++ b/llvm_tools/patch_sync/src/main.rs
@@ -2,19 +2,28 @@ mod android_utils;
 mod patch_parsing;
 mod version_control;
 
-use anyhow::{Context, Result};
-use patch_parsing::PatchCollection;
 use std::borrow::ToOwned;
-use std::path::PathBuf;
+use std::collections::BTreeSet;
+use std::path::{Path, PathBuf};
+
+use anyhow::{Context, Result};
 use structopt::StructOpt;
 
+use patch_parsing::{filter_patches_by_platform, PatchCollection, PatchDictSchema};
+
 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,
@@ -44,29 +53,47 @@ fn main() -> Result<()> {
     }
 }
 
-fn show_subcmd(
+struct ShowOpt {
     cros_checkout_path: PathBuf,
     android_checkout_path: PathBuf,
+    keep_unmerged: bool,
     sync: bool,
-) -> Result<()> {
+}
+
+fn show_subcmd(args: ShowOpt) -> Result<()> {
+    let ShowOpt {
+        cros_checkout_path,
+        android_checkout_path,
+        keep_unmerged,
+        sync,
+    } = args;
     let ctx = version_control::RepoSetupContext {
         cros_checkout: cros_checkout_path,
         android_checkout: android_checkout_path,
         sync_before: sync,
     };
     ctx.setup()?;
-    let cros_patches_path = ctx.cros_patches_path();
-    let android_patches_path = ctx.android_patches_path();
-    let cur_cros_collection = PatchCollection::parse_from_file(&cros_patches_path)
-        .context("could not parse cros PATCHES.json")?;
-    let cur_android_collection = 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| {
+                PatchDictSchema {
+                    platforms: BTreeSet::from([platform.to_string()]),
+                    ..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,
@@ -184,6 +211,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,
     },
diff --git a/llvm_tools/patch_sync/src/patch_parsing.rs b/llvm_tools/patch_sync/src/patch_parsing.rs
index 1da2c2a6..2f0fbc87 100644
--- a/llvm_tools/patch_sync/src/patch_parsing.rs
+++ b/llvm_tools/patch_sync/src/patch_parsing.rs
@@ -242,8 +242,8 @@ pub fn new_patches(
     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));
+        .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(
@@ -264,6 +264,16 @@ pub fn new_patches(
     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,