diff options
Diffstat (limited to 'nixpkgs/pkgs/test')
70 files changed, 872 insertions, 312 deletions
diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.lock b/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.lock index 904a9cff0e78..19435c2ab76e 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.lock +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.lock @@ -299,12 +299,14 @@ dependencies = [ "itertools", "lazy_static", "regex", + "relative-path", "rnix", "rowan", "serde", "serde_json", "temp-env", "tempfile", + "textwrap", ] [[package]] @@ -393,6 +395,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5ea92a5b6195c6ef2a0295ea818b312502c6fc94dde986c5553242e18fd4ce2" [[package]] +name = "relative-path" +version = "1.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc" + +[[package]] name = "rnix" version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -483,6 +491,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "942b4a808e05215192e39f4ab80813e599068285906cc91aa64f923db842bd5a" [[package]] +name = "smawk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" + +[[package]] name = "strsim" version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -528,12 +542,35 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f18aa187839b2bdb1ad2fa35ead8c4c2976b64e4363c386d45ac0f7ee85c9233" [[package]] +name = "textwrap" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" +dependencies = [ + "smawk", + "unicode-linebreak", + "unicode-width", +] + +[[package]] name = "unicode-ident" version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" [[package]] +name = "unicode-linebreak" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" + +[[package]] +name = "unicode-width" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" + +[[package]] name = "utf8parse" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.toml b/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.toml index 5240cd69f996..50cdabb7e2dd 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.toml +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/Cargo.toml @@ -15,7 +15,9 @@ lazy_static = "1.4.0" colored = "2.0.4" itertools = "0.11.0" rowan = "0.15.11" +indoc = "2.0.4" +relative-path = "1.9.2" +textwrap = "0.16.1" [dev-dependencies] temp-env = "0.3.5" -indoc = "2.0.4" diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/eval.rs index e90a95533144..094508f595d8 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,10 +1,14 @@ +use crate::nix_file::CallPackageArgumentInfo; use crate::nixpkgs_problem::NixpkgsProblem; use crate::ratchet; +use crate::ratchet::RatchetState::Loose; +use crate::ratchet::RatchetState::Tight; use crate::structure; use crate::utils; use crate::validation::ResultIteratorExt as _; use crate::validation::{self, Validation::Success}; use crate::NixFileStore; +use relative_path::RelativePathBuf; use std::path::Path; use anyhow::Context; @@ -51,6 +55,20 @@ struct Location { pub column: usize, } +impl Location { + // Returns the [file] field, but relative to Nixpkgs + fn relative_file(&self, nixpkgs_path: &Path) -> anyhow::Result<RelativePathBuf> { + let path = self.file.strip_prefix(nixpkgs_path).with_context(|| { + format!( + "The file ({}) is outside Nixpkgs ({})", + self.file.display(), + nixpkgs_path.display() + ) + })?; + Ok(RelativePathBuf::from_path(path).expect("relative path")) + } +} + #[derive(Deserialize)] pub enum AttributeVariant { /// The attribute is not an attribute set, we're limited in the amount of information we can get @@ -163,6 +181,7 @@ pub fn check_values( Attribute::NonByName(non_by_name_attribute) => handle_non_by_name_attribute( nixpkgs_path, nix_file_store, + &attribute_name, non_by_name_attribute, )?, Attribute::ByName(by_name_attribute) => by_name( @@ -195,7 +214,6 @@ fn by_name( use ByNameAttribute::*; let relative_package_file = structure::relative_file_for_package(attribute_name); - let absolute_package_file = nixpkgs_path.join(&relative_package_file); // At this point we know that `pkgs/by-name/fo/foo/package.nix` has to exists. // This match decides whether the attribute `foo` is defined accordingly @@ -276,53 +294,31 @@ fn by_name( // We should expect manual definitions to have a location, otherwise we can't // enforce the expected format if let Some(location) = location { - // Parse the Nix file in the location and figure out whether it's an - // attribute definition of the form `= callPackage <arg1> <arg2>`, + // Parse the Nix file in the location + let nix_file = nix_file_store.get(&location.file)?; + + // The relative path of the Nix file, for error messages + let relative_location_file = location.relative_file(nixpkgs_path).with_context(|| { + format!("Failed to resolve the file where attribute {attribute_name} is defined") + })?; + + // Figure out whether it's an attribute definition of the form `= callPackage <arg1> <arg2>`, // returning the arguments if so. - let optional_syntactic_call_package = nix_file_store - .get(&location.file)? - .call_package_argument_info_at( - location.line, - location.column, - // We're passing `pkgs/by-name/fo/foo/package.nix` here, which causes - // the function to verify that `<arg1>` is the same path, - // making `syntactic_call_package.relative_path` end up as `""` - // TODO: This is confusing and should be improved - &absolute_package_file, - )?; - - // At this point, we completed two different checks for whether it's a - // `callPackage` - match (is_semantic_call_package, optional_syntactic_call_package) { - // Something like `<attr> = { ... }` - // or a `pkgs.callPackage` but with the wrong file - (false, None) - // Something like `<attr> = pythonPackages.callPackage ./pkgs/by-name/...` - | (false, Some(_)) - // Something like `<attr> = bar` where `bar = pkgs.callPackage ...` - // or a `callPackage` but with the wrong file - | (true, None) => { - // All of these are not of the expected form, so error out - // TODO: Make error more specific, don't lump everything together - NixpkgsProblem::WrongCallPackage { - relative_package_file: relative_package_file.to_owned(), - package_name: attribute_name.to_owned(), - }.into() - } - // Something like `<attr> = pkgs.callPackage ./pkgs/by-name/...`, - // with the correct file - (true, Some(syntactic_call_package)) => { - Success( - // Manual definitions with empty arguments are not allowed - // anymore - if syntactic_call_package.empty_arg { - Loose(()) - } else { - Tight - } - ) - } - } + let (optional_syntactic_call_package, definition) = nix_file + .call_package_argument_info_at(location.line, location.column, nixpkgs_path) + .with_context(|| { + format!("Failed to get the definition info for attribute {attribute_name}") + })?; + + by_name_override( + attribute_name, + relative_package_file, + is_semantic_call_package, + optional_syntactic_call_package, + definition, + location, + relative_location_file, + ) } else { // If manual definitions don't have a location, it's likely `mapAttrs`'d // over, e.g. if it's defined in aliases.nix. @@ -350,11 +346,91 @@ fn by_name( ) } +/// Handles the case for packages in `pkgs/by-name` that are manually overridden, e.g. in +/// all-packages.nix +fn by_name_override( + attribute_name: &str, + expected_package_file: RelativePathBuf, + is_semantic_call_package: bool, + optional_syntactic_call_package: Option<CallPackageArgumentInfo>, + definition: String, + location: Location, + relative_location_file: RelativePathBuf, +) -> validation::Validation<ratchet::RatchetState<ratchet::ManualDefinition>> { + // At this point, we completed two different checks for whether it's a + // `callPackage` + match (is_semantic_call_package, optional_syntactic_call_package) { + // Something like `<attr> = foo` + (_, None) => NixpkgsProblem::NonSyntacticCallPackage { + package_name: attribute_name.to_owned(), + file: relative_location_file, + line: location.line, + column: location.column, + definition, + } + .into(), + // Something like `<attr> = pythonPackages.callPackage ...` + (false, Some(_)) => NixpkgsProblem::NonToplevelCallPackage { + package_name: attribute_name.to_owned(), + file: relative_location_file, + line: location.line, + column: location.column, + definition, + } + .into(), + // Something like `<attr> = pkgs.callPackage ...` + (true, Some(syntactic_call_package)) => { + if let Some(actual_package_file) = syntactic_call_package.relative_path { + if actual_package_file != expected_package_file { + // Wrong path + NixpkgsProblem::WrongCallPackagePath { + package_name: attribute_name.to_owned(), + file: relative_location_file, + line: location.line, + actual_path: actual_package_file, + expected_path: expected_package_file, + } + .into() + } else { + // Manual definitions with empty arguments are not allowed + // anymore, but existing ones should continue to be allowed + let manual_definition_ratchet = if syntactic_call_package.empty_arg { + // This is the state to migrate away from + Loose(NixpkgsProblem::EmptyArgument { + package_name: attribute_name.to_owned(), + file: relative_location_file, + line: location.line, + column: location.column, + definition, + }) + } else { + // This is the state to migrate to + Tight + }; + + Success(manual_definition_ratchet) + } + } else { + // No path + NixpkgsProblem::NonPath { + package_name: attribute_name.to_owned(), + file: relative_location_file, + line: location.line, + column: location.column, + definition, + } + .into() + } + } + } +} + /// Handles the evaluation result for an attribute _not_ in `pkgs/by-name`, /// turning it into a validation result. fn handle_non_by_name_attribute( nixpkgs_path: &Path, nix_file_store: &mut NixFileStore, + attribute_name: &str, non_by_name_attribute: NonByNameAttribute, ) -> validation::Result<ratchet::Package> { use ratchet::RatchetState::*; @@ -405,11 +481,17 @@ fn handle_non_by_name_attribute( location: Some(location), }) = non_by_name_attribute { - // Parse the Nix file in the location and figure out whether it's an - // attribute definition of the form `= callPackage <arg1> <arg2>`, + // Parse the Nix file in the location + let nix_file = nix_file_store.get(&location.file)?; + + // The relative path of the Nix file, for error messages + let relative_location_file = location.relative_file(nixpkgs_path).with_context(|| { + format!("Failed to resolve the file where attribute {attribute_name} is defined") + })?; + + // Figure out whether it's an attribute definition of the form `= callPackage <arg1> <arg2>`, // returning the arguments if so. - let optional_syntactic_call_package = nix_file_store - .get(&location.file)? + let (optional_syntactic_call_package, _definition) = nix_file .call_package_argument_info_at( location.line, location.column, @@ -417,7 +499,10 @@ fn handle_non_by_name_attribute( // strips the absolute Nixpkgs path from it, such that // syntactic_call_package.relative_path is relative to Nixpkgs nixpkgs_path - )?; + ) + .with_context(|| { + format!("Failed to get the definition info for attribute {attribute_name}") + })?; // At this point, we completed two different checks for whether it's a // `callPackage` @@ -453,7 +538,7 @@ fn handle_non_by_name_attribute( _ => { // Otherwise, the path is outside `pkgs/by-name`, which means it can be // migrated - Loose(syntactic_call_package) + Loose((syntactic_call_package, relative_location_file)) } } } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs index 0d0ddcd7e632..dcc9cb9e716d 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -1,4 +1,5 @@ use crate::nix_file::NixFileStore; +use std::panic; mod eval; mod nix_file; mod nixpkgs_problem; @@ -17,6 +18,7 @@ use colored::Colorize; use std::io; use std::path::{Path, PathBuf}; use std::process::ExitCode; +use std::thread; /// Program to check the validity of pkgs/by-name /// @@ -46,15 +48,9 @@ pub struct Args { fn main() -> ExitCode { let args = Args::parse(); - match process(&args.base, &args.nixpkgs, false, &mut io::stderr()) { - Ok(true) => { - eprintln!("{}", "Validated successfully".green()); - ExitCode::SUCCESS - } - Ok(false) => { - eprintln!("{}", "Validation failed, see above errors".yellow()); - ExitCode::from(1) - } + match process(args.base, args.nixpkgs, false, &mut io::stderr()) { + Ok(true) => ExitCode::SUCCESS, + Ok(false) => ExitCode::from(1), Err(e) => { eprintln!("{} {:#}", "I/O error: ".yellow(), e); ExitCode::from(2) @@ -77,34 +73,66 @@ fn main() -> ExitCode { /// - `Ok(false)` if there are problems, all of which will be written to `error_writer`. /// - `Ok(true)` if there are no problems pub fn process<W: io::Write>( - base_nixpkgs: &Path, - main_nixpkgs: &Path, + base_nixpkgs: PathBuf, + main_nixpkgs: PathBuf, keep_nix_path: bool, error_writer: &mut W, ) -> anyhow::Result<bool> { - // Check the main Nixpkgs first - let main_result = check_nixpkgs(main_nixpkgs, keep_nix_path, error_writer)?; - let check_result = main_result.result_map(|nixpkgs_version| { - // If the main Nixpkgs doesn't have any problems, run the ratchet checks against the base - // Nixpkgs - check_nixpkgs(base_nixpkgs, keep_nix_path, error_writer)?.result_map( - |base_nixpkgs_version| { - Ok(ratchet::Nixpkgs::compare( - base_nixpkgs_version, - nixpkgs_version, - )) - }, - ) - })?; + // Very easy to parallelise this, since it's totally independent + let base_thread = thread::spawn(move || check_nixpkgs(&base_nixpkgs, keep_nix_path)); + let main_result = check_nixpkgs(&main_nixpkgs, keep_nix_path)?; + + let base_result = match base_thread.join() { + Ok(res) => res?, + Err(e) => panic::resume_unwind(e), + }; - match check_result { - Failure(errors) => { + match (base_result, main_result) { + (Failure(_), Failure(errors)) => { + // Base branch fails and the PR doesn't fix it and may also introduce additional problems for error in errors { writeln!(error_writer, "{}", error.to_string().red())? } + writeln!(error_writer, "{}", "The base branch is broken and still has above problems with this PR, which need to be fixed first.\nConsider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.".yellow())?; Ok(false) } - Success(()) => Ok(true), + (Failure(_), Success(_)) => { + writeln!( + error_writer, + "{}", + "The base branch is broken, but this PR fixes it. Nice job!".green() + )?; + Ok(true) + } + (Success(_), Failure(errors)) => { + for error in errors { + writeln!(error_writer, "{}", error.to_string().red())? + } + writeln!( + error_writer, + "{}", + "This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break." + .yellow() + )?; + Ok(false) + } + (Success(base), Success(main)) => { + // Both base and main branch succeed, check ratchet state + match ratchet::Nixpkgs::compare(base, main) { + Failure(errors) => { + for error in errors { + writeln!(error_writer, "{}", error.to_string().red())? + } + writeln!(error_writer, "{}", "This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.".yellow())?; + + Ok(false) + } + Success(()) => { + writeln!(error_writer, "{}", "Validated successfully".green())?; + Ok(true) + } + } + } } } @@ -113,10 +141,9 @@ pub fn process<W: io::Write>( /// This does not include ratchet checks, see ../README.md#ratchet-checks /// Instead a `ratchet::Nixpkgs` value is returned, whose `compare` method allows performing the /// ratchet check against another result. -pub fn check_nixpkgs<W: io::Write>( +pub fn check_nixpkgs( nixpkgs_path: &Path, keep_nix_path: bool, - error_writer: &mut W, ) -> validation::Result<ratchet::Nixpkgs> { let mut nix_file_store = NixFileStore::default(); @@ -129,11 +156,7 @@ pub fn check_nixpkgs<W: io::Write>( })?; if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() { - writeln!( - error_writer, - "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.", - utils::BASE_SUBPATH - )?; + // No pkgs/by-name directory, always valid Success(ratchet::Nixpkgs::default()) } else { check_structure(&nixpkgs_path, &mut nix_file_store)?.result_map(|package_names| @@ -163,8 +186,8 @@ mod tests { continue; } - let expected_errors = - fs::read_to_string(path.join("expected")).unwrap_or(String::new()); + let expected_errors = fs::read_to_string(path.join("expected")) + .expect("No expected file for test {name}"); test_nixpkgs(&name, &path, &expected_errors)?; } @@ -201,7 +224,7 @@ mod tests { test_nixpkgs( "case_sensitive", &path, - "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\n", + "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\nThis PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.\n", )?; Ok(()) @@ -225,7 +248,11 @@ mod tests { let tmpdir = temp_root.path().join("symlinked"); temp_env::with_var("TMPDIR", Some(&tmpdir), || { - test_nixpkgs("symlinked_tmpdir", Path::new("tests/success"), "") + test_nixpkgs( + "symlinked_tmpdir", + Path::new("tests/success"), + "Validated successfully\n", + ) }) } @@ -240,7 +267,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![]; - process(base_nixpkgs, &path, true, &mut writer) + process(base_nixpkgs.to_owned(), path.to_owned(), true, &mut writer) .with_context(|| format!("Failed test case {name}"))?; Ok(writer) })?; @@ -249,7 +276,7 @@ mod tests { if actual_errors != expected_errors { panic!( - "Failed test case {name}, expected these errors:\n\n{}\n\nbut got these:\n\n{}", + "Failed test case {name}, expected these errors:\n=======\n{}\n=======\nbut got these:\n=======\n{}\n=======", expected_errors, actual_errors ); } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs index 836c5e2dcdda..e2dc1e196141 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs @@ -2,10 +2,11 @@ use crate::utils::LineIndex; use anyhow::Context; +use itertools::Either::{self, Left, Right}; +use relative_path::RelativePathBuf; use rnix::ast; use rnix::ast::Expr; use rnix::ast::HasEntry; -use rnix::SyntaxKind; use rowan::ast::AstNode; use rowan::TextSize; use rowan::TokenAtOffset; @@ -79,7 +80,7 @@ impl NixFile { #[derive(Debug, PartialEq)] pub struct CallPackageArgumentInfo { /// The relative path of the first argument, or `None` if it's not a path. - pub relative_path: Option<PathBuf>, + pub relative_path: Option<RelativePathBuf>, /// Whether the second argument is an empty attribute set pub empty_arg: bool, } @@ -87,8 +88,9 @@ pub struct CallPackageArgumentInfo { impl NixFile { /// Returns information about callPackage arguments for an attribute at a specific line/column /// index. - /// If the location is not of the form `<attr> = callPackage <arg1> <arg2>;`, `None` is - /// returned. + /// If the definition at the given location is not of the form `<attr> = callPackage <arg1> <arg2>;`, + /// `Ok((None, String))` is returned, with `String` being the definition itself. + /// /// This function only returns `Err` for problems that can't be caused by the Nix contents, /// but rather problems in this programs code itself. /// @@ -109,7 +111,10 @@ impl NixFile { /// /// You'll get back /// ```rust - /// Some(CallPackageArgumentInfo { path = Some("default.nix"), empty_arg: true }) + /// Ok(( + /// Some(CallPackageArgumentInfo { path = Some("default.nix"), empty_arg: true }), + /// "foo = self.callPackage ./default.nix { };", + /// )) /// ``` /// /// Note that this also returns the same for `pythonPackages.callPackage`. It doesn't make an @@ -119,11 +124,16 @@ impl NixFile { line: usize, column: usize, relative_to: &Path, - ) -> anyhow::Result<Option<CallPackageArgumentInfo>> { - let Some(attrpath_value) = self.attrpath_value_at(line, column)? else { - return Ok(None); - }; - self.attrpath_value_call_package_argument_info(attrpath_value, relative_to) + ) -> anyhow::Result<(Option<CallPackageArgumentInfo>, String)> { + Ok(match self.attrpath_value_at(line, column)? { + Left(definition) => (None, definition), + Right(attrpath_value) => { + let definition = attrpath_value.to_string(); + let attrpath_value = + self.attrpath_value_call_package_argument_info(attrpath_value, relative_to)?; + (attrpath_value, definition) + } + }) } // Internal function mainly to make it independently testable @@ -131,7 +141,7 @@ impl NixFile { &self, line: usize, column: usize, - ) -> anyhow::Result<Option<ast::AttrpathValue>> { + ) -> anyhow::Result<Either<String, ast::AttrpathValue>> { let index = self.line_index.fromlinecolumn(line, column); let token_at_offset = self @@ -158,6 +168,22 @@ impl NixFile { ) }; + if ast::Attr::can_cast(node.kind()) { + // Something like `foo`, `"foo"` or `${"foo"}` + } else if ast::Inherit::can_cast(node.kind()) { + // Something like `inherit <attr>` or `inherit (<source>) <attr>` + // This is the only other way how `builtins.unsafeGetAttrPos` can return + // attribute positions, but we only look for ones like `<attr-path> = <value>`, so + // ignore this + return Ok(Left(node.to_string())); + } else { + // However, anything else is not expected and smells like a bug + anyhow::bail!( + "Node in {} is neither an attribute node nor an inherit node: {node:?}", + self.path.display() + ) + } + // node looks like "foo" let Some(attrpath_node) = node.parent() else { anyhow::bail!( @@ -166,10 +192,14 @@ impl NixFile { ) }; - if attrpath_node.kind() != SyntaxKind::NODE_ATTRPATH { - // This can happen for e.g. `inherit foo`, so definitely not a syntactic `callPackage` - return Ok(None); + if !ast::Attrpath::can_cast(attrpath_node.kind()) { + // We know that `node` is an attribute, its parent should be an attribute path + anyhow::bail!( + "In {}, attribute parent node is not an attribute path node: {attrpath_node:?}", + self.path.display() + ) } + // attrpath_node looks like "foo.bar" let Some(attrpath_value_node) = attrpath_node.parent() else { anyhow::bail!( @@ -189,7 +219,9 @@ impl NixFile { // unwrap is fine because we confirmed that we can cast with the above check. // We could avoid this `unwrap` for a `clone`, since `cast` consumes the argument, // but we still need it for the error message when the cast fails. - Ok(Some(ast::AttrpathValue::cast(attrpath_value_node).unwrap())) + Ok(Right( + ast::AttrpathValue::cast(attrpath_value_node).unwrap(), + )) } // Internal function mainly to make attrpath_value_at independently testable @@ -338,8 +370,8 @@ pub enum ResolvedPath { /// The path is outside the given absolute path Outside, /// The path is within the given absolute path. - /// The `PathBuf` is the relative path under the given absolute path. - Within(PathBuf), + /// The `RelativePathBuf` is the relative path under the given absolute path. + Within(RelativePathBuf), } impl NixFile { @@ -371,7 +403,9 @@ impl NixFile { // Check if it's within relative_to match resolved.strip_prefix(relative_to) { Err(_prefix_error) => ResolvedPath::Outside, - Ok(suffix) => ResolvedPath::Within(suffix.to_path_buf()), + Ok(suffix) => ResolvedPath::Within( + RelativePathBuf::from_path(suffix).expect("a relative path"), + ), } } } @@ -408,6 +442,7 @@ mod tests { /**/quuux/**/=/**/5/**/;/*E*/ inherit toInherit; + inherit (toInherit) toInherit; } "#}; @@ -417,20 +452,28 @@ mod tests { // These are builtins.unsafeGetAttrPos locations for the attributes let cases = [ - (2, 3, Some("foo = 1;")), - (3, 3, Some(r#""bar" = 2;"#)), - (4, 3, Some(r#"${"baz"} = 3;"#)), - (5, 3, Some(r#""${"qux"}" = 4;"#)), - (8, 3, Some("quux\n # B\n =\n # C\n 5\n # D\n ;")), - (17, 7, Some("quuux/**/=/**/5/**/;")), - (19, 10, None), + (2, 3, Right("foo = 1;")), + (3, 3, Right(r#""bar" = 2;"#)), + (4, 3, Right(r#"${"baz"} = 3;"#)), + (5, 3, Right(r#""${"qux"}" = 4;"#)), + (8, 3, Right("quux\n # B\n =\n # C\n 5\n # D\n ;")), + (17, 7, Right("quuux/**/=/**/5/**/;")), + (19, 10, Left("inherit toInherit;")), + (20, 22, Left("inherit (toInherit) toInherit;")), ]; for (line, column, expected_result) in cases { let actual_result = nix_file - .attrpath_value_at(line, column)? - .map(|node| node.to_string()); - assert_eq!(actual_result.as_deref(), expected_result); + .attrpath_value_at(line, column) + .context(format!("line {line}, column {column}"))? + .map_right(|node| node.to_string()); + let owned_expected_result = expected_result + .map(|x| x.to_string()) + .map_left(|x| x.to_string()); + assert_eq!( + actual_result, owned_expected_result, + "line {line}, column {column}" + ); } Ok(()) @@ -466,14 +509,14 @@ mod tests { ( 6, Some(CallPackageArgumentInfo { - relative_path: Some(PathBuf::from("file.nix")), + relative_path: Some(RelativePathBuf::from("file.nix")), empty_arg: true, }), ), ( 7, Some(CallPackageArgumentInfo { - relative_path: Some(PathBuf::from("file.nix")), + relative_path: Some(RelativePathBuf::from("file.nix")), empty_arg: true, }), ), @@ -487,7 +530,7 @@ mod tests { ( 9, Some(CallPackageArgumentInfo { - relative_path: Some(PathBuf::from("file.nix")), + relative_path: Some(RelativePathBuf::from("file.nix")), empty_arg: false, }), ), @@ -501,8 +544,10 @@ mod tests { ]; for (line, expected_result) in cases { - let actual_result = nix_file.call_package_argument_info_at(line, 3, temp_dir.path())?; - assert_eq!(actual_result, expected_result); + let (actual_result, _definition) = nix_file + .call_package_argument_info_at(line, 3, temp_dir.path()) + .context(format!("line {line}"))?; + assert_eq!(actual_result, expected_result, "line {line}"); } Ok(()) diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs index e13869adaa41..7e257c0ed5d8 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs @@ -1,96 +1,141 @@ use crate::structure; use crate::utils::PACKAGE_NIX_FILENAME; +use indoc::writedoc; +use relative_path::RelativePath; +use relative_path::RelativePathBuf; use std::ffi::OsString; use std::fmt; -use std::io; -use std::path::PathBuf; /// Any problem that can occur when checking Nixpkgs +/// All paths are relative to Nixpkgs such that the error messages can't be influenced by Nixpkgs absolute +/// location +#[derive(Clone)] pub enum NixpkgsProblem { ShardNonDir { - relative_shard_path: PathBuf, + relative_shard_path: RelativePathBuf, }, InvalidShardName { - relative_shard_path: PathBuf, + relative_shard_path: RelativePathBuf, shard_name: String, }, PackageNonDir { - relative_package_dir: PathBuf, + relative_package_dir: RelativePathBuf, }, CaseSensitiveDuplicate { - relative_shard_path: PathBuf, + relative_shard_path: RelativePathBuf, first: OsString, second: OsString, }, InvalidPackageName { - relative_package_dir: PathBuf, + relative_package_dir: RelativePathBuf, package_name: String, }, IncorrectShard { - relative_package_dir: PathBuf, - correct_relative_package_dir: PathBuf, + relative_package_dir: RelativePathBuf, + correct_relative_package_dir: RelativePathBuf, }, PackageNixNonExistent { - relative_package_dir: PathBuf, + relative_package_dir: RelativePathBuf, }, PackageNixDir { - relative_package_dir: PathBuf, + relative_package_dir: RelativePathBuf, }, UndefinedAttr { - relative_package_file: PathBuf, + relative_package_file: RelativePathBuf, package_name: String, }, - WrongCallPackage { - relative_package_file: PathBuf, + EmptyArgument { package_name: String, + file: RelativePathBuf, + line: usize, + column: usize, + definition: String, + }, + NonToplevelCallPackage { + package_name: String, + file: RelativePathBuf, + line: usize, + column: usize, + definition: String, + }, + NonPath { + package_name: String, + file: RelativePathBuf, + line: usize, + column: usize, + definition: String, + }, + WrongCallPackagePath { + package_name: String, + file: RelativePathBuf, + line: usize, + actual_path: RelativePathBuf, + expected_path: RelativePathBuf, + }, + NonSyntacticCallPackage { + package_name: String, + file: RelativePathBuf, + line: usize, + column: usize, + definition: String, }, NonDerivation { - relative_package_file: PathBuf, + relative_package_file: RelativePathBuf, package_name: String, }, OutsideSymlink { - relative_package_dir: PathBuf, - subpath: PathBuf, + relative_package_dir: RelativePathBuf, + subpath: RelativePathBuf, }, UnresolvableSymlink { - relative_package_dir: PathBuf, - subpath: PathBuf, - io_error: io::Error, + relative_package_dir: RelativePathBuf, + subpath: RelativePathBuf, + io_error: String, }, PathInterpolation { - relative_package_dir: PathBuf, - subpath: PathBuf, + relative_package_dir: RelativePathBuf, + subpath: RelativePathBuf, line: usize, text: String, }, SearchPath { - relative_package_dir: PathBuf, - subpath: PathBuf, + relative_package_dir: RelativePathBuf, + subpath: RelativePathBuf, line: usize, text: String, }, OutsidePathReference { - relative_package_dir: PathBuf, - subpath: PathBuf, + relative_package_dir: RelativePathBuf, + subpath: RelativePathBuf, line: usize, text: String, }, UnresolvablePathReference { - relative_package_dir: PathBuf, - subpath: PathBuf, + relative_package_dir: RelativePathBuf, + subpath: RelativePathBuf, line: usize, text: String, - io_error: io::Error, + io_error: String, + }, + MovedOutOfByNameEmptyArg { + package_name: String, + call_package_path: Option<RelativePathBuf>, + file: RelativePathBuf, + }, + MovedOutOfByNameNonEmptyArg { + package_name: String, + call_package_path: Option<RelativePathBuf>, + file: RelativePathBuf, }, - MovedOutOfByName { + NewPackageNotUsingByNameEmptyArg { package_name: String, - call_package_path: Option<PathBuf>, - empty_arg: bool, + call_package_path: Option<RelativePathBuf>, + file: RelativePathBuf, }, - NewPackageNotUsingByName { + NewPackageNotUsingByNameNonEmptyArg { package_name: String, - call_package_path: Option<PathBuf>, - empty_arg: bool, + call_package_path: Option<RelativePathBuf>, + file: RelativePathBuf, }, InternalCallPackageUsed { attr_name: String, @@ -106,156 +151,238 @@ impl fmt::Display for NixpkgsProblem { NixpkgsProblem::ShardNonDir { relative_shard_path } => write!( f, - "{}: This is a file, but it should be a directory.", - relative_shard_path.display(), + "{relative_shard_path}: This is a file, but it should be a directory.", ), 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() + "{relative_shard_path}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", ), NixpkgsProblem::PackageNonDir { relative_package_dir } => write!( f, - "{}: This path is a file, but it should be a directory.", - relative_package_dir.display(), + "{relative_package_dir}: This path is a file, but it should be a directory.", ), NixpkgsProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } => write!( f, - "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.", - relative_shard_path.display(), + "{relative_shard_path}: Duplicate case-sensitive package directories {first:?} and {second:?}.", ), 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(), + "{relative_package_dir}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", ), 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(), + "{relative_package_dir}: Incorrect directory location, should be {correct_relative_package_dir} instead.", ), NixpkgsProblem::PackageNixNonExistent { relative_package_dir } => write!( f, - "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", - relative_package_dir.display(), + "{relative_package_dir}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", ), NixpkgsProblem::PackageNixDir { relative_package_dir } => write!( f, - "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", - relative_package_dir.display(), + "{relative_package_dir}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", ), 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() + "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {relative_package_file}", ), - NixpkgsProblem::WrongCallPackage { relative_package_file, package_name } => - write!( + NixpkgsProblem::EmptyArgument { package_name, file, line, column, definition } => { + let relative_package_dir = structure::relative_dir_for_package(package_name); + let relative_package_file = structure::relative_file_for_package(package_name); + let indented_definition = indent_definition(*column, definition.clone()); + writedoc!( 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() - ), + " + - Because {relative_package_dir} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage ./{relative_package_file} {{ /* ... */ }}; + + However, in this PR, the second argument is empty. See the definition in {file}:{line}: + + {indented_definition} + + Such a definition is provided automatically and therefore not necessary. Please remove it. + ", + ) + } + NixpkgsProblem::NonToplevelCallPackage { package_name, file, line, column, definition } => { + let relative_package_dir = structure::relative_dir_for_package(package_name); + let relative_package_file = structure::relative_file_for_package(package_name); + let indented_definition = indent_definition(*column, definition.clone()); + writedoc!( + f, + " + - Because {relative_package_dir} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage ./{relative_package_file} {{ /* ... */ }}; + + However, in this PR, a different `callPackage` is used. See the definition in {file}:{line}: + + {indented_definition} + ", + ) + } + NixpkgsProblem::NonPath { package_name, file, line, column, definition } => { + let relative_package_dir = structure::relative_dir_for_package(package_name); + let relative_package_file = structure::relative_file_for_package(package_name); + let indented_definition = indent_definition(*column, definition.clone()); + writedoc!( + f, + " + - Because {relative_package_dir} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage ./{relative_package_file} {{ /* ... */ }}; + + However, in this PR, the first `callPackage` argument is not a path. See the definition in {file}:{line}: + + {indented_definition} + ", + ) + } + NixpkgsProblem::WrongCallPackagePath { package_name, file, line, actual_path, expected_path } => { + let relative_package_dir = structure::relative_dir_for_package(package_name); + let expected_path_expr = create_path_expr(file, expected_path); + let actual_path_expr = create_path_expr(file, actual_path); + writedoc! { + f, + " + - Because {relative_package_dir} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage {expected_path_expr} {{ /* ... */ }}; + + However, in this PR, the first `callPackage` argument is the wrong path. See the definition in {file}:{line}: + + {package_name} = callPackage {actual_path_expr} {{ /* ... */ }}; + ", + } + } + NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => { + let relative_package_dir = structure::relative_dir_for_package(package_name); + let relative_package_file = structure::relative_file_for_package(package_name); + let indented_definition = indent_definition(*column, definition.clone()); + writedoc!( + f, + " + - Because {relative_package_dir} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage ./{relative_package_file} {{ /* ... */ }}; + + However, in this PR, it isn't defined that way. See the definition in {file}:{line} + + {indented_definition} + ", + ) + } NixpkgsProblem::NonDerivation { relative_package_file, package_name } => write!( f, - "pkgs.{package_name}: This attribute defined by {} is not a derivation", - relative_package_file.display() + "pkgs.{package_name}: This attribute defined by {relative_package_file} is not a derivation", ), 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(), + "{relative_package_dir}: Path {subpath} is a symlink pointing to a path outside the directory of that package.", ), 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(), + "{relative_package_dir}: Path {subpath} is a symlink which cannot be resolved: {io_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 + "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\", which is not yet supported and may point outside the directory of that package.", ), 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 + "{relative_package_dir}: File {subpath} at line {line} contains the nix search path expression \"{text}\" which may point outside the directory of that package.", ), 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, + "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which may point outside the directory of that package.", ), 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, + "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which cannot be resolved: {io_error}.", ), - NixpkgsProblem::MovedOutOfByName { package_name, call_package_path, empty_arg } => { + NixpkgsProblem::MovedOutOfByNameEmptyArg { package_name, call_package_path, file } => { let call_package_arg = if let Some(path) = &call_package_path { - format!("./{}", path.display()) + format!("./{path}") } else { "...".into() }; - if *empty_arg { - write!( - f, - "pkgs.{package_name}: This top-level package was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ }}` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.", - structure::relative_file_for_package(package_name).display(), - ) - } else { - // This can happen if users mistakenly assume that for custom arguments, - // pkgs/by-name can't be used. - write!( - f, - "pkgs.{package_name}: This top-level package was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.", - structure::relative_file_for_package(package_name).display(), - ) - } + let relative_package_file = structure::relative_file_for_package(package_name); + writedoc!( + f, + " + - Attribute `pkgs.{package_name}` was previously defined in {relative_package_file}, but is now manually defined as `callPackage {call_package_arg} {{ /* ... */ }}` in {file}. + Please move the package back and remove the manual `callPackage`. + ", + ) }, - NixpkgsProblem::NewPackageNotUsingByName { package_name, call_package_path, empty_arg } => { + NixpkgsProblem::MovedOutOfByNameNonEmptyArg { package_name, call_package_path, file } => { let call_package_arg = if let Some(path) = &call_package_path { - format!("./{}", path.display()) + format!("./{}", path) } else { "...".into() }; - let extra = - if *empty_arg { - "Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore." + let relative_package_file = structure::relative_file_for_package(package_name); + // This can happen if users mistakenly assume that for custom arguments, + // pkgs/by-name can't be used. + writedoc!( + f, + " + - Attribute `pkgs.{package_name}` was previously defined in {relative_package_file}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` in {file}. + While the manual `callPackage` is still needed, it's not necessary to move the package files. + ", + ) + }, + NixpkgsProblem::NewPackageNotUsingByNameEmptyArg { package_name, call_package_path, file } => { + let call_package_arg = + if let Some(path) = &call_package_path { + format!("./{}", path) } else { - "Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed." + "...".into() }; - write!( + let relative_package_file = structure::relative_file_for_package(package_name); + writedoc!( + f, + " + - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`. + Please define it in {relative_package_file} instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {file} is needed anymore. + ", + ) + }, + NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg { package_name, call_package_path, file } => { + let call_package_arg = + if let Some(path) = &call_package_path { + format!("./{}", path) + } else { + "...".into() + }; + let relative_package_file = structure::relative_file_for_package(package_name); + writedoc!( f, - "pkgs.{package_name}: This is a new top-level package of the form `callPackage {call_package_arg} {{ }}`. Please define it in {} instead. See `pkgs/by-name/README.md` for more details. {extra}", - structure::relative_file_for_package(package_name).display(), + " + - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`. + Please define it in {relative_package_file} instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {file} is still needed. + ", ) }, NixpkgsProblem::InternalCallPackageUsed { attr_name } => @@ -271,3 +398,28 @@ impl fmt::Display for NixpkgsProblem { } } } + +fn indent_definition(column: usize, definition: String) -> String { + // The entire code should be indented 4 spaces + textwrap::indent( + // But first we want to strip the code's natural indentation + &textwrap::dedent( + // The definition _doesn't_ include the leading spaces, but we can + // recover those from the column + &format!("{}{definition}", " ".repeat(column - 1)), + ), + " ", + ) +} + +/// Creates a Nix path expression that when put into Nix file `from_file`, would point to the `to_file`. +fn create_path_expr( + from_file: impl AsRef<RelativePath>, + to_file: impl AsRef<RelativePath>, +) -> String { + // This `expect` calls should never trigger because we only call this function with files. + // That's why we `expect` them! + let from = from_file.as_ref().parent().expect("a parent for this path"); + let rel = from.relative(to_file); + format!("./{rel}") +} 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 200bf92c516a..8136d641c351 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs @@ -4,8 +4,8 @@ use crate::nix_file::CallPackageArgumentInfo; use crate::nixpkgs_problem::NixpkgsProblem; -use crate::structure; use crate::validation::{self, Validation, Validation::Success}; +use relative_path::RelativePathBuf; use std::collections::HashMap; /// The ratchet value for the entirety of Nixpkgs. @@ -127,17 +127,14 @@ impl<Context: ToNixpkgsProblem> RatchetState<Context> { pub enum ManualDefinition {} impl ToNixpkgsProblem for ManualDefinition { - type ToContext = (); + type ToContext = NixpkgsProblem; fn to_nixpkgs_problem( - name: &str, + _name: &str, _optional_from: Option<()>, - _to: &Self::ToContext, + to: &Self::ToContext, ) -> NixpkgsProblem { - NixpkgsProblem::WrongCallPackage { - relative_package_file: structure::relative_file_for_package(name), - package_name: name.to_owned(), - } + (*to).clone() } } @@ -149,24 +146,38 @@ impl ToNixpkgsProblem for ManualDefinition { pub enum UsesByName {} impl ToNixpkgsProblem for UsesByName { - type ToContext = CallPackageArgumentInfo; + type ToContext = (CallPackageArgumentInfo, RelativePathBuf); fn to_nixpkgs_problem( name: &str, optional_from: Option<()>, - to: &Self::ToContext, + (to, file): &Self::ToContext, ) -> NixpkgsProblem { if let Some(()) = optional_from { - NixpkgsProblem::MovedOutOfByName { + if to.empty_arg { + NixpkgsProblem::MovedOutOfByNameEmptyArg { + package_name: name.to_owned(), + call_package_path: to.relative_path.clone(), + file: file.to_owned(), + } + } else { + NixpkgsProblem::MovedOutOfByNameNonEmptyArg { + package_name: name.to_owned(), + call_package_path: to.relative_path.clone(), + file: file.to_owned(), + } + } + } else if to.empty_arg { + NixpkgsProblem::NewPackageNotUsingByNameEmptyArg { package_name: name.to_owned(), call_package_path: to.relative_path.clone(), - empty_arg: to.empty_arg, + file: file.to_owned(), } } else { - NixpkgsProblem::NewPackageNotUsingByName { + NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg { package_name: name.to_owned(), call_package_path: to.relative_path.clone(), - empty_arg: to.empty_arg, + file: file.to_owned(), } } } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/references.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/references.rs index 169e996300ba..e2319163ccc6 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -2,6 +2,7 @@ use crate::nixpkgs_problem::NixpkgsProblem; use crate::utils; use crate::validation::{self, ResultIteratorExt, Validation::Success}; use crate::NixFileStore; +use relative_path::RelativePath; use anyhow::Context; use rowan::ast::AstNode; @@ -12,14 +13,14 @@ use std::path::Path; /// Both symlinks and Nix path expressions are checked. pub fn check_references( nix_file_store: &mut NixFileStore, - relative_package_dir: &Path, + relative_package_dir: &RelativePath, absolute_package_dir: &Path, ) -> validation::Result<()> { // The first subpath to check is the package directory itself, which we can represent as an // empty path, since the absolute package directory gets prepended to this. // We don't use `./.` to keep the error messages cleaner // (there's no canonicalisation going on underneath) - let subpath = Path::new(""); + let subpath = RelativePath::new(""); check_path( nix_file_store, relative_package_dir, @@ -29,7 +30,7 @@ pub fn check_references( .with_context(|| { format!( "While checking the references in package directory {}", - relative_package_dir.display() + relative_package_dir ) }) } @@ -41,11 +42,11 @@ pub fn check_references( /// The absolute package directory gets prepended before doing anything with it though. fn check_path( nix_file_store: &mut NixFileStore, - relative_package_dir: &Path, + relative_package_dir: &RelativePath, absolute_package_dir: &Path, - subpath: &Path, + subpath: &RelativePath, ) -> validation::Result<()> { - let path = absolute_package_dir.join(subpath); + let path = subpath.to_path(absolute_package_dir); Ok(if path.is_symlink() { // Check whether the symlink resolves to outside the package directory @@ -55,8 +56,8 @@ fn check_path( // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(absolute_package_dir) { NixpkgsProblem::OutsideSymlink { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), + relative_package_dir: relative_package_dir.to_owned(), + subpath: subpath.to_owned(), } .into() } else { @@ -64,9 +65,9 @@ fn check_path( } } Err(io_error) => NixpkgsProblem::UnresolvableSymlink { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), - io_error, + relative_package_dir: relative_package_dir.to_owned(), + subpath: subpath.to_owned(), + io_error: io_error.to_string(), } .into(), } @@ -80,11 +81,12 @@ fn check_path( nix_file_store, relative_package_dir, absolute_package_dir, - &subpath.join(entry.file_name()), + // TODO: The relative_path crate doesn't seem to support OsStr + &subpath.join(entry.file_name().to_string_lossy().to_string()), ) }) .collect_vec() - .with_context(|| format!("Error while recursing into {}", subpath.display()))?, + .with_context(|| format!("Error while recursing into {}", subpath))?, ) } else if path.is_file() { // Only check Nix files @@ -96,7 +98,7 @@ fn check_path( absolute_package_dir, subpath, ) - .with_context(|| format!("Error while checking Nix file {}", subpath.display()))? + .with_context(|| format!("Error while checking Nix file {}", subpath))? } else { Success(()) } @@ -105,7 +107,7 @@ fn check_path( } } else { // This should never happen, git doesn't support other file types - anyhow::bail!("Unsupported file type for path {}", subpath.display()); + anyhow::bail!("Unsupported file type for path {}", subpath); }) } @@ -113,11 +115,11 @@ fn check_path( /// directory fn check_nix_file( nix_file_store: &mut NixFileStore, - relative_package_dir: &Path, + relative_package_dir: &RelativePath, absolute_package_dir: &Path, - subpath: &Path, + subpath: &RelativePath, ) -> validation::Result<()> { - let path = absolute_package_dir.join(subpath); + let path = subpath.to_path(absolute_package_dir); let nix_file = nix_file_store.get(&path)?; @@ -135,32 +137,32 @@ fn check_nix_file( match nix_file.static_resolve_path(path, absolute_package_dir) { ResolvedPath::Interpolated => NixpkgsProblem::PathInterpolation { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), + relative_package_dir: relative_package_dir.to_owned(), + subpath: subpath.to_owned(), line, text, } .into(), ResolvedPath::SearchPath => NixpkgsProblem::SearchPath { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), + relative_package_dir: relative_package_dir.to_owned(), + subpath: subpath.to_owned(), line, text, } .into(), ResolvedPath::Outside => NixpkgsProblem::OutsidePathReference { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), + relative_package_dir: relative_package_dir.to_owned(), + subpath: subpath.to_owned(), line, text, } .into(), ResolvedPath::Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), + relative_package_dir: relative_package_dir.to_owned(), + subpath: subpath.to_owned(), line, text, - io_error: e, + io_error: e.to_string(), } .into(), ResolvedPath::Within(..) => { diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 9b615dd9969a..09806bc3d4dc 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -7,8 +7,9 @@ use crate::NixFileStore; use itertools::concat; use lazy_static::lazy_static; use regex::Regex; +use relative_path::RelativePathBuf; use std::fs::DirEntry; -use std::path::{Path, PathBuf}; +use std::path::Path; lazy_static! { static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^[a-z0-9_-]{1,2}$").unwrap(); @@ -21,15 +22,15 @@ pub fn shard_for_package(package_name: &str) -> String { package_name.to_lowercase().chars().take(2).collect() } -pub fn relative_dir_for_shard(shard_name: &str) -> PathBuf { - PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}")) +pub fn relative_dir_for_shard(shard_name: &str) -> RelativePathBuf { + RelativePathBuf::from(format!("{BASE_SUBPATH}/{shard_name}")) } -pub fn relative_dir_for_package(package_name: &str) -> PathBuf { +pub fn relative_dir_for_package(package_name: &str) -> RelativePathBuf { relative_dir_for_shard(&shard_for_package(package_name)).join(package_name) } -pub fn relative_file_for_package(package_name: &str) -> PathBuf { +pub fn relative_file_for_package(package_name: &str) -> RelativePathBuf { relative_dir_for_package(package_name).join(PACKAGE_NIX_FILENAME) } @@ -120,7 +121,8 @@ fn check_package( ) -> validation::Result<String> { let package_path = package_entry.path(); let package_name = package_entry.file_name().to_string_lossy().into_owned(); - let relative_package_dir = PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}")); + let relative_package_dir = + RelativePathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}")); Ok(if !package_path.is_dir() { NixpkgsProblem::PackageNonDir { @@ -174,7 +176,7 @@ fn check_package( let result = result.and(references::check_references( nix_file_store, &relative_package_dir, - &path.join(&relative_package_dir), + &relative_package_dir.to_path(path), )?); result.map(|_| package_name.clone()) diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/aliases/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/aliases/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/aliases/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/all-packages.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/all-packages.nix new file mode 100644 index 000000000000..399f8eee0a18 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/all-packages.nix @@ -0,0 +1,7 @@ +self: super: { + + alt.callPackage = self.lib.callPackageWith {}; + + foo = self.alt.callPackage ({ }: self.someDrv) { }; + +} diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/default.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/default.nix new file mode 100644 index 000000000000..861260cdca4b --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/default.nix @@ -0,0 +1 @@ +import <test-nixpkgs> { root = ./.; } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected new file mode 100644 index 000000000000..1d92e652200e --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected @@ -0,0 +1,9 @@ +- Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined like + + foo = callPackage ./pkgs/by-name/fo/foo/package.nix { /* ... */ }; + + However, in this PR, a different `callPackage` is used. See the definition in all-packages.nix:5: + + foo = self.alt.callPackage ({ }: self.someDrv) { }; + +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/pkgs/by-name/fo/foo/package.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/pkgs/by-name/fo/foo/package.nix new file mode 100644 index 000000000000..a1b92efbbadb --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/pkgs/by-name/fo/foo/package.nix @@ -0,0 +1 @@ +{ someDrv }: someDrv diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/default.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/default.nix new file mode 100644 index 000000000000..861260cdca4b --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/default.nix @@ -0,0 +1 @@ +import <test-nixpkgs> { root = ./.; } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/pkgs/by-name/foo b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/pkgs/by-name/foo new file mode 100644 index 000000000000..e69de29bb2d1 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/base/pkgs/by-name/foo diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/default.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/default.nix new file mode 100644 index 000000000000..861260cdca4b --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/default.nix @@ -0,0 +1 @@ +import <test-nixpkgs> { root = ./.; } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected new file mode 100644 index 000000000000..e209e1855314 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected @@ -0,0 +1 @@ +The base branch is broken, but this PR fixes it. Nice job! diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/pkgs/by-name/README.md b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/pkgs/by-name/README.md new file mode 100644 index 000000000000..e69de29bb2d1 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/pkgs/by-name/README.md diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/default.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/default.nix new file mode 100644 index 000000000000..861260cdca4b --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/default.nix @@ -0,0 +1 @@ +import <test-nixpkgs> { root = ./.; } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/pkgs/by-name/foo b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/pkgs/by-name/foo new file mode 100644 index 000000000000..e69de29bb2d1 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/base/pkgs/by-name/foo diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/default.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/default.nix new file mode 100644 index 000000000000..861260cdca4b --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/default.nix @@ -0,0 +1 @@ +import <test-nixpkgs> { root = ./.; } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected new file mode 100644 index 000000000000..c25f06b4150e --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected @@ -0,0 +1,3 @@ +pkgs/by-name/bar: This is a file, but it should be a directory. +The base branch is broken and still has above problems with this PR, which need to be fixed first. +Consider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/pkgs/by-name/bar b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/pkgs/by-name/bar new file mode 100644 index 000000000000..e69de29bb2d1 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/pkgs/by-name/bar diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected index fff17c6c7cd5..15b3e3ff6ede 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected @@ -1 +1,2 @@ pkgs.foo: This attribute is not defined but it should be defined automatically as pkgs/by-name/fo/foo/package.nix +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/empty-base/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/empty-base/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/empty-base/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected index 3627368c0ef0..3b0146cdc146 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected @@ -1 +1,2 @@ pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected index 404795ee5c79..b3d0c6fc1a40 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected @@ -1 +1,2 @@ pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected index 8c8eafdcb3d1..80f6e7dd5998 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected @@ -1 +1,2 @@ pkgs/by-name/fo/fo@: Invalid package directory name "fo@", must be ASCII characters consisting of a-z, A-Z, 0-9, "-" or "_". +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected index 248aa8877966..7ca9ff8565bd 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected @@ -1 +1,2 @@ pkgs/by-name/A: Invalid directory name "A", must be at most 2 ASCII characters consisting of a-z, 0-9, "-" or "_". +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected index 29d33f7dbdc0..4d906ec0d086 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected @@ -1,2 +1,21 @@ -pkgs.noEval: 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 pkgs/by-name/no/noEval/package.nix { ... }` with a non-empty second argument. -pkgs.onlyMove: 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 pkgs/by-name/on/onlyMove/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/no/noEval exists, the attribute `pkgs.noEval` must be defined like + + noEval = callPackage ./pkgs/by-name/no/noEval/package.nix { /* ... */ }; + + However, in this PR, the second argument is empty. See the definition in all-packages.nix:9: + + noEval = self.callPackage ./pkgs/by-name/no/noEval/package.nix { }; + + Such a definition is provided automatically and therefore not necessary. Please remove it. + +- Because pkgs/by-name/on/onlyMove exists, the attribute `pkgs.onlyMove` must be defined like + + onlyMove = callPackage ./pkgs/by-name/on/onlyMove/package.nix { /* ... */ }; + + However, in this PR, the second argument is empty. See the definition in all-packages.nix:7: + + onlyMove = self.callPackage ./pkgs/by-name/on/onlyMove/package.nix { }; + + Such a definition is provided automatically and therefore not necessary. Please remove it. + +This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected index ce1afcbf2d34..1b67704817cf 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected @@ -1 +1,2 @@ pkgs/by-name/fo/foo: Missing required "package.nix" file. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix index 81a9c916ac2d..fbd51f656138 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/mock-nixpkgs.nix @@ -28,13 +28,22 @@ let lib = import <test-nixpkgs/lib>; # The base fixed-point function to populate the resulting attribute set - pkgsFun = self: { - inherit lib; - newScope = extra: lib.callPackageWith (self // extra); - callPackage = self.newScope { }; - callPackages = lib.callPackagesWith self; - someDrv = { type = "derivation"; }; - }; + pkgsFun = + self: { + inherit lib; + newScope = extra: lib.callPackageWith (self // extra); + callPackage = self.newScope { }; + callPackages = lib.callPackagesWith self; + } + # This mapAttrs is a very hacky workaround necessary because for all attributes defined in Nixpkgs, + # the files that define them are verified to be within Nixpkgs. + # This is usually a very safe assumption, but it fails in these tests for someDrv, + # because it's technically defined outside the Nixpkgs directories of each test case. + # By using `mapAttrs`, `builtins.unsafeGetAttrPos` just returns `null`, + # which then doesn't trigger this check + // lib.mapAttrs (name: value: value) { + someDrv = { type = "derivation"; }; + }; baseDirectory = root + "/pkgs/by-name"; diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected index 96da50b52491..123e24daab8a 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected @@ -1,4 +1,13 @@ -pkgs.foo1: This top-level package was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`. -pkgs.foo2: This top-level package was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`. -pkgs.foo3: This top-level package was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files. -pkgs.foo4: This top-level package was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files. +- Attribute `pkgs.foo1` was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { /* ... */ }` in all-packages.nix. + Please move the package back and remove the manual `callPackage`. + +- Attribute `pkgs.foo2` was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { /* ... */ }` in all-packages.nix. + Please move the package back and remove the manual `callPackage`. + +- Attribute `pkgs.foo3` was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` in all-packages.nix. + While the manual `callPackage` is still needed, it's not necessary to move the package files. + +- Attribute `pkgs.foo4` was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` in all-packages.nix. + While the manual `callPackage` is still needed, it's not necessary to move the package files. + +This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected index ff5d18556ef0..0105b19078c7 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected @@ -11,3 +11,4 @@ pkgs/by-name/ba/foo: File package.nix at line 3 contains the path expression ".. pkgs/by-name/ba/foo: File package.nix at line 4 contains the nix search path expression "<nixpkgs>" which may point outside the directory of that package. pkgs/by-name/ba/foo: File package.nix at line 5 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package. pkgs/by-name/fo/foo: Missing required "package.nix" file. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected index 3f294f26dfd8..92668a231b48 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected @@ -1,4 +1,21 @@ -pkgs.new1: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new1/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore. -pkgs.new2: This is a new top-level package of the form `callPackage ./without-config.nix { }`. Please define it in pkgs/by-name/ne/new2/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore. -pkgs.new3: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new3/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed. -pkgs.new4: This is a new top-level package of the form `callPackage ./with-config.nix { }`. Please define it in pkgs/by-name/ne/new4/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed. +- Attribute `pkgs.new1` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. + Please define it in pkgs/by-name/ne/new1/package.nix instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore. + +- Attribute `pkgs.new2` is a new top-level package using `pkgs.callPackage ./without-config.nix { /* ... */ }`. + Please define it in pkgs/by-name/ne/new2/package.nix instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore. + +- Attribute `pkgs.new3` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. + Please define it in pkgs/by-name/ne/new3/package.nix instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is not `{ }`, the manual `callPackage` in all-packages.nix is still needed. + +- Attribute `pkgs.new4` is a new top-level package using `pkgs.callPackage ./with-config.nix { /* ... */ }`. + Please define it in pkgs/by-name/ne/new4/package.nix instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is not `{ }`, the manual `callPackage` in all-packages.nix is still needed. + +This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-by-name/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-by-name/expected index ddcb2df46e5f..defae2634c34 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-by-name/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-by-name/expected @@ -1 +1 @@ -Given Nixpkgs path does not contain a pkgs/by-name subdirectory, no check necessary. +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-eval/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-eval/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/no-eval/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected index e6c923790102..1705d22be798 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected @@ -1 +1,2 @@ pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected index e6c923790102..1705d22be798 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected @@ -1 +1,2 @@ pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected index 9df788191ece..e09e931bb658 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected @@ -1 +1,9 @@ -pkgs.foo: 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 pkgs/by-name/fo/foo/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined like + + foo = callPackage ./pkgs/by-name/fo/foo/package.nix { /* ... */ }; + + However, in this PR, it isn't defined that way. See the definition in all-packages.nix:4 + + foo = self.bar; + +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/one-letter/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/one-letter/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/one-letter/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/only-callPackage-derivations/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/only-callPackage-derivations/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/only-callPackage-derivations/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected index 51479e22d26f..16292c0c0eb1 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected @@ -1 +1,9 @@ -pkgs.nonDerivation: 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 pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like + + nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; + + However, in this PR, the first `callPackage` argument is the wrong path. See the definition in all-packages.nix:2: + + nonDerivation = callPackage ./someDrv.nix { /* ... */ }; + +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg-gradual/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg-gradual/expected index e69de29bb2d1..defae2634c34 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg-gradual/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg-gradual/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected index 51479e22d26f..f3306aadbbb7 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected @@ -1 +1,11 @@ -pkgs.nonDerivation: 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 pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like + + nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; + + However, in this PR, the second argument is empty. See the definition in all-packages.nix:2: + + nonDerivation = self.callPackage ./pkgs/by-name/no/nonDerivation/package.nix { }; + + Such a definition is provided automatically and therefore not necessary. Please remove it. + +This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected index 51479e22d26f..d91d58d629f2 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected @@ -1 +1,9 @@ -pkgs.nonDerivation: 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 pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like + + nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; + + However, in this PR, it isn't defined that way. See the definition in all-packages.nix:2 + + nonDerivation = self.someDrv; + +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected index 51479e22d26f..807c440dd3d2 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected @@ -1 +1,9 @@ -pkgs.nonDerivation: 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 pkgs/by-name/no/nonDerivation/package.nix { ... }` with a non-empty second argument. +- Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like + + nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; + + However, in this PR, the first `callPackage` argument is not a path. See the definition in all-packages.nix:2: + + nonDerivation = self.callPackage ({ someDrv }: someDrv) { }; + +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/all-packages.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/all-packages.nix new file mode 100644 index 000000000000..f07e7a42744a --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/all-packages.nix @@ -0,0 +1,5 @@ +self: super: { + + foo = self.callPackage ({ someDrv, someFlag }: someDrv) { someFlag = true; }; + +} diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/default.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/default.nix new file mode 100644 index 000000000000..861260cdca4b --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/default.nix @@ -0,0 +1 @@ +import <test-nixpkgs> { root = ./.; } diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected new file mode 100644 index 000000000000..4adbaf66edc0 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected @@ -0,0 +1,9 @@ +- Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined like + + foo = callPackage ./pkgs/by-name/fo/foo/package.nix { /* ... */ }; + + However, in this PR, the first `callPackage` argument is not a path. See the definition in all-packages.nix:3: + + foo = self.callPackage ({ someDrv, someFlag }: someDrv) { someFlag = true; }; + +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/pkgs/by-name/fo/foo/package.nix b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/pkgs/by-name/fo/foo/package.nix new file mode 100644 index 000000000000..a1b92efbbadb --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/pkgs/by-name/fo/foo/package.nix @@ -0,0 +1 @@ +{ someDrv }: someDrv diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-success/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-success/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/override-success/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected index 3ad4b8f820f5..adee1d0137fa 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected @@ -1 +1,2 @@ pkgs/by-name/fo/foo: This path is a file, but it should be a directory. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected index 67a0c69fe29e..d03e1eceea26 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected @@ -1 +1,2 @@ pkgs/by-name/fo/foo: "package.nix" must be a file. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-variants/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-variants/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/package-variants/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected index 7d20c32aad68..0bdb00f20cb9 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected @@ -1 +1,2 @@ pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "/foo" which cannot be resolved: No such file or directory (os error 2). +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected index 3d7fb64e80a3..2e4338ccc7ae 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected @@ -1 +1,2 @@ pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "../." which may point outside the directory of that package. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected index b0cdff4a4778..30125570794a 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected @@ -1 +1,2 @@ pkgs/by-name/aa/aa: File package.nix at line 2 contains the nix search path expression "<nixpkgs>" which may point outside the directory of that package. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected index ad662af27a86..6567439b77f8 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected @@ -1 +1,2 @@ pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-success/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-success/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/ref-success/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected index 447b38e6b6c1..689cee41f1e3 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected @@ -1 +1,2 @@ pkgs/by-name/fo: This is a file, but it should be a directory. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected index 349e9ad47c41..8a8104b73720 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected @@ -1,4 +1,31 @@ -pkgs.a: 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 pkgs/by-name/a/a/package.nix { ... }` with a non-empty second argument. -pkgs.b: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/b/b/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore. -pkgs.c: 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 pkgs/by-name/c/c/package.nix { ... }` with a non-empty second argument. -pkgs.d: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/d/d/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore. +- Because pkgs/by-name/a/a exists, the attribute `pkgs.a` must be defined like + + a = callPackage ./pkgs/by-name/a/a/package.nix { /* ... */ }; + + However, in this PR, the second argument is empty. See the definition in all-packages.nix:2: + + a = self.callPackage ./pkgs/by-name/a/a/package.nix { }; + + Such a definition is provided automatically and therefore not necessary. Please remove it. + +- Attribute `pkgs.b` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. + Please define it in pkgs/by-name/b/b/package.nix instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore. + +- Because pkgs/by-name/c/c exists, the attribute `pkgs.c` must be defined like + + c = callPackage ./pkgs/by-name/c/c/package.nix { /* ... */ }; + + However, in this PR, the second argument is empty. See the definition in all-packages.nix:4: + + c = self.callPackage ./pkgs/by-name/c/c/package.nix { }; + + Such a definition is provided automatically and therefore not necessary. Please remove it. + +- Attribute `pkgs.d` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`. + Please define it in pkgs/by-name/d/d/package.nix instead. + See `pkgs/by-name/README.md` for more details. + Since the second `callPackage` argument is `{ }`, no manual `callPackage` in all-packages.nix is needed anymore. + +This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/success/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/success/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/success/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected index 335c5d6b6e5d..cd555c658a09 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected @@ -1 +1,2 @@ pkgs/by-name/fo/foo: Path package.nix is a symlink pointing to a path outside the directory of that package. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected index c1e7a28205a7..1b06bcf4972b 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected @@ -1 +1,2 @@ pkgs/by-name/fo/foo: Path foo is a symlink which cannot be resolved: No such file or directory (os error 2). +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected index 2a248c23ab50..608843d93903 100644 --- a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected @@ -1 +1,2 @@ pkgs.foo: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`. +This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break. diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/uppercase/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/uppercase/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/uppercase/expected @@ -0,0 +1 @@ +Validated successfully diff --git a/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/with-readme/expected b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/with-readme/expected new file mode 100644 index 000000000000..defae2634c34 --- /dev/null +++ b/nixpkgs/pkgs/test/nixpkgs-check-by-name/tests/with-readme/expected @@ -0,0 +1 @@ +Validated successfully |