Simpler commenter flow, allow piping stderr from curator

2026-01-11 23:08:30 +01:00 · 2021-07-13 17:16:06 +02:00 · 2021-07-13 17:16:06 +02:00 · 3163a84b97
commit 3163a84b97
parent e9f1385463
5 changed files with 138 additions and 53 deletions
--- a/CURATORS.md
+++ b/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
-a comment saying why it was disabled:
+If a package needs to be disabled due to build failures: Add a `< 0`
+bound to the package to exclude it, and add a comment stating why it
+was disabled: `- swagger < 0 # compile failure againts aeson 1.0`

-```
-        # - swagger # bounds: aeson 1.0
-```
+If a package needs to be disabled due to bounds issues, see the "Large
+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

--- a/etc/commenter/Cargo.lock
+++ b/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"
--- a/etc/commenter/Cargo.toml
+++ b/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"
--- a/etc/commenter/README.md
+++ b/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.
--- a/etc/commenter/src/main.rs
+++ b/etc/commenter/src/main.rs
@ -1,8 +1,7 @@
-use std::collections::HashMap;
-use std::fs::File;
+use std::collections::{HashMap};
 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") {
-        for line in lines {
-            if let Ok(line) = line {
-                if empty.captures(&line).is_some() {
-                } else if let Some(cap) = package.captures(&line) {
-                    let root = last_header.clone().unwrap();
-                    let package = cap.name("package").unwrap().as_str();
-                    let version = cap.name("version").unwrap().as_str();
-                    let component = cap.name("component").unwrap().as_str();
-                    match component {
-                        "library" | "executable" => {
-                            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);
+    let header_versioned = regex!(
+        r#"^(?P<package>[a-zA-z]([a-zA-z0-9.-]*?))-(?P<version>(\d+(\.\d+)*)).+?is out of bounds for:$"#
+    );
+    let header_missing = regex!(r#"^(?P<package>[a-zA-z]([a-zA-z0-9.-]*)).+?depended on by:$"#);
+    let package = regex!(
+        r#"^- \[ \] (?P<package>[a-zA-z]([a-zA-z0-9.-]*?))-(?P<version>(\d+(\.\d+)*)).+?Used by: (?P<component>.+)$"#
+    );
+
+    // Ignore everything until the bounds issues show up.
+    let mut process_line = false;
+
+    for line in io::stdin().lock().lines().flatten() {
+        if is_reg_match(&line, regex!(r#"^\s*$"#)) {
+            // noop
+        } else if line == "curator: Snapshot dependency graph contains errors:" {
+            process_line = true;
+        } else if !process_line {
+            println!("[INFO] {}", line);
+        } else if let Some(cap) = package.captures(&line) {
+            let root = last_header.clone().unwrap();
+            let package = cap.name("package").unwrap().as_str();
+            let version = cap.name("version").unwrap().as_str();
+            let component = cap.name("component").unwrap().as_str();
+            match component {
+                "library" | "executable" => {
+                    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>>>
-where
-    P: AsRef<Path>,
-{
-    let file = File::open(filename)?;
-    Ok(io::BufReader::new(file).lines())
+fn is_reg_match(s: &str, r: &Regex) -> bool {
+    r.captures(s).is_some()
 }