about summary refs log tree commit diff
path: root/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs
diff options
context:
space:
mode:
Diffstat (limited to 'nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs')
-rw-r--r--nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs109
1 files changed, 68 insertions, 41 deletions
diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs
index 0d0ddcd7e632..dcc9cb9e716d 100644
--- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -1,4 +1,5 @@
 use crate::nix_file::NixFileStore;
+use std::panic;
 mod eval;
 mod nix_file;
 mod nixpkgs_problem;
@@ -17,6 +18,7 @@ use colored::Colorize;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
+use std::thread;
 
 /// Program to check the validity of pkgs/by-name
 ///
@@ -46,15 +48,9 @@ pub struct Args {
 
 fn main() -> ExitCode {
     let args = Args::parse();
-    match process(&args.base, &args.nixpkgs, false, &mut io::stderr()) {
-        Ok(true) => {
-            eprintln!("{}", "Validated successfully".green());
-            ExitCode::SUCCESS
-        }
-        Ok(false) => {
-            eprintln!("{}", "Validation failed, see above errors".yellow());
-            ExitCode::from(1)
-        }
+    match process(args.base, args.nixpkgs, false, &mut io::stderr()) {
+        Ok(true) => ExitCode::SUCCESS,
+        Ok(false) => ExitCode::from(1),
         Err(e) => {
             eprintln!("{} {:#}", "I/O error: ".yellow(), e);
             ExitCode::from(2)
@@ -77,34 +73,66 @@ fn main() -> ExitCode {
 /// - `Ok(false)` if there are problems, all of which will be written to `error_writer`.
 /// - `Ok(true)` if there are no problems
 pub fn process<W: io::Write>(
-    base_nixpkgs: &Path,
-    main_nixpkgs: &Path,
+    base_nixpkgs: PathBuf,
+    main_nixpkgs: PathBuf,
     keep_nix_path: bool,
     error_writer: &mut W,
 ) -> anyhow::Result<bool> {
-    // Check the main Nixpkgs first
-    let main_result = check_nixpkgs(main_nixpkgs, keep_nix_path, error_writer)?;
-    let check_result = main_result.result_map(|nixpkgs_version| {
-        // If the main Nixpkgs doesn't have any problems, run the ratchet checks against the base
-        // Nixpkgs
-        check_nixpkgs(base_nixpkgs, keep_nix_path, error_writer)?.result_map(
-            |base_nixpkgs_version| {
-                Ok(ratchet::Nixpkgs::compare(
-                    base_nixpkgs_version,
-                    nixpkgs_version,
-                ))
-            },
-        )
-    })?;
+    // Very easy to parallelise this, since it's totally independent
+    let base_thread = thread::spawn(move || check_nixpkgs(&base_nixpkgs, keep_nix_path));
+    let main_result = check_nixpkgs(&main_nixpkgs, keep_nix_path)?;
+
+    let base_result = match base_thread.join() {
+        Ok(res) => res?,
+        Err(e) => panic::resume_unwind(e),
+    };
 
-    match check_result {
-        Failure(errors) => {
+    match (base_result, main_result) {
+        (Failure(_), Failure(errors)) => {
+            // Base branch fails and the PR doesn't fix it and may also introduce additional problems
             for error in errors {
                 writeln!(error_writer, "{}", error.to_string().red())?
             }
+            writeln!(error_writer, "{}", "The base branch is broken and still has above problems with this PR, which need to be fixed first.\nConsider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.".yellow())?;
             Ok(false)
         }
-        Success(()) => Ok(true),
+        (Failure(_), Success(_)) => {
+            writeln!(
+                error_writer,
+                "{}",
+                "The base branch is broken, but this PR fixes it. Nice job!".green()
+            )?;
+            Ok(true)
+        }
+        (Success(_), Failure(errors)) => {
+            for error in errors {
+                writeln!(error_writer, "{}", error.to_string().red())?
+            }
+            writeln!(
+                error_writer,
+                "{}",
+                "This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break."
+                    .yellow()
+            )?;
+            Ok(false)
+        }
+        (Success(base), Success(main)) => {
+            // Both base and main branch succeed, check ratchet state
+            match ratchet::Nixpkgs::compare(base, main) {
+                Failure(errors) => {
+                    for error in errors {
+                        writeln!(error_writer, "{}", error.to_string().red())?
+                    }
+                    writeln!(error_writer, "{}", "This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.".yellow())?;
+
+                    Ok(false)
+                }
+                Success(()) => {
+                    writeln!(error_writer, "{}", "Validated successfully".green())?;
+                    Ok(true)
+                }
+            }
+        }
     }
 }
 
@@ -113,10 +141,9 @@ pub fn process<W: io::Write>(
 /// This does not include ratchet checks, see ../README.md#ratchet-checks
 /// Instead a `ratchet::Nixpkgs` value is returned, whose `compare` method allows performing the
 /// ratchet check against another result.
-pub fn check_nixpkgs<W: io::Write>(
+pub fn check_nixpkgs(
     nixpkgs_path: &Path,
     keep_nix_path: bool,
-    error_writer: &mut W,
 ) -> validation::Result<ratchet::Nixpkgs> {
     let mut nix_file_store = NixFileStore::default();
 
@@ -129,11 +156,7 @@ pub fn check_nixpkgs<W: io::Write>(
         })?;
 
         if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() {
-            writeln!(
-                error_writer,
-                "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
-                utils::BASE_SUBPATH
-            )?;
+            // No pkgs/by-name directory, always valid
             Success(ratchet::Nixpkgs::default())
         } else {
             check_structure(&nixpkgs_path, &mut nix_file_store)?.result_map(|package_names|
@@ -163,8 +186,8 @@ mod tests {
                 continue;
             }
 
-            let expected_errors =
-                fs::read_to_string(path.join("expected")).unwrap_or(String::new());
+            let expected_errors = fs::read_to_string(path.join("expected"))
+                .expect("No expected file for test {name}");
 
             test_nixpkgs(&name, &path, &expected_errors)?;
         }
@@ -201,7 +224,7 @@ mod tests {
         test_nixpkgs(
             "case_sensitive",
             &path,
-            "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\n",
+            "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\nThis PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.\n",
         )?;
 
         Ok(())
@@ -225,7 +248,11 @@ mod tests {
         let tmpdir = temp_root.path().join("symlinked");
 
         temp_env::with_var("TMPDIR", Some(&tmpdir), || {
-            test_nixpkgs("symlinked_tmpdir", Path::new("tests/success"), "")
+            test_nixpkgs(
+                "symlinked_tmpdir",
+                Path::new("tests/success"),
+                "Validated successfully\n",
+            )
         })
     }
 
@@ -240,7 +267,7 @@ mod tests {
         // We don't want coloring to mess up the tests
         let writer = temp_env::with_var("NO_COLOR", Some("1"), || -> anyhow::Result<_> {
             let mut writer = vec![];
-            process(base_nixpkgs, &path, true, &mut writer)
+            process(base_nixpkgs.to_owned(), path.to_owned(), true, &mut writer)
                 .with_context(|| format!("Failed test case {name}"))?;
             Ok(writer)
         })?;
@@ -249,7 +276,7 @@ mod tests {
 
         if actual_errors != expected_errors {
             panic!(
-                "Failed test case {name}, expected these errors:\n\n{}\n\nbut got these:\n\n{}",
+                "Failed test case {name}, expected these errors:\n=======\n{}\n=======\nbut got these:\n=======\n{}\n=======",
                 expected_errors, actual_errors
             );
         }