Merge pull request #307 from NixOS/maintainer-tag

Maintainer tag

ofborg/src/maintainers.rs

@@ -1,20 +1,22 @@
 use ofborg::nix::Nix;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::io::Write;
 use std::path::Path;
 use tempfile::NamedTempFile;
 
 #[derive(Deserialize, Debug, Eq, PartialEq)]
 pub struct ImpactedMaintainers(HashMap<Maintainer, Vec<Package>>);
-#[derive(Deserialize, Debug, Eq, PartialEq, Hash)]
+pub struct MaintainersByPackage(pub HashMap<Package, HashSet<Maintainer>>);
-struct Maintainer(String);
+
+#[derive(Deserialize, Clone, Debug, Eq, PartialEq, Hash)]
+pub struct Maintainer(String);
 impl<'a> From<&'a str> for Maintainer {
     fn from(name: &'a str) -> Maintainer {
         Maintainer(name.to_owned())
     }
 }
-#[derive(Deserialize, Debug, Eq, PartialEq, Hash)]
+#[derive(Deserialize, Clone, Debug, Eq, PartialEq, Hash)]
-struct Package(String);
+pub struct Package(String);
 impl<'a> From<&'a str> for Package {
     fn from(name: &'a str) -> Package {
         Package(name.to_owned())
@@ -80,6 +82,22 @@ impl ImpactedMaintainers {
             .map(|(maintainer, _)| maintainer.0.clone())
             .collect()
     }
+
+    pub fn maintainers_by_package(&self) -> MaintainersByPackage {
+        let mut bypkg = MaintainersByPackage(HashMap::new());
+
+        for (maintainer, packages) in self.0.iter() {
+            for package in packages.iter() {
+                bypkg
+                    .0
+                    .entry(package.clone())
+                    .or_insert_with(HashSet::new)
+                    .insert(maintainer.clone());
+            }
+        }
+
+        bypkg
+    }
 }
 
 impl std::fmt::Display for ImpactedMaintainers {

ofborg/src/tagger.rs

@@ -1,3 +1,4 @@
+use crate::maintainers::{Maintainer, MaintainersByPackage};
 use ofborg::outpathdiff::PackageArch;
 use ofborg::tasks;
 use std::collections::HashMap;
@@ -247,6 +248,61 @@ impl PathsTagger {
     }
 }
 
+pub struct MaintainerPRTagger {
+    possible: Vec<String>,
+    selected: Vec<String>,
+}
+
+impl Default for MaintainerPRTagger {
+    fn default() -> MaintainerPRTagger {
+        let mut t = MaintainerPRTagger {
+            possible: vec![String::from("11.by: package-maintainer")],
+            selected: vec![],
+        };
+        t.possible.sort();
+
+        t
+    }
+}
+
+impl MaintainerPRTagger {
+    pub fn new() -> MaintainerPRTagger {
+        Default::default()
+    }
+
+    pub fn record_maintainer(
+        &mut self,
+        pr_submitter: &str,
+        identified_maintainers: &MaintainersByPackage,
+    ) {
+        let submitter = Maintainer::from(pr_submitter);
+
+        if identified_maintainers.0.is_empty() {
+            // No packages -> not from the maintainer
+            return;
+        }
+
+        for (_package, maintainers) in identified_maintainers.0.iter() {
+            if !maintainers.contains(&submitter) {
+                // One of the packages is not maintained by this submitter
+                return;
+            }
+        }
+
+        self.selected
+            .push(String::from("11.by: package-maintainer"));
+    }
+
+    pub fn tags_to_add(&self) -> Vec<String> {
+        self.selected.clone()
+    }
+
+    pub fn tags_to_remove(&self) -> Vec<String> {
+        // The cleanup tag is too vague to automatically remove.
+        vec![]
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

ofborg/src/tasks/massrebuilder.rs

@@ -7,6 +7,7 @@ use crate::maintainers;
 use crate::maintainers::ImpactedMaintainers;
 use amqp::protocol::basic::{BasicProperties, Deliver};
 use hubcaps;
+use hubcaps::issues::Issue;
 use ofborg::acl::ACL;
 use ofborg::checkout;
 use ofborg::commentparser::Subset;
@@ -19,7 +20,9 @@ use ofborg::outpathdiff::{OutPathDiff, OutPaths};
 use ofborg::stats;
 use ofborg::stats::Event;
 use ofborg::systems;
-use ofborg::tagger::{PathsTagger, PkgsAddedRemovedTagger, RebuildTagger, StdenvTagger};
+use ofborg::tagger::{
+    MaintainerPRTagger, PathsTagger, PkgsAddedRemovedTagger, RebuildTagger, StdenvTagger,
+};
 use ofborg::worker;
 use std::collections::HashMap;
 use std::path::Path;
@@ -122,11 +125,11 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
         let gists = self.github.gists();
         let pulls = repo.pulls();
         let pull = pulls.get(job.pr.number);
-        let issue = repo.issue(job.pr.number);
+        let issue_ref = repo.issue(job.pr.number);
-
+        let issue: Issue;
         let auto_schedule_build_archs: Vec<systems::System>;
 
-        match issue.get() {
+        match issue_ref.get() {
             Ok(iss) => {
                 if iss.state == "closed" {
                     self.events.notify(Event::IssueAlreadyClosed);
@@ -142,16 +145,19 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
                         &job.repo.full_name,
                     );
                 }
+
+                issue = iss;
             }
+
             Err(e) => {
                 self.events.notify(Event::IssueFetchFailed);
                 info!("Error fetching {}!", job.pr.number);
                 info!("E: {:?}", e);
                 return self.actions().skip(&job);
             }
-        }
+        };
 
-        self.tag_from_title(&issue);
+        self.tag_from_title(&issue_ref);
 
         let mut overall_status = CommitStatus::new(
             repo.statuses(),
@@ -247,7 +253,7 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
         let changed_paths = co
             .files_changed_from_head(&job.pr.head_sha)
             .unwrap_or_else(|_| vec![]);
-        self.tag_from_paths(&issue, &changed_paths);
+        self.tag_from_paths(&issue_ref, &changed_paths);
 
         overall_status.set_with_description("Merging PR", hubcaps::statuses::State::Pending);
 
@@ -257,11 +263,11 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
 
             info!("Failed to merge {}", job.pr.head_sha);
 
-            update_labels(&issue, &["2.status: merge conflict".to_owned()], &[]);
+            update_labels(&issue_ref, &["2.status: merge conflict".to_owned()], &[]);
 
             return self.actions().skip(&job);
         } else {
-            update_labels(&issue, &[], &["2.status: merge conflict".to_owned()]);
+            update_labels(&issue_ref, &[], &["2.status: merge conflict".to_owned()]);
         }
 
         overall_status
@@ -510,7 +516,7 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
                 stdenvtagger.changed(stdenvs.changed());
             }
             update_labels(
-                &issue,
+                &issue_ref,
                 &stdenvtagger.tags_to_add(),
                 &stdenvtagger.tags_to_remove(),
             );
@@ -519,7 +525,7 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
                 let mut addremovetagger = PkgsAddedRemovedTagger::new();
                 addremovetagger.changed(&removed, &added);
                 update_labels(
-                    &issue,
+                    &issue_ref,
                     &addremovetagger.tags_to_add(),
                     &addremovetagger.tags_to_remove(),
                 );
@@ -563,7 +569,17 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
                         },
                     );
 
