Merge pull request #275539 from tweag/by-name-enforce (f61d0270) · Commits · nix / nixpkgs

pkgs/by-name/README.md

+2 −4

Original line number	Diff line number	Diff line
		# Name-based package directories

		The structure of this directory maps almost directly to top-level package attributes.
		This is the recommended way to add new top-level packages to Nixpkgs [when possible](#limitations).
		Add new top-level packages to Nixpkgs using this mechanism [whenever possible](#limitations).

		Packages found in the named-based structure do not need to be explicitly added to the
		`top-level/all-packages.nix` file unless they require overriding the default value
		of an implicit attribute (see below).
		Packages found in the name-based structure are automatically included, without needing to be added to `all-packages.nix`. However if the implicit attribute defaults need to be changed for a package, this [must still be declared in `all-packages.nix`](#changing-implicit-attribute-defaults).

		## Example

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

+2 −0

Original line number	Diff line number	Diff line
		@@ -45,6 +45,8 @@ The current ratchets are:

		- New manual definitions of `pkgs.${name}` (e.g. in `pkgs/top-level/all-packages.nix`) with `args = { }`
		(see [nix evaluation checks](#nix-evaluation-checks)) must not be introduced.
		- New top-level packages defined using `pkgs.callPackage` must be defined with a package directory.
		- Once a top-level package uses `pkgs/by-name`, it also can't be moved back out of it.

		## Development

pkgs/test/nixpkgs-check-by-name/default.nix

+4 −0

Original line number	Diff line number	Diff line
		@@ -9,6 +9,7 @@
		}:
		let
		runtimeExprPath = ./src/eval.nix;
		nixpkgsLibPath = ../../../lib;
		package =
		rustPlatform.buildRustPackage {
		name = "nixpkgs-check-by-name";
		@@ -30,6 +31,8 @@ let
		export NIX_STATE_DIR=$TEST_ROOT/var/nix
		export NIX_STORE_DIR=$TEST_ROOT/store

		export NIXPKGS_LIB_PATH=${nixpkgsLibPath}

		# Ensure that even if tests run in parallel, we don't get an error
		# We'd run into https://github.com/NixOS/nix/issues/2706 unless the store is initialised first
		nix-store --init
		@@ -44,6 +47,7 @@ let
		'';
		passthru.shell = mkShell {
		env.NIX_CHECK_BY_NAME_EXPR_PATH = toString runtimeExprPath;
		env.NIXPKGS_LIB_PATH = toString nixpkgsLibPath;
		inputsFrom = [ package ];
		};
		};

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

+29 −7

Original line number	Diff line number	Diff line
		@@ -79,15 +79,37 @@ let
		};
		};

		attrInfos = map (name: [
		name
		(
		byNameAttrs = builtins.listToAttrs (map (name: {
		inherit name;
		value.ByName =
		if ! pkgs ? ${name} then
		{ Missing = null; }
		else
		{ Existing = attrInfo name pkgs.${name}; }
		)
		]) attrs;
		{ Existing = attrInfo name pkgs.${name}; };
		}) attrs);

		# Information on all attributes that exist but are not in pkgs/by-name.
		# We need this to enforce pkgs/by-name for new packages
		nonByNameAttrs = builtins.mapAttrs (name: value:
		let
		output = attrInfo name value;
		result = builtins.tryEval (builtins.deepSeq output null);
		in
		{
		NonByName =
		if result.success then
		{ EvalSuccess = output; }
		else
		{ EvalFailure = null; };
		}
		) (builtins.removeAttrs pkgs attrs);

		# All attributes
		attributes = byNameAttrs // nonByNameAttrs;
		in
		attrInfos
		# We output them in the form [ [ <name> <value> ] ]` such that the Rust side
		# doesn't need to sort them again to get deterministic behavior (good for testing)
		map (name: [
		name
		attributes.${name}
		]) (builtins.attrNames attributes)

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

+89 −10

Original line number	Diff line number	Diff line
		@@ -2,6 +2,8 @@ use crate::nixpkgs_problem::NixpkgsProblem;
		use crate::ratchet;
		use crate::structure;
		use crate::validation::{self, Validation::Success};
		use std::collections::HashMap;
		use std::ffi::OsString;
		use std::path::Path;

		use anyhow::Context;
		@@ -11,6 +13,21 @@ use std::process;
		use tempfile::NamedTempFile;

		/// Attribute set of this structure is returned by eval.nix
		#[derive(Deserialize)]
		enum Attribute {
		/// An attribute that should be defined via pkgs/by-name
		ByName(ByNameAttribute),
		/// An attribute not defined via pkgs/by-name
		NonByName(NonByNameAttribute),
		}

		#[derive(Deserialize)]
		enum NonByNameAttribute {
		/// The attribute doesn't evaluate
		EvalFailure,
		EvalSuccess(AttributeInfo),
		}

		#[derive(Deserialize)]
		enum ByNameAttribute {
		/// The attribute doesn't exist at all
		@@ -56,7 +73,7 @@ enum CallPackageVariant {
		pub fn check_values(
		nixpkgs_path: &Path,
		package_names: Vec<String>,
		eval_accessible_paths: &[&Path],
		eval_nix_path: &HashMap<String, PathBuf>,
		) -> validation::Result<ratchet::Nixpkgs> {
		// Write the list of packages we need to check into a temporary JSON file.
		// This can then get read by the Nix evaluation.
		@@ -105,9 +122,13 @@ pub fn check_values(
		.arg(nixpkgs_path);

		// Also add extra paths that need to be accessible
		for path in eval_accessible_paths {
		for (name, path) in eval_nix_path {
		command.arg("-I");
		command.arg(path);
		let mut name_value = OsString::new();
		name_value.push(name);
		name_value.push("=");
		name_value.push(path);
		command.arg(name_value);
		}
		command.args(["-I", &expr_path]);
		command.arg(expr_path);
		@@ -120,7 +141,7 @@ pub fn check_values(
		anyhow::bail!("Failed to run command {command:?}");
		}
		// Parse the resulting JSON value
		let attributes: Vec<(String, ByNameAttribute)> = serde_json::from_slice(&result.stdout)
		let attributes: Vec<(String, Attribute)> = serde_json::from_slice(&result.stdout)
		.with_context(\|\| {
		format!(
		"Failed to deserialise {}",
		@@ -133,30 +154,86 @@ pub fn check_values(
		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 {
		Missing => NixpkgsProblem::UndefinedAttr {
		// 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(Tight),
		NonCallPackage => Success(Tight),
		// This is an odd case when _internalCallByNamePackageFile is used to define a package.
		CallPackage(CallPackageInfo {
		call_package_variant: Auto,
		..
		}) => NixpkgsProblem::InternalCallPackageUsed {
		attr_name: attribute_name.clone(),
		}
		.into(),
		// Only derivations can be in pkgs/by-name,
		// so this attribute doesn't qualify
		CallPackage(CallPackageInfo {
		is_derivation: false,
		..
		}) => Success(Tight),

		// The case of an attribute that qualifies:
		// - Uses callPackage
		// - Is a derivation
		CallPackage(CallPackageInfo {
		is_derivation: true,
		call_package_variant: Manual { path, empty_arg },
		}) => Success(Loose(ratchet::UsesByName {
		call_package_path: path,
		empty_arg,
		})),
		};
		uses_by_name.map(\|x\| ratchet::Package {
		empty_non_auto_called: Tight,
		uses_by_name: x,
		})
		}
		NonByName(EvalFailure) => {
		// This is a bit of an odd case: We don't even _know_ whether this attribute
		// would qualify for using pkgs/by-name. We can either:
		// - Assume it's not using pkgs/by-name, which has the problem that if a
		// package evaluation gets broken temporarily, the fix can remove it from
		// pkgs/by-name again
		// - Assume it's using pkgs/by-name already, which has the problem that if a
		// package evaluation gets broken temporarily, fixing it requires a move to
		// pkgs/by-name
		// We choose the latter, since we want to move towards pkgs/by-name, not away
		// from it
		Success(ratchet::Package {
		empty_non_auto_called: Tight,
		uses_by_name: Tight,
		})
		}
		ByName(Missing) => NixpkgsProblem::UndefinedAttr {
		relative_package_file: relative_package_file.clone(),
		package_name: attribute_name.clone(),
		}
		.into(),
		Existing(NonAttributeSet) => NixpkgsProblem::NonDerivation {
		ByName(Existing(NonAttributeSet)) => NixpkgsProblem::NonDerivation {
		relative_package_file: relative_package_file.clone(),
		package_name: attribute_name.clone(),
		}
		.into(),
		Existing(NonCallPackage) => NixpkgsProblem::WrongCallPackage {
		ByName(Existing(NonCallPackage)) => NixpkgsProblem::WrongCallPackage {
		relative_package_file: relative_package_file.clone(),
		package_name: attribute_name.clone(),
		}
		.into(),
		Existing(CallPackage(CallPackageInfo {
		ByName(Existing(CallPackage(CallPackageInfo {
		is_derivation,
		call_package_variant,
		})) => {
		}))) => {
		let check_result = if !is_derivation {
		NixpkgsProblem::NonDerivation {
		relative_package_file: relative_package_file.clone(),
		@@ -170,6 +247,7 @@ pub fn check_values(
		check_result.and(match &call_package_variant {
		Auto => Success(ratchet::Package {
		empty_non_auto_called: Tight,
		uses_by_name: Tight,
		}),
		Manual { path, empty_arg } => {
		let correct_file = if let Some(call_package_path) = path {
		@@ -186,6 +264,7 @@ pub fn check_values(
		} else {
		Tight
		},
		uses_by_name: Tight,
		})
		} else {
		NixpkgsProblem::WrongCallPackage {
		@@ -203,7 +282,7 @@ pub fn check_values(
		));

		Ok(check_result.map(\|elems\| ratchet::Nixpkgs {
		package_names,
		package_names: elems.iter().map(\|(name, _)\| name.to_owned()).collect(),
		package_map: elems.into_iter().collect(),
		}))
		}

Admin message