about summary refs log tree commit diff
path: root/pkgs/test/nixpkgs-check-by-name/src/eval.rs
diff options
context:
space:
mode:
Diffstat (limited to 'pkgs/test/nixpkgs-check-by-name/src/eval.rs')
-rw-r--r--pkgs/test/nixpkgs-check-by-name/src/eval.rs68
1 files changed, 31 insertions, 37 deletions
diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
index c4dad8257cf98..dd30cb9045e53 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -2,8 +2,6 @@ use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::ratchet;
 use crate::structure;
 use crate::validation::{self, Validation::Success};
-use std::collections::HashMap;
-use std::ffi::OsString;
 use std::path::Path;
 
 use anyhow::Context;
@@ -73,7 +71,7 @@ enum CallPackageVariant {
 pub fn check_values(
     nixpkgs_path: &Path,
     package_names: Vec<String>,
-    eval_nix_path: &HashMap<String, PathBuf>,
+    keep_nix_path: bool,
 ) -> validation::Result<ratchet::Nixpkgs> {
     // Write the list of packages we need to check into a temporary JSON file.
     // This can then get read by the Nix evaluation.
@@ -99,8 +97,6 @@ pub fn check_values(
     command
         // Inherit stderr so that error messages always get shown
         .stderr(process::Stdio::inherit())
-        // Clear NIX_PATH to be sure it doesn't influence the result
-        .env_remove("NIX_PATH")
         .args([
             "--eval",
             "--json",
@@ -121,15 +117,12 @@ pub fn check_values(
         .arg("-I")
         .arg(nixpkgs_path);
 
-    // Also add extra paths that need to be accessible
-    for (name, path) in eval_nix_path {
-        command.arg("-I");
-        let mut name_value = OsString::new();
-        name_value.push(name);
-        name_value.push("=");
-        name_value.push(path);
-        command.arg(name_value);
+    // Clear NIX_PATH to be sure it doesn't influence the result
+    // But not when requested to keep it, used so that the tests can pass extra Nix files
+    if !keep_nix_path {
+        command.env_remove("NIX_PATH");
     }
+
     command.args(["-I", &expr_path]);
     command.arg(expr_path);
 
@@ -166,8 +159,8 @@ pub fn check_values(
                     let uses_by_name = match attribute_info {
                         // In these cases the package doesn't qualify for being in pkgs/by-name,
                         // so the UsesByName ratchet is already as tight as it can be
-                        NonAttributeSet => Success(Tight),
-                        NonCallPackage => Success(Tight),
+                        NonAttributeSet => Success(NonApplicable),
+                        NonCallPackage => Success(NonApplicable),
                         // This is the case when the `pkgs/by-name`-internal _internalCallByNamePackageFile
                         // is used for a package outside `pkgs/by-name`
                         CallPackage(CallPackageInfo {
@@ -183,14 +176,14 @@ pub fn check_values(
                             // In the future we could kind of abuse this behavior to have better
                             // enforcement of conditional aliases, but for now we just need to not
                             // give an error.
-                            Success(Tight)
+                            Success(NonApplicable)
                         }
                         // Only derivations can be in pkgs/by-name,
                         // so this attribute doesn't qualify
                         CallPackage(CallPackageInfo {
                             is_derivation: false,
                             ..
-                        }) => Success(Tight),
+                        }) => Success(NonApplicable),
 
                         // The case of an attribute that qualifies:
                         // - Uses callPackage
@@ -198,30 +191,35 @@ pub fn check_values(
                         CallPackage(CallPackageInfo {
                             is_derivation: true,
                             call_package_variant: Manual { path, empty_arg },
-                        }) => Success(Loose(ratchet::UsesByName {
+                        }) => Success(Loose(ratchet::CouldUseByName {
                             call_package_path: path,
                             empty_arg,
                         })),
                     };
                     uses_by_name.map(|x| ratchet::Package {
-                        empty_non_auto_called: Tight,
+                        manual_definition: Tight,
                         uses_by_name: x,
                     })
                 }
                 NonByName(EvalFailure) => {
-                    // This is a bit of an odd case: We don't even _know_ whether this attribute
-                    // would qualify for using pkgs/by-name. We can either:
-                    // - Assume it's not using pkgs/by-name, which has the problem that if a
-                    //   package evaluation gets broken temporarily, the fix can remove it from
-                    //   pkgs/by-name again
-                    // - Assume it's using pkgs/by-name already, which has the problem that if a
-                    //   package evaluation gets broken temporarily, fixing it requires a move to
-                    //   pkgs/by-name
-                    // We choose the latter, since we want to move towards pkgs/by-name, not away
-                    // from it
+                    // We don't know anything about this attribute really
                     Success(ratchet::Package {
-                        empty_non_auto_called: Tight,
-                        uses_by_name: Tight,
+                        // We'll assume that we can't remove any manual definitions, which has the
+                        // minimal drawback that if there was a manual definition that could've
+                        // been removed, fixing the package requires removing the definition, no
+                        // big deal, that's a minor edit.
+                        manual_definition: Tight,
+
+                        // Regarding whether this attribute could `pkgs/by-name`, we don't really
+                        // know, so return NonApplicable, which has the effect that if a
+                        // package evaluation gets broken temporarily, the fix can remove it from
+                        // pkgs/by-name again. For now this isn't our problem, but in the future we
+                        // might have another check to enforce that evaluation must not be broken.
+                        // The alternative of assuming that it's using `pkgs/by-name` already
+                        // has the problem that if a package evaluation gets broken temporarily,
+                        // fixing it requires a move to pkgs/by-name, which could happen more
+                        // often and isn't really justified.
+                        uses_by_name: NonApplicable,
                     })
                 }
                 ByName(Missing) => NixpkgsProblem::UndefinedAttr {
@@ -255,7 +253,7 @@ pub fn check_values(
 
                     check_result.and(match &call_package_variant {
                         Auto => Success(ratchet::Package {
-                            empty_non_auto_called: Tight,
+                            manual_definition: Tight,
                             uses_by_name: Tight,
                         }),
                         Manual { path, empty_arg } => {
@@ -268,11 +266,7 @@ pub fn check_values(
                             if correct_file {
                                 Success(ratchet::Package {
                                     // Empty arguments for non-auto-called packages are not allowed anymore.
-                                    empty_non_auto_called: if *empty_arg {
-                                        Loose(ratchet::EmptyNonAutoCalled)
-                                    } else {
-                                        Tight
-                                    },
+                                    manual_definition: if *empty_arg { Loose(()) } else { Tight },
                                     uses_by_name: Tight,
                                 })
                             } else {