-                    request_reviews(&m, &pull);
+                    if let Ok(ref maint) = m {
+                        request_reviews(&maint, &pull);
+                        let mut maint_tagger = MaintainerPRTagger::new();
+                        maint_tagger
+                            .record_maintainer(&issue.user.login, &maint.maintainers_by_package());
+                        update_labels(
+                            &issue_ref,
+                            &maint_tagger.tags_to_add(),
+                            &maint_tagger.tags_to_remove(),
+                        );
+                    }
 
                     let mut status = CommitStatus::new(
                         repo.statuses(),
@@ -580,7 +596,7 @@ impl<E: stats::SysEvents + 'static> worker::SimpleWorker for MassRebuildWorker<E
             }
 
             update_labels(
-                &issue,
+                &issue_ref,
                 &rebuild_tags.tags_to_add(),
                 &rebuild_tags.tags_to_remove(),
             );
@@ -745,22 +761,17 @@ fn indicates_wip(text: &str) -> bool {
     false
 }
 
-fn request_reviews(
+fn request_reviews(maint: &maintainers::ImpactedMaintainers, pull: &hubcaps::pulls::PullRequest) {
-    m: &Result<maintainers::ImpactedMaintainers, maintainers::CalculationError>,
+    if maint.maintainers().len() < 10 {
-    pull: &hubcaps::pulls::PullRequest,
+        for maintainer in maint.maintainers() {
-) {
+            if let Err(e) =
-    if let Ok(ref maint) = m {
+                pull.review_requests()
-        if maint.maintainers().len() < 10 {
+                    .create(&hubcaps::review_requests::ReviewRequestOptions {
-            for maintainer in maint.maintainers() {
+                        reviewers: vec![maintainer.clone()],
-                if let Err(e) =
+                        team_reviewers: vec![],
-                    pull.review_requests()
+                    })
-                        .create(&hubcaps::review_requests::ReviewRequestOptions {
+            {
-                            reviewers: vec![maintainer.clone()],
+                println!("Failure requesting a review from {}: {:#?}", maintainer, e,);
-                            team_reviewers: vec![],
-                        })
-                {
-                    println!("Failure requesting a review from {}: {:#?}", maintainer, e,);
-                }
             }
         }
     }