Commit 8be41ace authored by Silvan Mosberger's avatar Silvan Mosberger
Browse files

tests.nixpkgs-check-by-name: Separate file for all problems

And introduce a function for some smaller indentation
parent eac0b690
Loading
Loading
Loading
Loading
+12 −227
Original line number Diff line number Diff line
use crate::utils::PACKAGE_NIX_FILENAME;
use crate::nixpkgs_problem::NixpkgsProblem;
use itertools::concat;
use itertools::{
    Either,
    Either::{Left, Right},
    Itertools,
};
use rnix::parser::ParseError;
use std::ffi::OsString;
use std::fmt;
use std::io;
use std::path::PathBuf;

pub enum CheckProblem {
    ShardNonDir {
        relative_shard_path: PathBuf,
    },
    InvalidShardName {
        relative_shard_path: PathBuf,
        shard_name: String,
    },
    PackageNonDir {
        relative_package_dir: PathBuf,
    },
    CaseSensitiveDuplicate {
        relative_shard_path: PathBuf,
        first: OsString,
        second: OsString,
    },
    InvalidPackageName {
        relative_package_dir: PathBuf,
        package_name: String,
    },
    IncorrectShard {
        relative_package_dir: PathBuf,
        correct_relative_package_dir: PathBuf,
    },
    PackageNixNonExistent {
        relative_package_dir: PathBuf,
    },
    PackageNixDir {
        relative_package_dir: PathBuf,
    },
    UndefinedAttr {
        relative_package_file: PathBuf,
        package_name: String,
    },
    WrongCallPackage {
        relative_package_file: PathBuf,
        package_name: String,
    },
    NonDerivation {
        relative_package_file: PathBuf,
        package_name: String,
    },
    OutsideSymlink {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
    },
    UnresolvableSymlink {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        io_error: io::Error,
    },
    CouldNotParseNix {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        error: ParseError,
    },
    PathInterpolation {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
    },
    SearchPath {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
    },
    OutsidePathReference {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
    },
    UnresolvablePathReference {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
        io_error: io::Error,
    },
}

impl CheckProblem {
    pub fn into_result<A>(self) -> CheckResult<A> {
        Ok(Left(vec![self]))
    }
}

impl fmt::Display for CheckProblem {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            CheckProblem::ShardNonDir { relative_shard_path } =>
                write!(
                    f,
                    "{}: This is a file, but it should be a directory.",
                    relative_shard_path.display(),
                ),
            CheckProblem::InvalidShardName { relative_shard_path, shard_name } =>
                write!(
                    f,
                    "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".",
                    relative_shard_path.display()
                ),
            CheckProblem::PackageNonDir { relative_package_dir } =>
                write!(
                    f,
                    "{}: This path is a file, but it should be a directory.",
                    relative_package_dir.display(),
                ),
            CheckProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
                write!(
                    f,
                    "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.",
                    relative_shard_path.display(),
                ),
            CheckProblem::InvalidPackageName { relative_package_dir, package_name } =>
                write!(
                    f,
                    "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".",
                    relative_package_dir.display(),
                ),
            CheckProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
                write!(
                    f,
                    "{}: Incorrect directory location, should be {} instead.",
                    relative_package_dir.display(),
                    correct_relative_package_dir.display(),
                ),
            CheckProblem::PackageNixNonExistent { relative_package_dir } =>
                write!(
                    f,
                    "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.",
                    relative_package_dir.display(),
                ),
            CheckProblem::PackageNixDir { relative_package_dir } =>
                write!(
                    f,
                    "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.",
                    relative_package_dir.display(),
                ),
            CheckProblem::UndefinedAttr { relative_package_file, package_name } =>
                write!(
                    f,
                    "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
                    relative_package_file.display()
                ),
            CheckProblem::WrongCallPackage { relative_package_file, package_name } =>
                write!(
                    f,
                    "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.",
                    relative_package_file.display()
                ),
            CheckProblem::NonDerivation { relative_package_file, package_name } =>
                write!(
                    f,
                    "pkgs.{package_name}: This attribute defined by {} is not a derivation",
                    relative_package_file.display()
                ),
            CheckProblem::OutsideSymlink { relative_package_dir, subpath } =>
                write!(
                    f,
                    "{}: Path {} is a symlink pointing to a path outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                ),
            CheckProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
                write!(
                    f,
                    "{}: Path {} is a symlink which cannot be resolved: {io_error}.",
                    relative_package_dir.display(),
                    subpath.display(),
                ),
            CheckProblem::CouldNotParseNix { relative_package_dir, subpath, error } =>
                write!(
                    f,
                    "{}: File {} could not be parsed by rnix: {}",
                    relative_package_dir.display(),
                    subpath.display(),
                    error,
                ),
            CheckProblem::PathInterpolation { relative_package_dir, subpath, line, text } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text
                ),
            CheckProblem::SearchPath { relative_package_dir, subpath, line, text } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text
                ),
            CheckProblem::OutsidePathReference { relative_package_dir, subpath, line, text } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text,
                ),
            CheckProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text,
                ),
        }
    }
}

