From 3163a84b971a90c4b92296d9ee483bc15e561143 Mon Sep 17 00:00:00 2001 From: Adam Bergmark Date: Tue, 13 Jul 2021 17:16:06 +0200 Subject: [PATCH] Simpler `commenter` flow, allow piping stderr from `curator` --- CURATORS.md | 32 ++++++++++---- etc/commenter/Cargo.lock | 67 +++++++++++++++++++++++++++++ etc/commenter/Cargo.toml | 1 + etc/commenter/README.md | 2 + etc/commenter/src/main.rs | 89 +++++++++++++++++++-------------------- 5 files changed, 138 insertions(+), 53 deletions(-) diff --git a/CURATORS.md b/CURATORS.md index f449d1f9..834fa30b 100644 --- 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 diff --git a/etc/commenter/Cargo.lock b/etc/commenter/Cargo.lock index 697797d2..3b623a76 100644 --- 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" diff --git a/etc/commenter/Cargo.toml b/etc/commenter/Cargo.toml index 12ef031f..87d6f56b 100644 --- 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" diff --git a/etc/commenter/README.md b/etc/commenter/README.md index 6e505cc8..29b7ed7e 100644 --- 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. diff --git a/etc/commenter/src/main.rs b/etc/commenter/src/main.rs index 66ff23c9..9a2cb491 100644 --- 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>; @@ -18,43 +17,47 @@ fn main() { let mut tests: H = Default::default(); let mut benches: H = Default::default(); let mut last_header: Option
= None; - let empty = Regex::new(r#"^\s*$"#).unwrap(); - let header_versioned = - Regex::new(r#"^(?P[a-zA-z]([a-zA-z0-9.-]*?))-(?P(\d+(\.\d+)*)).+?is out of bounds for:$"#).unwrap(); - let header_missing = - Regex::new(r#"^(?P[a-zA-z]([a-zA-z0-9.-]*)).+?depended on by:$"#).unwrap(); - let package = - Regex::new(r#"^- \[ \] (?P[a-zA-z]([a-zA-z0-9.-]*?))-(?P(\d+(\.\d+)*)).+?Used by: (?P.+)$"#) - .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[a-zA-z]([a-zA-z0-9.-]*?))-(?P(\d+(\.\d+)*)).+?is out of bounds for:$"# + ); + let header_missing = regex!(r#"^(?P[a-zA-z]([a-zA-z0-9.-]*)).+?depended on by:$"#); + let package = regex!( + r#"^- \[ \] (?P[a-zA-z]([a-zA-z0-9.-]*?))-(?P(\d+(\.\d+)*)).+?Used by: (?P.+)$"# + ); + + // 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

(filename: P) -> io::Result>> -where - P: AsRef, -{ - 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() }