tests.nixpkgs-check-by-name: Refactor eval.rs

There was too much indentation!
This commit is contained in:
Silvan Mosberger 2024-02-05 01:35:23 +01:00
parent 37a54e3ad5
commit 474e7e7040

View file

@ -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 `<attr> = callPackage <arg1> <arg2>;`,
// 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<ratchet::Package> {
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<ratchet::Package> {
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 `<attr> = callPackage <arg1> <arg2>;`,
// 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,
})
}
})
}