Merge pull request #256792 from tweag/by-name-empty-args (e0e16540) · Commits · nix / nixpkgs

pkgs/test/nixpkgs-check-by-name/README.md

+8 −0

Original line number	Diff line number	Diff line
		@@ -11,6 +11,13 @@ This API may be changed over time if the CI workflow making use of it is adjuste
		- Command line: `nixpkgs-check-by-name <NIXPKGS>`
		- Arguments:
		- `<NIXPKGS>`: The path to the Nixpkgs to check
		- `--version <VERSION>`: The version of the checks to perform.

		Possible values:
		- `v0` (default)
		- `v1`

		See [validation](#validity-checks) for the differences.
		- Exit code:
		- `0`: If the [validation](#validity-checks) is successful
		- `1`: If the [validation](#validity-checks) is not successful
		@@ -35,6 +42,7 @@ These checks are performed by this tool:

		### Nix evaluation checks
		- `pkgs.${name}` is defined as `callPackage pkgs/by-name/${shard}/${name}/package.nix args` for some `args`.
		- Only after --version v1: If `pkgs.${name}` is not auto-called from `pkgs/by-name`, `args` must not be empty
		- `pkgs.lib.isDerivation pkgs.${name}` is `true`.

		## Development

pkgs/test/nixpkgs-check-by-name/src/eval.nix

+35 −12

Original line number	Diff line number	Diff line
		@@ -18,19 +18,42 @@ let
		callPackage = fn: args:
		let
		result = super.callPackage fn args;
		variantInfo._attributeVariant = {
		# These names are used by the deserializer on the Rust side
		CallPackage.path =
		if builtins.isPath fn then
		toString fn
		else
		null;
		CallPackage.empty_arg =
		args == { };
		};
		in
		if builtins.isAttrs result then
		# If this was the last overlay to be applied, we could just only return the `_callPackagePath`,
		# but that's not the case because stdenv has another overlays on top of user-provided ones.
		# So to not break the stdenv build we need to return the mostly proper result here
		result // {
		_callPackagePath = fn;
		}
		result // variantInfo
		else
		# It's very rare that callPackage doesn't return an attribute set, but it can occur.
		{
		_callPackagePath = fn;
		variantInfo;

		_internalCallByNamePackageFile = file:
		let
		result = super._internalCallByNamePackageFile file;
		variantInfo._attributeVariant = {
		# This name is used by the deserializer on the Rust side
		AutoCalled = null;
		};
		in
		if builtins.isAttrs result then
		# If this was the last overlay to be applied, we could just only return the `_callPackagePath`,
		# but that's not the case because stdenv has another overlays on top of user-provided ones.
		# So to not break the stdenv build we need to return the mostly proper result here
		result // variantInfo
		else
		# It's very rare that callPackage doesn't return an attribute set, but it can occur.
		variantInfo;
		};

		pkgs = import nixpkgsPath {
		@@ -39,14 +62,14 @@ let
		overlays = [ callPackageOverlay ];
		};

		attrInfo = attr: {
		attrInfo = attr:
		let
		value = pkgs.${attr};
		in
		{
		# These names are used by the deserializer on the Rust side
		call_package_path =
		if pkgs.${attr} ? _callPackagePath && builtins.isPath pkgs.${attr}._callPackagePath then
		toString pkgs.${attr}._callPackagePath
		else
		null;
		is_derivation = pkgs.lib.isDerivation pkgs.${attr};
		variant = value._attributeVariant or { Other = null; };
		is_derivation = pkgs.lib.isDerivation value;
		};

		attrInfos = builtins.listToAttrs (map (name: {

pkgs/test/nixpkgs-check-by-name/src/eval.rs

+41 −9

Original line number	Diff line number	Diff line
		use crate::structure;
		use crate::utils::ErrorWriter;
		use crate::Version;
		use std::path::Path;

		use anyhow::Context;
		@@ -13,16 +14,34 @@ use tempfile::NamedTempFile;
		/// Attribute set of this structure is returned by eval.nix
		#[derive(Deserialize)]
		struct AttributeInfo {
		call_package_path: Option<PathBuf>,
		variant: AttributeVariant,
		is_derivation: bool,
		}

		#[derive(Deserialize)]
		enum AttributeVariant {
		/// The attribute is auto-called as pkgs.callPackage using pkgs/by-name,
		/// and it is not overridden by a definition in all-packages.nix
		AutoCalled,
		/// The attribute is defined as a pkgs.callPackage <path> <args>,
		/// and overridden by all-packages.nix
		CallPackage {
		/// The <path> argument or None if it's not a path
		path: Option<PathBuf>,
		/// true if <args> is { }
		empty_arg: bool,
		},
		/// The attribute is not defined as pkgs.callPackage
		Other,
		}

		const EXPR: &str = include_str!("eval.nix");

		/// Check that the Nixpkgs attribute values corresponding to the packages in pkgs/by-name are
		/// of the form `callPackage <package_file> { ... }`.
		/// See the `eval.nix` file for how this is achieved on the Nix side
		pub fn check_values<W: io::Write>(
		version: Version,
		error_writer: &mut ErrorWriter<W>,
		nixpkgs: &structure::Nixpkgs,
		eval_accessible_paths: Vec<&Path>,
		@@ -97,16 +116,29 @@ pub fn check_values<W: io::Write>(
		let absolute_package_file = nixpkgs.path.join(&relative_package_file);

		if let Some(attribute_info) = actual_files.get(package_name) {
		let is_expected_file =
		if let Some(call_package_path) = &attribute_info.call_package_path {
		let valid = match &attribute_info.variant {
		AttributeVariant::AutoCalled => true,
		AttributeVariant::CallPackage { path, empty_arg } => {
		let correct_file = if let Some(call_package_path) = path {
		absolute_package_file == *call_package_path
		} else {
		false
		};
		// Only check for the argument to be non-empty if the version is V1 or
		// higher
		let non_empty = if version >= Version::V1 {
		!empty_arg
		} else {
		true
		};
		correct_file && non_empty
		}
		AttributeVariant::Other => false,
		};

		if !is_expected_file {
		if !valid {
		error_writer.write(&format!(
		"pkgs.{package_name}: This attribute is not defined as `pkgs.callPackage {} {{ ... }}`.",
		"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()
		))?;
		continue;

pkgs/test/nixpkgs-check-by-name/src/main.rs

+21 −5

Original line number	Diff line number	Diff line
		@@ -4,7 +4,7 @@ mod structure;
		mod utils;

		use anyhow::Context;
		use clap::Parser;
		use clap::{Parser, ValueEnum};
		use colored::Colorize;
		use std::io;
		use std::path::{Path, PathBuf};
		@@ -15,14 +15,28 @@ use utils::ErrorWriter;
		/// Program to check the validity of pkgs/by-name
		#[derive(Parser, Debug)]
		#[command(about)]
		struct Args {
		pub struct Args {
		/// Path to nixpkgs
		nixpkgs: PathBuf,
		/// The version of the checks
		/// Increasing this may cause failures for a Nixpkgs that succeeded before
		/// TODO: Remove default once Nixpkgs CI passes this argument
		#[arg(long, value_enum, default_value_t = Version::V0)]
		version: Version,
		}

		/// The version of the checks
		#[derive(Debug, Clone, PartialEq, PartialOrd, ValueEnum)]
		pub enum Version {
		/// Initial version
		V0,
		/// Empty argument check
		V1,
		}

		fn main() -> ExitCode {
		let args = Args::parse();
		match check_nixpkgs(&args.nixpkgs, vec![], &mut io::stderr()) {
		match check_nixpkgs(&args.nixpkgs, args.version, vec![], &mut io::stderr()) {
		Ok(true) => {
		eprintln!("{}", "Validated successfully".green());
		ExitCode::SUCCESS
		@@ -53,6 +67,7 @@ fn main() -> ExitCode {
		/// - `Ok(true)` if the structure is valid, nothing will have been written to `error_writer`.
		pub fn check_nixpkgs<W: io::Write>(
		nixpkgs_path: &Path,
		version: Version,
		eval_accessible_paths: Vec<&Path>,
		error_writer: &mut W,
		) -> anyhow::Result<bool> {
		@@ -75,7 +90,7 @@ pub fn check_nixpkgs<W: io::Write>(

		if error_writer.empty {
		// Only if we could successfully parse the structure, we do the semantic checks
		eval::check_values(&mut error_writer, &nixpkgs, eval_accessible_paths)?;
		eval::check_values(version, &mut error_writer, &nixpkgs, eval_accessible_paths)?;
		references::check_references(&mut error_writer, &nixpkgs)?;
		}
		}
		@@ -86,6 +101,7 @@ pub fn check_nixpkgs<W: io::Write>(
		mod tests {
		use crate::check_nixpkgs;
		use crate::structure;
		use crate::Version;
		use anyhow::Context;
		use std::fs;
		use std::path::Path;
		@@ -174,7 +190,7 @@ mod tests {
		// We don't want coloring to mess up the tests
		let writer = temp_env::with_var("NO_COLOR", Some("1"), \|\| -> anyhow::Result<_> {
		let mut writer = vec![];
		check_nixpkgs(&path, vec![&extra_nix_path], &mut writer)
		check_nixpkgs(&path, Version::V1, vec![&extra_nix_path], &mut writer)
		.context(format!("Failed test case {name}"))?;
		Ok(writer)
		})?;

pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix

+8 −3

Original line number	Diff line number	Diff line
		@@ -75,9 +75,14 @@ let

		# Turns autoCalledPackageFiles into an overlay that `callPackage`'s all of them
		autoCalledPackages = self: super:
		builtins.mapAttrs (name: file:
		self.callPackage file { }
		) autoCalledPackageFiles;
		{
		# Needed to be able to detect empty arguments in all-packages.nix
		# See a more detailed description in pkgs/top-level/by-name-overlay.nix
		_internalCallByNamePackageFile = file: self.callPackage file { };
		}
		// builtins.mapAttrs
		(name: self._internalCallByNamePackageFile)
		autoCalledPackageFiles;

		# A list optionally containing the `all-packages.nix` file from the test case as an overlay
		optionalAllPackagesOverlay =

Admin message