diff options
Diffstat (limited to 'nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs')
-rw-r--r-- | nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs | 141 |
1 files changed, 101 insertions, 40 deletions
diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs index c12f1ead2540..f8c129626cc0 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs @@ -6,80 +6,141 @@ use crate::nixpkgs_problem::NixpkgsProblem; use crate::structure; use crate::validation::{self, Validation, Validation::Success}; use std::collections::HashMap; +use std::path::PathBuf; /// The ratchet value for the entirety of Nixpkgs. #[derive(Default)] pub struct Nixpkgs { - /// The ratchet values for each package in `pkgs/by-name` - pub packages: HashMap<String, Package>, + /// Sorted list of packages in package_map + pub package_names: Vec<String>, + /// The ratchet values for all packages + pub package_map: HashMap<String, Package>, } impl Nixpkgs { /// Validates the ratchet checks for Nixpkgs - pub fn compare(optional_from: Option<Self>, to: Self) -> Validation<()> { + pub fn compare(from: Self, to: Self) -> Validation<()> { validation::sequence_( // We only loop over the current attributes, // we don't need to check ones that were removed - to.packages.into_iter().map(|(name, attr_to)| { - let attr_from = if let Some(from) = &optional_from { - from.packages.get(&name) - } else { - // This pretends that if there's no base version to compare against, all - // attributes existed without conforming to the new strictness check for - // backwards compatibility. - // TODO: Remove this case. This is only needed because the `--base` - // argument is still optional, which doesn't need to be once CI is updated - // to pass it. - Some(&Package { - empty_non_auto_called: EmptyNonAutoCalled::Invalid, - }) - }; - Package::compare(&name, attr_from, &attr_to) + to.package_names.into_iter().map(|name| { + Package::compare(&name, from.package_map.get(&name), &to.package_map[&name]) }), ) } } -/// The ratchet value for a single package in `pkgs/by-name` +/// The ratchet value for a top-level package 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<EmptyNonAutoCalled>, + + /// The ratchet value for the check for new packages using pkgs/by-name + pub uses_by_name: RatchetState<UsesByName>, } impl Package { - /// Validates the ratchet checks for a single package defined in `pkgs/by-name` + /// Validates the ratchet checks for a top-level package pub fn compare(name: &str, optional_from: Option<&Self>, to: &Self) -> Validation<()> { - EmptyNonAutoCalled::compare( - name, - optional_from.map(|x| &x.empty_non_auto_called), - &to.empty_non_auto_called, - ) + validation::sequence_([ + RatchetState::<EmptyNonAutoCalled>::compare( + name, + optional_from.map(|x| &x.empty_non_auto_called), + &to.empty_non_auto_called, + ), + RatchetState::<UsesByName>::compare( + name, + optional_from.map(|x| &x.uses_by_name), + &to.uses_by_name, + ), + ]) } } -/// The ratchet value of a single package in `pkgs/by-name` +/// The ratchet state of a generic ratchet check. +pub enum RatchetState<Context> { + /// 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 not 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 that can convert an attribute-specific error context into a NixpkgsProblem +pub trait ToNixpkgsProblem { + /// How to convert an attribute-specific error context into a NixpkgsProblem + fn to_nixpkgs_problem(name: &str, context: &Self, existed_before: bool) -> NixpkgsProblem; +} + +impl<Context: ToNixpkgsProblem> RatchetState<Context> { + /// 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<&Self>, to: &Self) -> 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)) => { + Context::to_nixpkgs_problem(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 ToNixpkgsProblem for EmptyNonAutoCalled { + fn to_nixpkgs_problem(name: &str, _context: &Self, _existed_before: bool) -> NixpkgsProblem { + NixpkgsProblem::WrongCallPackage { + relative_package_file: structure::relative_file_for_package(name), + package_name: name.to_owned(), + } + } } -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(()) +/// The ratchet value of an attribute +/// for the check that new packages use pkgs/by-name +/// +/// This checks that all new package defined using callPackage must be defined via pkgs/by-name +/// It also checks that once a package uses pkgs/by-name, it can't switch back to all-packages.nix +#[derive(Clone)] +pub struct UsesByName { + /// The first callPackage argument, used for better errors + pub call_package_path: Option<PathBuf>, + /// Whether the second callPackage argument is empty, used for better errors + pub empty_arg: bool, +} + +impl ToNixpkgsProblem for UsesByName { + fn to_nixpkgs_problem(name: &str, a: &Self, existed_before: bool) -> NixpkgsProblem { + if existed_before { + NixpkgsProblem::MovedOutOfByName { + package_name: name.to_owned(), + call_package_path: a.call_package_path.clone(), + empty_arg: a.empty_arg, + } } else { - NixpkgsProblem::WrongCallPackage { - relative_package_file: structure::relative_file_for_package(name), + NixpkgsProblem::NewPackageNotUsingByName { package_name: name.to_owned(), + call_package_path: a.call_package_path.clone(), + empty_arg: a.empty_arg, } - .into() } } } |