Merge pull request #6113 from commercialhaskell/commenter-stdin

Simpler `commenter` flow, allow piping stderr from `curator`

CURATORS.md

@@ -173,12 +173,12 @@ This typically happens when we move to a new major GHC release or when
 there are only a few packages waiting for updates on an upper bounds
 issue.
 
-Comment out the offending packages from the "packages" section and add
+If a package needs to be disabled due to build failures: Add a `< 0`
-a comment saying why it was disabled:
+bound to the package to exclude it, and add a comment stating why it
+was disabled: `- swagger < 0 # compile failure againts aeson 1.0`
 
-```
+If a package needs to be disabled due to bounds issues, see the "Large
-        # - swagger # bounds: aeson 1.0
+scale enabling/disabling of packages" section below.
-```
 
 If this causes reverse dependencies to be disabled we should notify
 the maintainers of those packages.
@@ -413,7 +413,19 @@ errors for builds, tests and benchmarks.
 `build-constraints.yaml`. It can only handle bounds issues,
 compilation issues still need to be analyzed manually.
 
-Example usage: After disabling a few packages you get this curator output:
+It disables all offending packages/test suites/benchmarks, so it is
+only meant to be used when we close bounds issues and want to disable
+packages, and when upgrading GHC.
+
+#### Setup
+This is currently a rust program, You can install the rust toolchain
+by using [rustup](https://rustup.rs/).
+
+Then `cd etc/commenter && cargo install --locked --path .`
+
+#### Example usage
+
+After disabling a few packages you get this curator output:
 
 ```
 ConfigFile (GHC 9 bounds issues, @maintainer) (not present) depended on by:
@@ -428,10 +440,14 @@ testing-feat (GHC 9 bounds issues, Grandfathered dependencies) (not present) dep
 - [ ] dual-tree-0.2.3.0 (-any). Grandfathered dependencies. @handles. Used by: test-suite
 ```
 
-Copy this into `etc/commenter/comments.txt` and run `./commenter` (currently requires Rust)
+Now run:
+```
+./check 2>&1 >/dev/null | commenter
+```
 
 You will get this output:
 ```
+[INFO] ...
 LIBS + EXES
 
         - xdg-desktop-entry < 0 # tried xdg-desktop-entry-0.1.1.1, but its *library* requires the disabled package: ConfigFile
@@ -448,7 +464,7 @@ TESTS have a similar section under `skipped-tests`, and BENCHMARKS under `skippe
 
 #### Re-enabling
 
-We can periodically remove all packages under the bounds sections and then re-run the disabling flow above until we get a clean plan. This way we will automatically pick up packages that have been fixed.
+We can periodically remove all packages under the bounds sections and then re-run the disabling flow above until we get a clean plan. This will automatically pick up packages that have been fixed.
 
 #### Notes
 

etc/commenter/Cargo.lock

@@ -1,5 +1,7 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+version = 3
+
 [[package]]
 name = "aho-corasick"
 version = "0.7.18"
@@ -13,15 +15,63 @@ dependencies = [
 name = "commenter"
 version = "0.1.0"
 dependencies = [
+ "lazy-regex",
  "regex",
 ]
 
+[[package]]
+name = "lazy-regex"
+version = "2.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "17d198f91272f6e788a5c0bd5d741cf778da4e5bc761ec67b32d5d3b0db34a54"
+dependencies = [
+ "lazy-regex-proc_macros",
+ "once_cell",
+ "regex",
+]
+
+[[package]]
+name = "lazy-regex-proc_macros"
+version = "2.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6c12938b1b92cf5be22940527e15b79fd0c7e706e34bc70816f6a72b3484f84e"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "regex",
+ "syn",
+]
+
 [[package]]
 name = "memchr"
 version = "2.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b16bd47d9e329435e309c58469fe0791c2d0d1ba96ec0954152a5ae2b04387dc"
 
+[[package]]
+name = "once_cell"
+version = "1.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56"
+
+[[package]]
+name = "proc-macro2"
+version = "1.0.27"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f0d8caf72986c1a598726adc988bb5984792ef84f5ee5aa50209145ee8077038"
+dependencies = [
+ "unicode-xid",
+]
+
+[[package]]
+name = "quote"
+version = "1.0.9"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c3d0b9745dc2debf507c8422de05d7226cc1f0644216dfdfead988f9b1ab32a7"
+dependencies = [
+ "proc-macro2",
+]
+
 [[package]]
 name = "regex"
 version = "1.5.4"
@@ -38,3 +88,20 @@ name = "regex-syntax"
 version = "0.6.25"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b"
+
+[[package]]
+name = "syn"
+version = "1.0.73"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f71489ff30030d2ae598524f61326b902466f72a0fb1a8564c001cc63425bcc7"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-xid",
+]
+
+[[package]]
+name = "unicode-xid"
+version = "0.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3"

etc/commenter/Cargo.toml

@@ -7,4 +7,5 @@ edition = "2018"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+lazy-regex = "2.2.1"
 regex = "1.5.4"

etc/commenter/README.md

@@ -1 +1,3 @@
 Helps automate mass-disabling of packages in Stackage's build-constraint.yaml.
+
+See CURATORS.md for usage instructions.

etc/commenter/src/main.rs

@@ -1,8 +1,7 @@
-use std::collections::HashMap;
+use std::collections::{HashMap};
-use std::fs::File;
 use std::io::{self, BufRead};
-use std::path::Path;
 
