pkgs/rollup: Allow multiple owners in rollup config drop-ins

2 files changed, 71 insertions, 32 deletions

diff --git a/pkgs/by-name/ro/rollup/src/config.rs b/pkgs/by-name/ro/rollup/src/config.rs
index 64e0b792..66bd2366 100644
--- a/pkgs/by-name/ro/rollup/src/config.rs
+++ b/pkgs/by-name/ro/rollup/src/config.rs
@@ -36,7 +36,7 @@ pub(crate) struct Owner {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub(crate) struct DropInConfig {
-    pub(crate) owner: String,
+    pub(crate) owners: Vec<String>,
 }
 
 pub(crate) fn load(root: &PathBuf) -> Result<MainConfig> {
@@ -64,13 +64,41 @@ fn collect_drop_ins(path: &PathBuf) -> Result<Vec<(PathBuf, DropInConfig)>> {
         if ft.is_dir() {
             output.extend(collect_drop_ins(&entry.path())?);
         } else if ft.is_file() && entry.file_name() == OsStr::new(DROP_IN_NAME) {
-            let drop_in: DropInConfig = {
+            let drop_in: toml::Value = {
                 let content = fs::read_to_string(entry.path())?;
                 toml::from_str(&content)?
             };
+
+            let drop_in_config = DropInConfig {
+                owners: {
+                    let Some(owner) = drop_in.get("owner") else {
+                        bail!("Drop in config without `owner` field")
+                    };
+                    if owner.is_str() {
+                        vec![owner.as_str().expect("We checked").to_owned()]
+                    } else if owner.is_array() {
+                        let mut output = vec![];
+                        for v in owner.as_array().expect("We checked") {
+                            if let Some(v) = v.as_str() {
+                                output.push(v.to_owned())
+                            } else {
+                                bail!("Owner value in arry not string: {}", v);
+                            }
+                        }
+                        output
+                    } else {
+                        bail!("Owner value in drop in config not string or array")
+                    }
+                },
+            };
+
             output.push((
-                entry.path().parent().expect("All drop-ins have a parent").to_owned(),
-                drop_in,
+                entry
+                    .path()
+                    .parent()
+                    .expect("All drop-ins have a parent")
+                    .to_owned(),
+                drop_in_config,
             ));
         }
     }
diff --git a/pkgs/by-name/ro/rollup/src/main.rs b/pkgs/by-name/ro/rollup/src/main.rs
index 090ac12c..28ef0fea 100644
--- a/pkgs/by-name/ro/rollup/src/main.rs
+++ b/pkgs/by-name/ro/rollup/src/main.rs
@@ -130,6 +130,12 @@ macro_rules! git {
     }};
 }
 
+macro_rules! drop_msg {
+    ($fmt:literal, $($args:expr),*) => {
+       eprintln!("  > {}", format!($fmt, $($args),*));
+    };
+}
+
 fn main() -> Result<()> {
     let args = Cli::parse();
     let root = {
@@ -252,7 +258,7 @@ fn apply_patches(
     root: PathBuf,
     dry_run: bool,
 ) -> Result<()> {
-    let to = &owner.applies_to;
+    let applies_to = &owner.applies_to;
 
     let mut patches = vec![];
     for file in &patch_files {
@@ -262,11 +268,11 @@ fn apply_patches(
     eprintln!("Successfully parsed all {} patch files.", patches.len());
 
     {
-        let (stdout, _) = git!(in to; "status", "--porcelain");
+        let (stdout, _) = git!(in applies_to; "status", "--porcelain");
         if !stdout.is_empty() {
             bail!(
                 "`--to` (`{}`) workspace is dirty, refusing to continue.",
-                to.display()
+                applies_to.display()
             );
         }
     }
@@ -275,14 +281,17 @@ fn apply_patches(
         eprintln!(
             "Applying {}. patch: `{}`... ",
             idx + 1,
-            patch_files[idx].display()
+            patch_files[idx]
+                .file_name()
+                .expect("Has a file name")
+                .display()
         );
 
         let mut final_diffs = vec![];
         for file_diff in &patch.files {
             match file_diff {
                 FileDiff::Add { path, .. } => {
-                    if to.join(path).exists() {
+                    if applies_to.join(path).exists() {
                         bail!("A file was added, that already existed. This should be impossible.")
                     }
                     if drop_in_applies(&config.drop_ins, owner, &root, path)? {
@@ -290,46 +299,46 @@ fn apply_patches(
                     }
                 }
                 FileDiff::Remove { path, .. } => {
-                    if to.join(path).exists() {
+                    if applies_to.join(path).exists() {
                         if drop_in_applies(&config.drop_ins, owner, &root, path)? {
                             final_diffs.push(file_diff.to_owned());
                         }
                     } else {
-                        eprintln!(
-                            "  > Dropping remove of `{}`, as the file does not exist in the rollup target.",
+                        drop_msg!(
+                            "Dropping remove of `{}`, as the file does not exist in the rollup target.",
                             path.display()
                         );
                     }
                 }
                 FileDiff::RenameNoChange { from, to, .. }
                 | FileDiff::RenameWithChange { from, to, .. } => {
-                    if to.join(from).exists() {
-                        if to.join(to).exists() {
+                    if applies_to.join(from).exists() {
+                        if applies_to.join(to).exists() {
                             bail!(
                                 "A rename (`{}` -> `{}`) tried to rename a path to an already existing path. This should be impossible.",
                                 from.display(),
                                 to.display()
                             )
                         }
-                        if drop_in_applies(&config.drop_ins, owner, &root, from)? {
+                        if drop_in_applies(&config.drop_ins, owner, &root, to)? {
                             final_diffs.push(file_diff.to_owned());
                         }
                     } else {
-                        eprintln!(
-                            "  > Dropping rename from `{}` to `{}`, as the from file does not exist in the rollup target.",
+                        drop_msg!(
+                            "Dropping rename from `{}` to `{}`, as the from file does not exist in the rollup target.",
                             from.display(),
                             to.display()
                         );
                     }
                 }
                 FileDiff::Change { path, .. } => {
-                    if to.join(path).exists() {
+                    if applies_to.join(path).exists() {
                         if drop_in_applies(&config.drop_ins, owner, &root, path)? {
                             final_diffs.push(file_diff.to_owned());
                         }
                     } else {
-                        eprintln!(
-                            "  > Dropping change to `{}`, as the file does not exist in the rollup target.",
+                        drop_msg!(
+                            "Dropping change to `{}`, as the file does not exist in the rollup target.",
                             path.display()
                         );
                     }
@@ -348,11 +357,11 @@ fn apply_patches(
         };
 
         if dry_run {
-            eprintln!("Dry-run is on; Not actually applying, adding or commiting");
+            eprintln!("  (Dry-run is on; Not applying.)");
         } else {
-            git!(@set_stdin_to filtered_patch; in to; "apply", "-", "--whitespace=nowarn");
-            git!(in to; "add", ".");
-            git!(@set_stdin_to filtered_patch.commit_message; in to; "commit", "--file=-", format!("--date={}", filtered_patch.date).as_str(), "--quiet");
+            git!(@set_stdin_to filtered_patch; in applies_to; "apply", "-", "--whitespace=nowarn");
+            git!(in applies_to; "add", ".");
+            git!(@set_stdin_to filtered_patch.commit_message; in applies_to; "commit", "--file=-", format!("--date={}", filtered_patch.date).as_str(), "--quiet");
         }
     }
 
@@ -387,13 +396,13 @@ fn drop_in_applies(
     drop_ins: &HashMap<PathBuf, DropInConfig>,
     owner: &Owner,
     root: &Path,
-    path: &Path,
+    original_path: &Path,
 ) -> Result<bool> {
-    let path = &path.canonicalize()?;
+    let path = &original_path.canonicalize()?;
 
     let file_name = path.file_name().expect("All paths are to files");
     if file_name == OsStr::new(DROP_IN_NAME) || file_name == OsStr::new(MAIN_CONFIG_NAME) {
-        eprintln!(
+        drop_msg!(
             "Not applying change to `{}`, because it's a rollup config or drop in.",
             path.display()
         );
@@ -414,7 +423,7 @@ fn drop_in_applies(
     );
 
     if let Some(conf) = drop_ins.get(&path)
-        && conf.owner == owner.name
+        && conf.owners.contains(&owner.name)
     {
         return Ok(true);
     }
@@ -425,12 +434,14 @@ fn drop_in_applies(
 
         if let Some(conf) = drop_ins.get(&root.join(next)) {
             // eprint!("Found a drop-in for `{}`", next.display());
-            if conf.owner != owner.name {
+            if !conf.owners.contains(&owner.name) {
                 // eprintln!(", but it's for `{}`, so ignoring path.", conf.owner);
-                eprintln!(
-                    "`{}` says this dir is for `{}`, so ignoring path.",
+                drop_msg!(
+                    "Dropping `{}` as `{}` says this dir is for `{}` (but we look for `{}`).",
+                    original_path.display(),
                     next.join(DROP_IN_NAME).display(),
-                    conf.owner
+                    conf.owners.join(", "),
+                    owner.name
                 );
                 return Ok(false);
             } else {