/// A type alias representing the result of a check, either:
/// - Err(anyhow::Error): A fatal failure, typically I/O errors.
///   Such failures are not caused by the files in Nixpkgs.
///   This hints at a bug in the code or a problem with the deployment.
/// - Ok(Left(Vec<CheckProblem>)): A non-fatal problem with the Nixpkgs files.
/// - Ok(Left(Vec<NixpkgsProblem>)): A non-fatal problem with the Nixpkgs files.
///   Further checks can be run even with this result type.
///   Such problems can be fixed by changing the Nixpkgs files.
/// - Ok(Right(A)): A successful (potentially intermediate) result with an arbitrary value.
///   No fatal errors have occurred and no problems have been found with Nixpkgs.
pub type CheckResult<A> = anyhow::Result<Either<Vec<CheckProblem>, A>>;
pub type CheckResult<A> = anyhow::Result<Either<Vec<NixpkgsProblem>, A>>;

/// Map a `CheckResult<I>` to a `CheckResult<O>` by applying a function to the
/// potentially contained value in case of success.
@@ -250,8 +28,15 @@ pub fn ok<A>(value: A) -> CheckResult<A> {
    Ok(Right(value))
}

impl NixpkgsProblem {
    /// Create a `CheckResult<A>` from a single check problem
    pub fn into_result<A>(self) -> CheckResult<A> {
        Ok(Left(vec![self]))
    }
}