+use lazy_regex::regex;
 use regex::Regex;
 
 type H = HashMap<Header, Vec<(String, String, String)>>;
@@ -18,43 +17,47 @@ fn main() {
     let mut tests: H = Default::default();
     let mut benches: H = Default::default();
     let mut last_header: Option<Header> = None;
-    let empty = Regex::new(r#"^\s*$"#).unwrap();
-    let header_versioned =
-        Regex::new(r#"^(?P<package>[a-zA-z]([a-zA-z0-9.-]*?))-(?P<version>(\d+(\.\d+)*)).+?is out of bounds for:$"#).unwrap();
-    let header_missing =
-        Regex::new(r#"^(?P<package>[a-zA-z]([a-zA-z0-9.-]*)).+?depended on by:$"#).unwrap();
-    let package =
-        Regex::new(r#"^- \[ \] (?P<package>[a-zA-z]([a-zA-z0-9.-]*?))-(?P<version>(\d+(\.\d+)*)).+?Used by: (?P<component>.+)$"#)
-        .unwrap();
 
-    if let Ok(lines) = read_lines("./comments.txt") {
+    let header_versioned = regex!(
-        for line in lines {
+        r#"^(?P<package>[a-zA-z]([a-zA-z0-9.-]*?))-(?P<version>(\d+(\.\d+)*)).+?is out of bounds for:$"#
-            if let Ok(line) = line {
+    );
-                if empty.captures(&line).is_some() {
+    let header_missing = regex!(r#"^(?P<package>[a-zA-z]([a-zA-z0-9.-]*)).+?depended on by:$"#);
-                } else if let Some(cap) = package.captures(&line) {
+    let package = regex!(
-                    let root = last_header.clone().unwrap();
+        r#"^- \[ \] (?P<package>[a-zA-z]([a-zA-z0-9.-]*?))-(?P<version>(\d+(\.\d+)*)).+?Used by: (?P<component>.+)$"#
-                    let package = cap.name("package").unwrap().as_str();
+    );
-                    let version = cap.name("version").unwrap().as_str();
+
-                    let component = cap.name("component").unwrap().as_str();
+    // Ignore everything until the bounds issues show up.
-                    match component {
+    let mut process_line = false;
-                        "library" | "executable" => {
+
-                            insert(&mut lib_exes, root, package, version, component)
+    for line in io::stdin().lock().lines().flatten() {
-                        }
+        if is_reg_match(&line, regex!(r#"^\s*$"#)) {
-                        "benchmark" => insert(&mut benches, root, package, version, "benchmarks"),
+            // noop
-                        "test-suite" => insert(&mut tests, root, package, version, component),
+        } else if line == "curator: Snapshot dependency graph contains errors:" {
-                        _ => panic!("Bad component: {}", component),
+            process_line = true;
-                    }
+        } else if !process_line {
-                } else if let Some(cap) = header_versioned.captures(&line) {
+            println!("[INFO] {}", line);
-                    let package = cap.name("package").unwrap().as_str().to_owned();
+        } else if let Some(cap) = package.captures(&line) {
-                    let version = cap.name("version").unwrap().as_str().to_owned();
+            let root = last_header.clone().unwrap();
-                    last_header = Some(Header::Versioned { package, version });
+            let package = cap.name("package").unwrap().as_str();
-                } else if let Some(cap) = header_missing.captures(&line) {
+            let version = cap.name("version").unwrap().as_str();
-                    let package = cap.name("package").unwrap().as_str().to_owned();
+            let component = cap.name("component").unwrap().as_str();
-                    last_header = Some(Header::Missing { package });
+            match component {
-                } else {
+                "library" | "executable" => {
-                    panic!("Unhandled: {:?}", line);
+                    insert(&mut lib_exes, root, package, version, component)
                 }
+                "benchmark" => insert(&mut benches, root, package, version, "benchmarks"),
+                "test-suite" => insert(&mut tests, root, package, version, component),
+                _ => panic!("Bad component: {}", component),
             }
+        } else if let Some(cap) = header_versioned.captures(&line) {
+            let package = cap.name("package").unwrap().as_str().to_owned();
+            let version = cap.name("version").unwrap().as_str().to_owned();
+            last_header = Some(Header::Versioned { package, version });
+        } else if let Some(cap) = header_missing.captures(&line) {
+            let package = cap.name("package").unwrap().as_str().to_owned();
+            last_header = Some(Header::Missing { package });
+        } else {
+            panic!("Unhandled: {:?}", line);
         }
     }
 
@@ -93,7 +96,7 @@ fn printer(
     version: &str,
     component: &str,
     header: &Header,
-) -> () {
+) {
     let lt0 = if lt0 { " < 0" } else { "" };
     println!(
         "{indent}- {package}{lt0} # tried {package}-{version}, but its *{component}* {cause}",
@@ -117,17 +120,13 @@ fn printer(
 }
 
 fn insert(h: &mut H, header: Header, package: &str, version: &str, component: &str) {
-    (*h.entry(header).or_insert_with(|| vec![])).push((
+    (*h.entry(header).or_insert_with(Vec::new)).push((
         package.to_owned(),
         version.to_owned(),
         component.to_owned(),
     ));
 }
 
-fn read_lines<P>(filename: P) -> io::Result<io::Lines<io::BufReader<File>>>
+fn is_reg_match(s: &str, r: &Regex) -> bool {
-where
+    r.captures(s).is_some()
-    P: AsRef<Path>,
-{
-    let file = File::open(filename)?;
-    Ok(io::BufReader::new(file).lines())
 }