diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index bdde0e560057..cb49c3acbef3 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -159,184 +159,14 @@ pub fn check_values( attributes .into_iter() .map(|(attribute_name, attribute_value)| { - let relative_package_file = structure::relative_file_for_package(&attribute_name); - - use ratchet::RatchetState::*; - use Attribute::*; - use AttributeInfo::*; - use ByNameAttribute::*; - use CallPackageVariant::*; - use NonByNameAttribute::*; - let check_result = match attribute_value { - // The attribute succeeds evaluation and is NOT defined in pkgs/by-name - NonByName(EvalSuccess(attribute_info)) => { - 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(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 { - call_package_variant: Auto, - .. - }) => { - // With the current detection mechanism, this also triggers for aliases - // to pkgs/by-name packages, and there's no good method of - // distinguishing alias vs non-alias. - // Using `config.allowAliases = false` at least currently doesn't work - // because there's nothing preventing people from defining aliases that - // are present even with that disabled. - // 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(NonApplicable) - } - // Only derivations can be in pkgs/by-name, - // so this attribute doesn't qualify - CallPackage(CallPackageInfo { - is_derivation: false, - .. - }) => Success(NonApplicable), - // A location of None indicates something weird, we can't really know where - // this attribute is defined, probably an alias - CallPackage(CallPackageInfo { location: None, .. }) => Success(Tight), - // The case of an attribute that qualifies: - // - Uses callPackage - // - Is a derivation - CallPackage(CallPackageInfo { - is_derivation: true, - call_package_variant: Manual { .. }, - location: Some(location), - }) => - // We'll use the attribute's location to parse the file that defines it - { - match nix_file_store - .get(&location.file)? - .call_package_argument_info_at( - location.line, - location.column, - nixpkgs_path, - )? { - // If the definition is not of the form ` = callPackage ;`, - // it's generally not possible to migrate to `pkgs/by-name` - None => Success(NonApplicable), - Some(call_package_argument_info) => { - if let Some(ref rel_path) = - call_package_argument_info.relative_path - { - if rel_path.starts_with(utils::BASE_SUBPATH) { - // Package variants of by-name packages are explicitly allowed according to RFC 140 - // https://github.com/NixOS/rfcs/blob/master/rfcs/0140-simple-package-paths.md#package-variants: - // - // foo-variant = callPackage ../by-name/fo/foo/package.nix { - // someFlag = true; - // } - // - // While such definitions could be moved to `pkgs/by-name` by using - // `.override { someFlag = true; }` instead, this changes the semantics in - // relation with overlays. - Success(NonApplicable) - } else { - Success(Loose(call_package_argument_info)) - } - } else { - Success(Loose(call_package_argument_info)) - } - } - } - } - }; - uses_by_name.map(|x| ratchet::Package { - manual_definition: Tight, - uses_by_name: x, - }) - } - NonByName(EvalFailure) => { - // We don't know anything about this attribute really - Success(ratchet::Package { - // 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 { - relative_package_file: relative_package_file.clone(), - package_name: attribute_name.clone(), - } - .into(), - ByName(Existing(NonAttributeSet)) => NixpkgsProblem::NonDerivation { - relative_package_file: relative_package_file.clone(), - package_name: attribute_name.clone(), - } - .into(), - ByName(Existing(NonCallPackage)) => NixpkgsProblem::WrongCallPackage { - relative_package_file: relative_package_file.clone(), - package_name: attribute_name.clone(), - } - .into(), - ByName(Existing(CallPackage(CallPackageInfo { - is_derivation, - call_package_variant, - .. - }))) => { - let check_result = if !is_derivation { - NixpkgsProblem::NonDerivation { - relative_package_file: relative_package_file.clone(), - package_name: attribute_name.clone(), - } - .into() - } else { - Success(()) - }; - - check_result.and(match &call_package_variant { - Auto => Success(ratchet::Package { - manual_definition: Tight, - uses_by_name: Tight, - }), - // TODO: Use the call_package_argument_info_at instead/additionally and - // simplify the eval.nix code - Manual { path, empty_arg } => { - let correct_file = if let Some(call_package_path) = path { - relative_package_file == *call_package_path - } else { - false - }; - - if correct_file { - Success(ratchet::Package { - // Empty arguments for non-auto-called packages are not allowed anymore. - manual_definition: if *empty_arg { - Loose(()) - } else { - Tight - }, - uses_by_name: Tight, - }) - } else { - NixpkgsProblem::WrongCallPackage { - relative_package_file: relative_package_file.clone(), - package_name: attribute_name.clone(), - } - .into() - } - } - }) + Attribute::NonByName(non_by_name_attribute) => handle_non_by_name_attribute( + nixpkgs_path, + nix_file_store, + non_by_name_attribute, + )?, + Attribute::ByName(by_name_attribute) => { + by_name(&attribute_name, by_name_attribute) } }; Ok::<_, anyhow::Error>(check_result.map(|value| (attribute_name.clone(), value))) @@ -349,3 +179,199 @@ pub fn check_values( package_map: elems.into_iter().collect(), })) } + +/// Handles the evaluation result for an attribute in `pkgs/by-name`, +/// turning it into a validation result. +fn by_name( + attribute_name: &str, + by_name_attribute: ByNameAttribute, +) -> validation::Validation { + use ratchet::RatchetState::*; + use AttributeInfo::*; + use ByNameAttribute::*; + use CallPackageVariant::*; + + let relative_package_file = structure::relative_file_for_package(attribute_name); + + match by_name_attribute { + Missing => NixpkgsProblem::UndefinedAttr { + relative_package_file: relative_package_file.to_owned(), + package_name: attribute_name.to_owned(), + } + .into(), + Existing(NonAttributeSet) => NixpkgsProblem::NonDerivation { + relative_package_file: relative_package_file.to_owned(), + package_name: attribute_name.to_owned(), + } + .into(), + Existing(NonCallPackage) => NixpkgsProblem::WrongCallPackage { + relative_package_file: relative_package_file.to_owned(), + package_name: attribute_name.to_owned(), + } + .into(), + Existing(CallPackage(CallPackageInfo { + is_derivation, + call_package_variant, + .. + })) => { + let check_result = if !is_derivation { + NixpkgsProblem::NonDerivation { + relative_package_file: relative_package_file.to_owned(), + package_name: attribute_name.to_owned(), + } + .into() + } else { + Success(()) + }; + + check_result.and(match &call_package_variant { + Auto => Success(ratchet::Package { + manual_definition: Tight, + uses_by_name: Tight, + }), + // TODO: Use the call_package_argument_info_at instead/additionally and + // simplify the eval.nix code + Manual { path, empty_arg } => { + let correct_file = if let Some(call_package_path) = path { + relative_package_file == *call_package_path + } else { + false + }; + + if correct_file { + Success(ratchet::Package { + // Empty arguments for non-auto-called packages are not allowed anymore. + manual_definition: if *empty_arg { Loose(()) } else { Tight }, + uses_by_name: Tight, + }) + } else { + NixpkgsProblem::WrongCallPackage { + relative_package_file: relative_package_file.to_owned(), + package_name: attribute_name.to_owned(), + } + .into() + } + } + }) + } + } +} + +/// Handles the evaluation result for an attribute _not_ in `pkgs/by-name`, +/// turning it into a validation result. +fn handle_non_by_name_attribute( + nixpkgs_path: &Path, + nix_file_store: &mut NixFileStore, + non_by_name_attribute: NonByNameAttribute, +) -> validation::Result { + use ratchet::RatchetState::*; + use AttributeInfo::*; + use CallPackageVariant::*; + use NonByNameAttribute::*; + + Ok(match non_by_name_attribute { + // The attribute succeeds evaluation and is NOT defined in pkgs/by-name + EvalSuccess(attribute_info) => { + 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(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 { + call_package_variant: Auto, + .. + }) => { + // With the current detection mechanism, this also triggers for aliases + // to pkgs/by-name packages, and there's no good method of + // distinguishing alias vs non-alias. + // Using `config.allowAliases = false` at least currently doesn't work + // because there's nothing preventing people from defining aliases that + // are present even with that disabled. + // 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(NonApplicable) + } + // Only derivations can be in pkgs/by-name, + // so this attribute doesn't qualify + CallPackage(CallPackageInfo { + is_derivation: false, + .. + }) => Success(NonApplicable), + // A location of None indicates something weird, we can't really know where + // this attribute is defined, probably an alias + CallPackage(CallPackageInfo { location: None, .. }) => Success(Tight), + // The case of an attribute that qualifies: + // - Uses callPackage + // - Is a derivation + CallPackage(CallPackageInfo { + is_derivation: true, + call_package_variant: Manual { .. }, + location: Some(location), + }) => + // We'll use the attribute's location to parse the file that defines it + { + match nix_file_store + .get(&location.file)? + .call_package_argument_info_at( + location.line, + location.column, + nixpkgs_path, + )? { + // If the definition is not of the form ` = callPackage ;`, + // it's generally not possible to migrate to `pkgs/by-name` + None => Success(NonApplicable), + Some(call_package_argument_info) => { + if let Some(ref rel_path) = call_package_argument_info.relative_path { + if rel_path.starts_with(utils::BASE_SUBPATH) { + // Package variants of by-name packages are explicitly allowed according to RFC 140 + // https://github.com/NixOS/rfcs/blob/master/rfcs/0140-simple-package-paths.md#package-variants: + // + // foo-variant = callPackage ../by-name/fo/foo/package.nix { + // someFlag = true; + // } + // + // While such definitions could be moved to `pkgs/by-name` by using + // `.override { someFlag = true; }` instead, this changes the semantics in + // relation with overlays. + Success(NonApplicable) + } else { + Success(Loose(call_package_argument_info)) + } + } else { + Success(Loose(call_package_argument_info)) + } + } + } + } + }; + uses_by_name.map(|x| ratchet::Package { + manual_definition: Tight, + uses_by_name: x, + }) + } + EvalFailure => { + // We don't know anything about this attribute really + Success(ratchet::Package { + // 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, + }) + } + }) +}