/// Combine two check results, both of which need to be successful for the return value to be successful.
/// The `CheckProblem`s of both sides are returned concatenated.
/// The `NixpkgsProblem`s of both sides are returned concatenated.
pub fn and<A>(first: CheckResult<()>, second: CheckResult<A>) -> CheckResult<A> {
    match (first?, second?) {
        (Right(_), Right(right_value)) => Ok(Right(right_value)),
@@ -264,7 +49,7 @@ pub fn and<A>(first: CheckResult<()>, second: CheckResult<A>) -> CheckResult<A>
/// Combine many checks results into a single one.
/// All given check results need to be successful in order for the returned check result to be successful,
/// in which case the returned check result value contains each a `Vec` of each individual result value.
/// The `CheckProblem`s of all results are returned concatenated.
/// The `NixpkgsProblem`s of all results are returned concatenated.
pub fn sequence<A>(check_results: impl IntoIterator<Item = CheckResult<A>>) -> CheckResult<Vec<A>> {
    let (errors, values): (Vec<_>, Vec<_>) = check_results
        .into_iter()
+5 −4
Original line number Diff line number Diff line
use crate::check_result;
use crate::check_result::{CheckProblem, CheckResult};
use crate::check_result::CheckResult;
use crate::nixpkgs_problem::NixpkgsProblem;
use crate::structure;
use crate::Version;
use std::path::Path;
@@ -137,13 +138,13 @@ pub fn check_values(
            };

            if !valid {
                CheckProblem::WrongCallPackage {
                NixpkgsProblem::WrongCallPackage {
                    relative_package_file: relative_package_file.clone(),
                    package_name: package_name.clone(),
                }
                .into_result()
            } else if !attribute_info.is_derivation {
                CheckProblem::NonDerivation {
                NixpkgsProblem::NonDerivation {
                    relative_package_file: relative_package_file.clone(),
                    package_name: package_name.clone(),
                }
@@ -152,7 +153,7 @@ pub fn check_values(
                check_result::ok(())
            }
        } else {
            CheckProblem::UndefinedAttr {
            NixpkgsProblem::UndefinedAttr {
                relative_package_file: relative_package_file.clone(),
                package_name: package_name.clone(),
            }
+1 −0
Original line number Diff line number Diff line
mod check_result;
mod eval;
mod nixpkgs_problem;
mod references;
mod structure;
mod utils;
+218 −0
Original line number Diff line number Diff line
use crate::utils::PACKAGE_NIX_FILENAME;
use rnix::parser::ParseError;
use std::ffi::OsString;
use std::fmt;
use std::io;
use std::path::PathBuf;

/// Any problem that can occur when checking Nixpkgs
pub enum NixpkgsProblem {
    ShardNonDir {
        relative_shard_path: PathBuf,
    },
    InvalidShardName {
        relative_shard_path: PathBuf,
        shard_name: String,
    },
    PackageNonDir {
        relative_package_dir: PathBuf,
    },
    CaseSensitiveDuplicate {
        relative_shard_path: PathBuf,
        first: OsString,
        second: OsString,
    },
    InvalidPackageName {
        relative_package_dir: PathBuf,
        package_name: String,
    },
    IncorrectShard {
        relative_package_dir: PathBuf,
        correct_relative_package_dir: PathBuf,
    },
    PackageNixNonExistent {
        relative_package_dir: PathBuf,
    },
    PackageNixDir {
        relative_package_dir: PathBuf,
    },
    UndefinedAttr {
        relative_package_file: PathBuf,
        package_name: String,
    },
    WrongCallPackage {
        relative_package_file: PathBuf,
        package_name: String,
    },
    NonDerivation {
        relative_package_file: PathBuf,
        package_name: String,
    },
    OutsideSymlink {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
    },
    UnresolvableSymlink {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        io_error: io::Error,
    },
    CouldNotParseNix {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        error: ParseError,
    },
    PathInterpolation {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
    },
    SearchPath {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
    },
    OutsidePathReference {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
    },
    UnresolvablePathReference {
        relative_package_dir: PathBuf,
        subpath: PathBuf,
        line: usize,
        text: String,
        io_error: io::Error,
    },
}

impl fmt::Display for NixpkgsProblem {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            NixpkgsProblem::ShardNonDir { relative_shard_path } =>
                write!(
                    f,
                    "{}: This is a file, but it should be a directory.",
                    relative_shard_path.display(),
                ),
            NixpkgsProblem::InvalidShardName { relative_shard_path, shard_name } =>
                write!(
                    f,
                    "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".",
                    relative_shard_path.display()
                ),
            NixpkgsProblem::PackageNonDir { relative_package_dir } =>
                write!(
                    f,
                    "{}: This path is a file, but it should be a directory.",
                    relative_package_dir.display(),
                ),
            NixpkgsProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
                write!(
                    f,
                    "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.",
                    relative_shard_path.display(),
                ),
            NixpkgsProblem::InvalidPackageName { relative_package_dir, package_name } =>
                write!(
                    f,
                    "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".",
                    relative_package_dir.display(),
                ),
            NixpkgsProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
                write!(
                    f,
                    "{}: Incorrect directory location, should be {} instead.",
                    relative_package_dir.display(),
                    correct_relative_package_dir.display(),
                ),
            NixpkgsProblem::PackageNixNonExistent { relative_package_dir } =>
                write!(
                    f,
                    "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.",
                    relative_package_dir.display(),
                ),
            NixpkgsProblem::PackageNixDir { relative_package_dir } =>
                write!(
                    f,
                    "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.",
                    relative_package_dir.display(),
                ),
            NixpkgsProblem::UndefinedAttr { relative_package_file, package_name } =>
                write!(
                    f,
                    "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
                    relative_package_file.display()
                ),
            NixpkgsProblem::WrongCallPackage { relative_package_file, package_name } =>
                write!(
                    f,
                    "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.",
                    relative_package_file.display()
                ),
            NixpkgsProblem::NonDerivation { relative_package_file, package_name } =>
                write!(
                    f,
                    "pkgs.{package_name}: This attribute defined by {} is not a derivation",
                    relative_package_file.display()
                ),
            NixpkgsProblem::OutsideSymlink { relative_package_dir, subpath } =>
                write!(
                    f,
                    "{}: Path {} is a symlink pointing to a path outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                ),
            NixpkgsProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
                write!(
                    f,
                    "{}: Path {} is a symlink which cannot be resolved: {io_error}.",
                    relative_package_dir.display(),
                    subpath.display(),
                ),
            NixpkgsProblem::CouldNotParseNix { relative_package_dir, subpath, error } =>
                write!(
                    f,
                    "{}: File {} could not be parsed by rnix: {}",
                    relative_package_dir.display(),
                    subpath.display(),
                    error,
                ),
            NixpkgsProblem::PathInterpolation { relative_package_dir, subpath, line, text } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text
                ),
            NixpkgsProblem::SearchPath { relative_package_dir, subpath, line, text } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text
                ),
            NixpkgsProblem::OutsidePathReference { relative_package_dir, subpath, line, text } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text,
                ),
            NixpkgsProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
                write!(
                    f,
                    "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.",
                    relative_package_dir.display(),
                    subpath.display(),
                    text,
                ),
        }
    }
}
+9 −8
Original line number Diff line number Diff line
use crate::check_result;
use crate::check_result::{CheckProblem, CheckResult};
use crate::check_result::CheckResult;
use crate::nixpkgs_problem::NixpkgsProblem;
use crate::utils;
use crate::utils::LineIndex;

@@ -47,7 +48,7 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                // No need to handle the case of it being inside the directory, since we scan through the
                // entire directory recursively anyways
                if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
                    CheckProblem::OutsideSymlink {
                    NixpkgsProblem::OutsideSymlink {
                        relative_package_dir: context.relative_package_dir.clone(),
                        subpath: subpath.to_path_buf(),
                    }
@@ -56,7 +57,7 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                    check_result::ok(())
                }
            }
            Err(io_error) => CheckProblem::UnresolvableSymlink {
            Err(io_error) => NixpkgsProblem::UnresolvableSymlink {
                relative_package_dir: context.relative_package_dir.clone(),
                subpath: subpath.to_path_buf(),
                io_error,
@@ -104,7 +105,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {

    let root = Root::parse(&contents);
    if let Some(error) = root.errors().first() {
        return CheckProblem::CouldNotParseNix {
        return NixpkgsProblem::CouldNotParseNix {
            relative_package_dir: context.relative_package_dir.clone(),
            subpath: subpath.to_path_buf(),
            error: error.clone(),
@@ -124,7 +125,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
        } else if node.children().count() != 0 {
            // Filters out ./foo/${bar}/baz
            // TODO: We can just check ./foo
            CheckProblem::PathInterpolation {
            NixpkgsProblem::PathInterpolation {
                relative_package_dir: context.relative_package_dir.clone(),
                subpath: subpath.to_path_buf(),
                line,
@@ -133,7 +134,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
            .into_result()
        } else if text.starts_with('<') {
            // Filters out search paths like <nixpkgs>
            CheckProblem::SearchPath {
            NixpkgsProblem::SearchPath {
                relative_package_dir: context.relative_package_dir.clone(),
                subpath: subpath.to_path_buf(),
                line,
@@ -149,7 +150,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                    // No need to handle the case of it being inside the directory, since we scan through the
                    // entire directory recursively anyways
                    if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
                        CheckProblem::OutsidePathReference {
                        NixpkgsProblem::OutsidePathReference {
                            relative_package_dir: context.relative_package_dir.clone(),
                            subpath: subpath.to_path_buf(),
                            line,
@@ -160,7 +161,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                        check_result::ok(())
                    }
                }
                Err(e) => CheckProblem::UnresolvablePathReference {
                Err(e) => NixpkgsProblem::UnresolvablePathReference {
                    relative_package_dir: context.relative_package_dir.clone(),
                    subpath: subpath.to_path_buf(),
                    line,
Loading