diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 4e8fba549386..00bdc0bc9f39 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -130,6 +130,7 @@ pub fn check_values( let relative_package_file = structure::relative_file_for_package(&attribute_name); let absolute_package_file = nixpkgs_path.join(&relative_package_file); + use ratchet::RatchetState::*; use AttributeInfo::*; use ByNameAttribute::*; use CallPackageVariant::*; @@ -166,7 +167,7 @@ pub fn check_values( check_result.and(match &call_package_variant { Auto => Success(ratchet::Package { - empty_non_auto_called: ratchet::EmptyNonAutoCalled::Valid, + empty_non_auto_called: Tight, }), Manual { path, empty_arg } => { let correct_file = if let Some(call_package_path) = path { @@ -179,9 +180,9 @@ pub fn check_values( Success(ratchet::Package { // Empty arguments for non-auto-called packages are not allowed anymore. empty_non_auto_called: if *empty_arg { - ratchet::EmptyNonAutoCalled::Invalid + Loose(ratchet::EmptyNonAutoCalled) } else { - ratchet::EmptyNonAutoCalled::Valid + Tight }, }) } else { diff --git a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs index 3123f7e0e532..c363f85466e9 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs @@ -30,7 +30,7 @@ impl Nixpkgs { /// The ratchet value for a single package in `pkgs/by-name` pub struct Package { /// The ratchet value for the check for non-auto-called empty arguments - pub empty_non_auto_called: EmptyNonAutoCalled, + pub empty_non_auto_called: RatchetState, } impl Package { @@ -44,29 +44,61 @@ impl Package { } } -/// The ratchet value of a single package in `pkgs/by-name` +/// The ratchet state of a generic ratchet check. +pub enum RatchetState { + /// The ratchet is loose, it can be tightened more. + /// In other words, this is the legacy state we're trying to move away from. + /// Introducing new instances is now allowed but previous instances will continue to be allowed. + /// The `Context` is context for error messages in case a new instance of this state is + /// introduced + Loose(Context), + /// The ratchet is tight, it can't be tightened any further. + /// This is either because we already use the latest state, or because the ratchet isn't + /// relevant. + Tight, +} + +/// A trait for a specific ratchet check for an attribute. +trait AttributeRatchet: Sized { + /// What error to produce in case the ratchet went in the wrong direction + fn to_error(name: &str, context: &Self, existed_before: bool) -> NixpkgsProblem; + + /// Compare the previous ratchet state of an attribute to the new state. + /// The previous state may be `None` in case the attribute is new. + fn compare( + name: &str, + optional_from: Option<&RatchetState>, + to: &RatchetState, + ) -> Validation<()> { + // If we don't have a previous state, enforce a tight ratchet + let from = optional_from.unwrap_or(&RatchetState::Tight); + match (from, to) { + // Always okay to keep it tight or tighten the ratchet + (_, RatchetState::Tight) => Success(()), + + // Grandfathering policy for a loose ratchet + (RatchetState::Loose { .. }, RatchetState::Loose { .. }) => Success(()), + + // Loosening a ratchet is now allowed + (RatchetState::Tight, RatchetState::Loose(context)) => { + Self::to_error(name, context, optional_from.is_some()).into() + } + } + } +} + +/// The ratchet value of an attribute /// for the non-auto-called empty argument check of a single. /// /// This checks that packages defined in `pkgs/by-name` cannot be overridden /// with an empty second argument like `callPackage ... { }`. -#[derive(PartialEq, PartialOrd)] -pub enum EmptyNonAutoCalled { - Invalid, - Valid, -} +pub struct EmptyNonAutoCalled; -impl EmptyNonAutoCalled { - /// Validates the non-auto-called empty argument ratchet check for a single package defined in `pkgs/by-name` - fn compare(name: &str, optional_from: Option<&Self>, to: &Self) -> Validation<()> { - let from = optional_from.unwrap_or(&Self::Valid); - if to >= from { - Success(()) - } else { - NixpkgsProblem::WrongCallPackage { - relative_package_file: structure::relative_file_for_package(name), - package_name: name.to_owned(), - } - .into() +impl AttributeRatchet for EmptyNonAutoCalled { + fn to_error(name: &str, _context: &Self, _existed_before: bool) -> NixpkgsProblem { + NixpkgsProblem::WrongCallPackage { + relative_package_file: structure::relative_file_for_package(name), + package_name: name.to_owned(), } } }