From 5d7723dd5ba1ee02595f03554acc139b627b3834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Fri, 29 Sep 2023 19:46:10 +0200 Subject: [PATCH 01/52] nixos/switch-to-configuration: Lock the switch This prevents any concurrent switches from happening which is not an issue I have seen people complaining about but it seems like a good measure. --- nixos/modules/system/activation/switch-to-configuration.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index e05f89bb0fb4..31b24f855285 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -22,6 +22,7 @@ use JSON::PP; use IPC::Cmd; use Sys::Syslog qw(:standard :macros); use Cwd qw(abs_path); +use Fcntl ':flock'; ## no critic(ControlStructures::ProhibitDeepNests) ## no critic(ErrorHandling::RequireCarping) @@ -91,6 +92,8 @@ if (!-f "/etc/NIXOS" && (read_file("/etc/os-release", err_mode => "quiet") // "" } make_path("/run/nixos", { mode => oct(755) }); +open(my $stc_lock, '>>', '/run/nixos/switch-to-configuration.lock') or die "Could not open lock - $!"; +flock($stc_lock, LOCK_EX) or die "Could not acquire lock - $!"; openlog("nixos", "", LOG_USER); # Install or update the bootloader. @@ -983,4 +986,5 @@ if ($res == 0) { syslog(LOG_ERR, "switching to system configuration $toplevel failed (status $res)"); } +close($stc_lock) or die "Could not close lock - $!"; exit($res); From 2342298229c864082f38580ae7a5b54525095f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Fri, 6 Oct 2023 22:45:30 +0200 Subject: [PATCH 02/52] openssh: enable ldns ldns is used for validating DNSSEC responses. With ldns enabled, using SSHFP records on DNSSEC signed zones allows connecting to ssh servers with host keys being automatically validated. --- pkgs/tools/networking/openssh/common.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/tools/networking/openssh/common.nix b/pkgs/tools/networking/openssh/common.nix index 2b6ea743fec1..27661ae58fe7 100644 --- a/pkgs/tools/networking/openssh/common.nix +++ b/pkgs/tools/networking/openssh/common.nix @@ -18,11 +18,13 @@ , zlib , openssl , libedit +, ldns , pkg-config , pam , libredirect , etcDir ? null , withKerberos ? true +, withLdns ? true , libkrb5 , libfido2 , hostname @@ -72,6 +74,7 @@ stdenv.mkDerivation { buildInputs = [ zlib openssl libedit ] ++ lib.optional withFIDO libfido2 ++ lib.optional withKerberos libkrb5 + ++ lib.optional withLdns ldns ++ lib.optional withPAM pam; preConfigure = '' @@ -95,6 +98,7 @@ stdenv.mkDerivation { ++ lib.optional withKerberos (assert libkrb5 != null; "--with-kerberos5=${libkrb5}") ++ lib.optional stdenv.isDarwin "--disable-libutil" ++ lib.optional (!linkOpenssl) "--without-openssl" + ++ lib.optional withLdns "--with-ldns" ++ extraConfigureFlags; ${if stdenv.hostPlatform.isStatic then "NIX_LDFLAGS" else null}= [ "-laudit" ] ++ lib.optionals withKerberos [ "-lkeyutils" ]; From 741025d9b11c7c5fabb05d8a03a9be43f204cf31 Mon Sep 17 00:00:00 2001 From: Florian Brandes Date: Sun, 22 Oct 2023 11:15:17 +0200 Subject: [PATCH 03/52] python3Packages.flask-security-too: fix build Signed-off-by: Florian Brandes --- .../python-modules/flask-security-too/default.nix | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkgs/development/python-modules/flask-security-too/default.nix b/pkgs/development/python-modules/flask-security-too/default.nix index 529a1a63913a..0eae72147262 100644 --- a/pkgs/development/python-modules/flask-security-too/default.nix +++ b/pkgs/development/python-modules/flask-security-too/default.nix @@ -21,6 +21,7 @@ # extras: mfa , cryptography , phonenumbers +, webauthn # propagates , blinker @@ -31,10 +32,10 @@ , flask-wtf , itsdangerous , passlib +, importlib-resources # tests , argon2-cffi -, flask-mongoengine , mongoengine , mongomock , peewee @@ -47,7 +48,7 @@ buildPythonPackage rec { pname = "flask-security-too"; version = "5.3.0"; - format = "setuptools"; + format = "pyproject"; disabled = pythonOlder "3.7"; @@ -71,6 +72,7 @@ buildPythonPackage rec { flask-wtf itsdangerous passlib + importlib-resources ]; passthru.optional-dependencies = { @@ -92,12 +94,12 @@ buildPythonPackage rec { mfa = [ cryptography phonenumbers + webauthn ]; }; nativeCheckInputs = [ argon2-cffi - flask-mongoengine mongoengine mongomock peewee @@ -112,6 +114,11 @@ buildPythonPackage rec { ++ passthru.optional-dependencies.mfa; + disabledTests = [ + # needs /etc/resolv.conf + "test_login_email_whatever" + ]; + pythonImportsCheck = [ "flask_security" ]; From 37f8f6681c761a5788b633f1da8f1f8a940bfabc Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 02:24:29 +0200 Subject: [PATCH 04/52] tests.nixpkgs-check-by-name: Intermediate error type refactoring prep Currently the tool prints problems right as it is checking the code without an intermediate error representation. However for various reasons it would be beneficial to have an intermediate error type: - It makes the code cleaner, having all errors in one place - It allows printing the error in different ways, e.g. for a future --json mode This commit prepares for an incremental refactoring for an intermediate error/problem representation. Most notable is that we want to be able to collect multiple errors/problems and not just exit on the first one. We introduce the type alias CheckResult and CheckError (later renamed to NixpkgsProblem), where CheckError allows collecting multiple CheckErrors using the utility function flatten_check_results (later renamed to check_result::sequence) The write_check_result function is only temporarily introduced to help refactoring, it's removed again in later commits. --- pkgs/test/nixpkgs-check-by-name/Cargo.lock | 16 +++++ pkgs/test/nixpkgs-check-by-name/Cargo.toml | 1 + .../nixpkgs-check-by-name/src/check_result.rs | 61 +++++++++++++++++++ pkgs/test/nixpkgs-check-by-name/src/main.rs | 1 + 4 files changed, 79 insertions(+) create mode 100644 pkgs/test/nixpkgs-check-by-name/src/check_result.rs diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.lock b/pkgs/test/nixpkgs-check-by-name/Cargo.lock index aa4459c7cff8..fc3aeb9fd79b 100644 --- a/pkgs/test/nixpkgs-check-by-name/Cargo.lock +++ b/pkgs/test/nixpkgs-check-by-name/Cargo.lock @@ -162,6 +162,12 @@ version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7704b5fdd17b18ae31c4c1da5a2e0305a2bf17b5249300a9ee9ed7b72114c636" +[[package]] +name = "either" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" + [[package]] name = "errno" version = "0.3.2" @@ -218,6 +224,15 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "itertools" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.9" @@ -274,6 +289,7 @@ dependencies = [ "anyhow", "clap", "colored", + "itertools", "lazy_static", "regex", "rnix", diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.toml b/pkgs/test/nixpkgs-check-by-name/Cargo.toml index 70b44d048200..1e6eaa1106d5 100644 --- a/pkgs/test/nixpkgs-check-by-name/Cargo.toml +++ b/pkgs/test/nixpkgs-check-by-name/Cargo.toml @@ -13,6 +13,7 @@ serde = { version = "1.0.185", features = ["derive"] } anyhow = "1.0" lazy_static = "1.4.0" colored = "2.0.4" +itertools = "0.11.0" [dev-dependencies] temp-env = "0.3.5" diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs new file mode 100644 index 000000000000..df894df45c71 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -0,0 +1,61 @@ +use crate::ErrorWriter; +use itertools::{Either, Itertools}; +use std::fmt; +use std::io; +use std::path::PathBuf; + +pub enum CheckError {} + +impl CheckError { + pub fn into_result(self) -> CheckResult { + Ok(Either::Left(vec![self])) + } +} + +impl fmt::Display for CheckError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self {} + } +} + +pub fn write_check_result( + error_writer: &mut ErrorWriter, + check_result: CheckResult, +) -> anyhow::Result> { + match check_result? { + Either::Left(errors) => { + for error in errors { + error_writer.write(&error.to_string())? + } + Ok(None) + } + Either::Right(value) => Ok(Some(value)), + } +} + +pub fn pass(value: A) -> CheckResult { + Ok(Either::Right(value)) +} + +pub type CheckResult = anyhow::Result, A>>; + +pub fn flatten_check_results( + check_results: impl IntoIterator>, + value_transform: impl Fn(Vec) -> O, +) -> CheckResult { + let (errors, values): (Vec<_>, Vec<_>) = check_results + .into_iter() + .collect::>>()? + .into_iter() + .partition_map(|r| r); + + // To combine the errors from the results we flatten all the error Vec's into a new Vec + // This is not very efficient, but doesn't matter because generally we should have no errors + let flattened_errors = errors.into_iter().flatten().collect::>(); + + if flattened_errors.is_empty() { + Ok(Either::Right(value_transform(values))) + } else { + Ok(Either::Left(flattened_errors)) + } +} diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 5d28077ae4ae..ee98c5a1a893 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -1,3 +1,4 @@ +mod check_result; mod eval; mod references; mod structure; From ed56d74c089d6b058a28eaf4a5ef04b190ed3651 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 02:25:24 +0200 Subject: [PATCH 05/52] tests.nixpkgs-check-by-name: Intermediate path reference errors --- .../nixpkgs-check-by-name/src/check_result.rs | 35 +++++++++++++++++-- .../nixpkgs-check-by-name/src/references.rs | 32 ++++++++++------- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index df894df45c71..79d3dbe67662 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -4,7 +4,21 @@ use std::fmt; use std::io; use std::path::PathBuf; -pub enum CheckError {} +pub enum CheckError { + OutsidePathReference { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + }, + UnresolvablePathReference { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + io_error: io::Error, + }, +} impl CheckError { pub fn into_result(self) -> CheckResult { @@ -14,7 +28,24 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self {} + match self { + CheckError::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, + ), + CheckError::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, + ), + } } } diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 16dc60729c42..30eaee0d7484 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,3 +1,4 @@ +use crate::check_result::{pass, write_check_result, CheckError}; use crate::structure::Nixpkgs; use crate::utils; use crate::utils::{ErrorWriter, LineIndex}; @@ -155,29 +156,34 @@ fn check_nix_file( // Resolves the reference of the Nix path // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz` - match parent_dir.join(Path::new(&text)).canonicalize() { + let check_result = match parent_dir.join(Path::new(&text)).canonicalize() { Ok(target) => { // Then checking if it's still in the package directory // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - context.error_writer.write(&format!( - "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.", - context.relative_package_dir.display(), - subpath.display(), + CheckError::OutsidePathReference { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, text, - ))?; + } + .into_result() + } else { + pass(()) } } - Err(e) => { - context.error_writer.write(&format!( - "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {e}.", - context.relative_package_dir.display(), - subpath.display(), - text, - ))?; + Err(e) => CheckError::UnresolvablePathReference { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + io_error: e, } + .into_result(), }; + + write_check_result(context.error_writer, check_result)?; } Ok(()) From a755aa7d0251e4282ebdfdd33cbb00382b9a004c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 02:32:09 +0200 Subject: [PATCH 06/52] tests.nixpkgs-check-by-name: Intermediate SearchPath error --- .../nixpkgs-check-by-name/src/check_result.rs | 14 ++++ .../nixpkgs-check-by-name/src/references.rs | 64 +++++++++---------- 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 79d3dbe67662..cc004e012931 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -5,6 +5,12 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + SearchPath { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + }, OutsidePathReference { relative_package_dir: PathBuf, subpath: PathBuf, @@ -29,6 +35,14 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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 + ), CheckError::OutsidePathReference { relative_package_dir, subpath, line, text } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 30eaee0d7484..dacac9c9ee5c 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -144,43 +144,43 @@ fn check_nix_file( } // Filters out search paths like - if text.starts_with('<') { - context.error_writer.write(&format!( - "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.", - context.relative_package_dir.display(), - subpath.display(), - text - ))?; - continue; - } - - // Resolves the reference of the Nix path - // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz` - let check_result = match parent_dir.join(Path::new(&text)).canonicalize() { - Ok(target) => { - // Then checking if it's still in the package directory - // No need to handle the case of it being inside the directory, since we scan through the - // entire directory recursively anyways - if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - CheckError::OutsidePathReference { - relative_package_dir: context.relative_package_dir.clone(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into_result() - } else { - pass(()) - } - } - Err(e) => CheckError::UnresolvablePathReference { + let check_result = if text.starts_with('<') { + CheckError::SearchPath { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, text, - io_error: e, } - .into_result(), + .into_result() + } else { + // Resolves the reference of the Nix path + // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz` + match parent_dir.join(Path::new(&text)).canonicalize() { + Ok(target) => { + // Then checking if it's still in the package directory + // No need to handle the case of it being inside the directory, since we scan through the + // entire directory recursively anyways + if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { + CheckError::OutsidePathReference { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + } + .into_result() + } else { + pass(()) + } + } + Err(e) => CheckError::UnresolvablePathReference { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + io_error: e, + } + .into_result(), + } }; write_check_result(context.error_writer, check_result)?; From 96f6a350fa74e995dbbf750b17cad2d6cbb3186e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 02:42:22 +0200 Subject: [PATCH 07/52] tests.nixpkgs-check-by-name: Intermediate PathInterpolation error --- .../nixpkgs-check-by-name/src/check_result.rs | 14 ++++++++++++ .../nixpkgs-check-by-name/src/references.rs | 22 +++++++++---------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index cc004e012931..ed00f383f193 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -5,6 +5,12 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + PathInterpolation { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + }, SearchPath { relative_package_dir: PathBuf, subpath: PathBuf, @@ -35,6 +41,14 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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 + ), CheckError::SearchPath { relative_package_dir, subpath, line, text } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index dacac9c9ee5c..6cc933e721a3 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -133,18 +133,16 @@ fn check_nix_file( // Filters out ./foo/${bar}/baz // TODO: We can just check ./foo - if node.children().count() != 0 { - context.error_writer.write(&format!( - "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.", - context.relative_package_dir.display(), - subpath.display(), - text - ))?; - continue; - } - - // Filters out search paths like - let check_result = if text.starts_with('<') { + let check_result = if node.children().count() != 0 { + CheckError::PathInterpolation { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + } + .into_result() + } else if text.starts_with('<') { + // Filters out search paths like CheckError::SearchPath { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), From 9a3abc4383da1a430525902b87db02ddcfc279ee Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 02:50:15 +0200 Subject: [PATCH 08/52] tests.nixpkgs-check-by-name: Intermediate CouldNotParseNix error --- .../nixpkgs-check-by-name/src/check_result.rs | 14 ++++++ .../nixpkgs-check-by-name/src/references.rs | 49 +++++++++---------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index ed00f383f193..42468a29e809 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -1,10 +1,16 @@ use crate::ErrorWriter; use itertools::{Either, Itertools}; +use rnix::parser::ParseError; use std::fmt; use std::io; use std::path::PathBuf; pub enum CheckError { + CouldNotParseNix { + relative_package_dir: PathBuf, + subpath: PathBuf, + error: ParseError, + }, PathInterpolation { relative_package_dir: PathBuf, subpath: PathBuf, @@ -41,6 +47,14 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::CouldNotParseNix { relative_package_dir, subpath, error } => + write!( + f, + "{}: File {} could not be parsed by rnix: {}", + relative_package_dir.display(), + subpath.display(), + error, + ), CheckError::PathInterpolation { relative_package_dir, subpath, line, text } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 6cc933e721a3..9c88507ff99b 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,4 +1,6 @@ -use crate::check_result::{pass, write_check_result, CheckError}; +use crate::check_result::{ + flatten_check_results, pass, write_check_result, CheckError, CheckResult, +}; use crate::structure::Nixpkgs; use crate::utils; use crate::utils::{ErrorWriter, LineIndex}; @@ -81,10 +83,11 @@ fn check_path(context: &mut PackageContext, subpath: &Path) -> // Only check Nix files if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { - check_nix_file(context, subpath).context(format!( + let check_result = check_nix_file(context, subpath).context(format!( "Error while checking Nix file {}", subpath.display() - ))? + )); + write_check_result(context.error_writer, check_result)?; } } } else { @@ -99,7 +102,7 @@ fn check_path(context: &mut PackageContext, subpath: &Path) -> fn check_nix_file( context: &mut PackageContext, subpath: &Path, -) -> anyhow::Result<()> { +) -> CheckResult<()> { let path = context.absolute_package_dir.join(subpath); let parent_dir = path.parent().context(format!( "Could not get parent of path {}", @@ -111,29 +114,26 @@ fn check_nix_file( let root = Root::parse(&contents); if let Some(error) = root.errors().first() { - context.error_writer.write(&format!( - "{}: File {} could not be parsed by rnix: {}", - context.relative_package_dir.display(), - subpath.display(), - error, - ))?; - return Ok(()); + return CheckError::CouldNotParseNix { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + error: error.clone(), + } + .into_result(); } let line_index = LineIndex::new(&contents); - for node in root.syntax().descendants() { - // We're only interested in Path expressions - if node.kind() != NODE_PATH { - continue; - } - + let check_results = root.syntax().descendants().map(|node| { let text = node.text().to_string(); let line = line_index.line(node.text_range().start().into()); - // Filters out ./foo/${bar}/baz - // TODO: We can just check ./foo - let check_result = if node.children().count() != 0 { + if node.kind() != NODE_PATH { + // We're only interested in Path expressions + pass(()) + } else if node.children().count() != 0 { + // Filters out ./foo/${bar}/baz + // TODO: We can just check ./foo CheckError::PathInterpolation { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), @@ -179,10 +179,7 @@ fn check_nix_file( } .into_result(), } - }; - - write_check_result(context.error_writer, check_result)?; - } - - Ok(()) + } + }); + flatten_check_results(check_results, |_| ()) } From 4897b63ae67d6f56a0d9a7f0db421467ebe8828b Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 23:07:54 +0200 Subject: [PATCH 09/52] tests.nixpkgs-check-by-name: Intermediate Symlink errors --- .../nixpkgs-check-by-name/src/check_result.rs | 23 ++++++++++ .../nixpkgs-check-by-name/src/references.rs | 42 +++++++++++-------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 42468a29e809..32d43edc153a 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -6,6 +6,15 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + OutsideSymlink { + relative_package_dir: PathBuf, + subpath: PathBuf, + }, + UnresolvableSymlink { + relative_package_dir: PathBuf, + subpath: PathBuf, + io_error: io::Error, + }, CouldNotParseNix { relative_package_dir: PathBuf, subpath: PathBuf, @@ -47,6 +56,20 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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(), + ), + CheckError::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(), + ), CheckError::CouldNotParseNix { relative_package_dir, subpath, error } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 9c88507ff99b..f72074483b14 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -52,26 +52,28 @@ fn check_path(context: &mut PackageContext, subpath: &Path) -> if path.is_symlink() { // Check whether the symlink resolves to outside the package directory - match path.canonicalize() { + let check_result = match path.canonicalize() { Ok(target) => { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - context.error_writer.write(&format!( - "{}: Path {} is a symlink pointing to a path outside the directory of that package.", - context.relative_package_dir.display(), - subpath.display(), - ))?; + CheckError::OutsideSymlink { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + } + .into_result() + } else { + pass(()) } } - Err(e) => { - context.error_writer.write(&format!( - "{}: Path {} is a symlink which cannot be resolved: {e}.", - context.relative_package_dir.display(), - subpath.display(), - ))?; + Err(io_error) => CheckError::UnresolvableSymlink { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + io_error, } - } + .into_result(), + }; + write_check_result(context.error_writer, check_result)?; } else if path.is_dir() { // Recursively check each entry for entry in utils::read_dir_sorted(&path)? { @@ -81,15 +83,19 @@ fn check_path(context: &mut PackageContext, subpath: &Path) -> } } else if path.is_file() { // Only check Nix files - if let Some(ext) = path.extension() { + let check_result = if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { - let check_result = check_nix_file(context, subpath).context(format!( + check_nix_file(context, subpath).context(format!( "Error while checking Nix file {}", subpath.display() - )); - write_check_result(context.error_writer, check_result)?; + )) + } else { + pass(()) } - } + } else { + pass(()) + }; + write_check_result(context.error_writer, check_result)?; } else { // This should never happen, git doesn't support other file types anyhow::bail!("Unsupported file type for path {}", subpath.display()); From 9a0ef886236eefb13ea84422c5c8ba7865ccf6f8 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 23:19:13 +0200 Subject: [PATCH 10/52] tests.nixpkgs-check-by-name: Intermediate NonDerivation error --- .../nixpkgs-check-by-name/src/check_result.rs | 10 ++++ pkgs/test/nixpkgs-check-by-name/src/eval.rs | 17 +++--- pkgs/test/nixpkgs-check-by-name/src/main.rs | 3 +- .../nixpkgs-check-by-name/src/references.rs | 53 +++++++------------ 4 files changed, 43 insertions(+), 40 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 32d43edc153a..4f1ee9f674f6 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -6,6 +6,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + NonDerivation { + relative_package_file: PathBuf, + package_name: String, + }, OutsideSymlink { relative_package_dir: PathBuf, subpath: PathBuf, @@ -56,6 +60,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::NonDerivation { relative_package_file, package_name } => + write!( + f, + "pkgs.{package_name}: This attribute defined by {} is not a derivation", + relative_package_file.display() + ), CheckError::OutsideSymlink { relative_package_dir, subpath } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 26836501c97a..695d52c0a8f2 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,3 +1,4 @@ +use crate::check_result::{pass, write_check_result, CheckError}; use crate::structure; use crate::utils::ErrorWriter; use crate::Version; @@ -144,12 +145,16 @@ pub fn check_values( continue; } - if !attribute_info.is_derivation { - error_writer.write(&format!( - "pkgs.{package_name}: This attribute defined by {} is not a derivation", - relative_package_file.display() - ))?; - } + let check_result = if !attribute_info.is_derivation { + CheckError::NonDerivation { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into_result() + } else { + pass(()) + }; + write_check_result(error_writer, check_result)?; } else { error_writer.write(&format!( "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index ee98c5a1a893..80ed29646f72 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -5,6 +5,7 @@ mod structure; mod utils; use anyhow::Context; +use check_result::write_check_result; use clap::{Parser, ValueEnum}; use colored::Colorize; use std::io; @@ -92,7 +93,7 @@ pub fn check_nixpkgs( if error_writer.empty { // Only if we could successfully parse the structure, we do the semantic checks eval::check_values(version, &mut error_writer, &nixpkgs, eval_accessible_paths)?; - references::check_references(&mut error_writer, &nixpkgs)?; + write_check_result(&mut error_writer, references::check_references(&nixpkgs))?; } } Ok(error_writer.empty) diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index f72074483b14..b932b556f54f 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,20 +1,16 @@ -use crate::check_result::{ - flatten_check_results, pass, write_check_result, CheckError, CheckResult, -}; +use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult}; use crate::structure::Nixpkgs; use crate::utils; -use crate::utils::{ErrorWriter, LineIndex}; +use crate::utils::LineIndex; use anyhow::Context; use rnix::{Root, SyntaxKind::NODE_PATH}; use std::ffi::OsStr; use std::fs::read_to_string; -use std::io; use std::path::{Path, PathBuf}; /// Small helper so we don't need to pass in the same arguments to all functions -struct PackageContext<'a, W: io::Write> { - error_writer: &'a mut ErrorWriter, +struct PackageContext<'a> { /// The package directory relative to Nixpkgs, such as `pkgs/by-name/fo/foo` relative_package_dir: &'a PathBuf, /// The absolute package directory @@ -23,36 +19,32 @@ struct PackageContext<'a, W: io::Write> { /// Check that every package directory in pkgs/by-name doesn't link to outside that directory. /// Both symlinks and Nix path expressions are checked. -pub fn check_references( - error_writer: &mut ErrorWriter, - nixpkgs: &Nixpkgs, -) -> anyhow::Result<()> { +pub fn check_references(nixpkgs: &Nixpkgs) -> CheckResult<()> { // Check the directories for each package separately - for package_name in &nixpkgs.package_names { + let check_results = nixpkgs.package_names.iter().map(|package_name| { let relative_package_dir = Nixpkgs::relative_dir_for_package(package_name); - let mut context = PackageContext { - error_writer, + let context = PackageContext { relative_package_dir: &relative_package_dir, absolute_package_dir: &nixpkgs.path.join(&relative_package_dir), }; // The empty argument here is the subpath under the package directory to check // An empty one means the package directory itself - check_path(&mut context, Path::new("")).context(format!( + check_path(&context, Path::new("")).context(format!( "While checking the references in package directory {}", relative_package_dir.display() - ))?; - } - Ok(()) + )) + }); + flatten_check_results(check_results, |_| ()) } /// Checks for a specific path to not have references outside -fn check_path(context: &mut PackageContext, subpath: &Path) -> anyhow::Result<()> { +fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let path = context.absolute_package_dir.join(subpath); if path.is_symlink() { // Check whether the symlink resolves to outside the package directory - let check_result = match path.canonicalize() { + match path.canonicalize() { Ok(target) => { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways @@ -72,18 +64,18 @@ fn check_path(context: &mut PackageContext, subpath: &Path) -> io_error, } .into_result(), - }; - write_check_result(context.error_writer, check_result)?; + } } else if path.is_dir() { // Recursively check each entry - for entry in utils::read_dir_sorted(&path)? { + let check_results = utils::read_dir_sorted(&path)?.into_iter().map(|entry| { let entry_subpath = subpath.join(entry.file_name()); check_path(context, &entry_subpath) - .context(format!("Error while recursing into {}", subpath.display()))? - } + .context(format!("Error while recursing into {}", subpath.display())) + }); + flatten_check_results(check_results, |_| ()) } else if path.is_file() { // Only check Nix files - let check_result = if let Some(ext) = path.extension() { + if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { check_nix_file(context, subpath).context(format!( "Error while checking Nix file {}", @@ -94,21 +86,16 @@ fn check_path(context: &mut PackageContext, subpath: &Path) -> } } else { pass(()) - }; - write_check_result(context.error_writer, check_result)?; + } } else { // This should never happen, git doesn't support other file types anyhow::bail!("Unsupported file type for path {}", subpath.display()); } - Ok(()) } /// Check whether a nix file contains path expression references pointing outside the package /// directory -fn check_nix_file( - context: &mut PackageContext, - subpath: &Path, -) -> CheckResult<()> { +fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let path = context.absolute_package_dir.join(subpath); let parent_dir = path.parent().context(format!( "Could not get parent of path {}", From b688da8189f69a760d140400f3f19bd188754b57 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 23:46:06 +0200 Subject: [PATCH 11/52] tests.nixpkgs-check-by-name: Intermediate WrongCallPackage error --- .../nixpkgs-check-by-name/src/check_result.rs | 10 ++++++++++ pkgs/test/nixpkgs-check-by-name/src/eval.rs | 16 +++++++--------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 4f1ee9f674f6..6b1d120de8fa 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -6,6 +6,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + WrongCallPackage { + relative_package_file: PathBuf, + package_name: String, + }, NonDerivation { relative_package_file: PathBuf, package_name: String, @@ -60,6 +64,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::WrongCallPackage { relative_package_file, package_name } => + write!( + f, + "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.", + relative_package_file.display() + ), CheckError::NonDerivation { relative_package_file, package_name } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 695d52c0a8f2..d1c5135b7d85 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -137,15 +137,13 @@ pub fn check_values( AttributeVariant::Other => false, }; - if !valid { - error_writer.write(&format!( - "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.", - relative_package_file.display() - ))?; - continue; - } - - let check_result = if !attribute_info.is_derivation { + let check_result = if !valid { + CheckError::WrongCallPackage { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into_result() + } else if !attribute_info.is_derivation { CheckError::NonDerivation { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), From 4f17b9367d7535fc5308557b92e1b306662b3d3d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 23:48:31 +0200 Subject: [PATCH 12/52] tests.nixpkgs-check-by-name: Intermediate UndefinedAttr error --- .../nixpkgs-check-by-name/src/check_result.rs | 10 ++++++++++ pkgs/test/nixpkgs-check-by-name/src/eval.rs | 20 +++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 6b1d120de8fa..f546b1285450 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -6,6 +6,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + UndefinedAttr { + relative_package_file: PathBuf, + package_name: String, + }, WrongCallPackage { relative_package_file: PathBuf, package_name: String, @@ -64,6 +68,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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() + ), CheckError::WrongCallPackage { relative_package_file, package_name } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index d1c5135b7d85..81cf43a381ed 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -116,7 +116,7 @@ pub fn check_values( let relative_package_file = structure::Nixpkgs::relative_file_for_package(package_name); let absolute_package_file = nixpkgs.path.join(&relative_package_file); - if let Some(attribute_info) = actual_files.get(package_name) { + let check_result = if let Some(attribute_info) = actual_files.get(package_name) { let valid = match &attribute_info.variant { AttributeVariant::AutoCalled => true, AttributeVariant::CallPackage { path, empty_arg } => { @@ -137,7 +137,7 @@ pub fn check_values( AttributeVariant::Other => false, }; - let check_result = if !valid { + if !valid { CheckError::WrongCallPackage { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), @@ -151,15 +151,15 @@ pub fn check_values( .into_result() } else { pass(()) - }; - write_check_result(error_writer, check_result)?; + } } else { - error_writer.write(&format!( - "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", - relative_package_file.display() - ))?; - continue; - } + CheckError::UndefinedAttr { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into_result() + }; + write_check_result(error_writer, check_result)?; } Ok(()) } From e3979d14cda13b4a6cb15ebccf6052cc67c0db4c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 23:54:34 +0200 Subject: [PATCH 13/52] tests.nixpkgs-check-by-name: Intermediate PackageNixDir error --- .../nixpkgs-check-by-name/src/check_result.rs | 10 ++++++++++ pkgs/test/nixpkgs-check-by-name/src/eval.rs | 20 ++++++++----------- pkgs/test/nixpkgs-check-by-name/src/main.rs | 20 ++++++++++++------- .../nixpkgs-check-by-name/src/structure.rs | 15 +++++++------- pkgs/test/nixpkgs-check-by-name/src/utils.rs | 3 +++ 5 files changed, 41 insertions(+), 27 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index f546b1285450..13d780cfd56e 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -1,3 +1,4 @@ +use crate::utils::PACKAGE_NIX_FILENAME; use crate::ErrorWriter; use itertools::{Either, Itertools}; use rnix::parser::ParseError; @@ -6,6 +7,9 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + PackageNixDir { + relative_package_dir: PathBuf, + }, UndefinedAttr { relative_package_file: PathBuf, package_name: String, @@ -68,6 +72,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::PackageNixDir { relative_package_dir } => + write!( + f, + "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", + relative_package_dir.display(), + ), CheckError::UndefinedAttr { relative_package_file, package_name } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 81cf43a381ed..eb90a295a577 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,13 +1,11 @@ -use crate::check_result::{pass, write_check_result, CheckError}; +use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult}; use crate::structure; -use crate::utils::ErrorWriter; use crate::Version; use std::path::Path; use anyhow::Context; use serde::Deserialize; use std::collections::HashMap; -use std::io; use std::path::PathBuf; use std::process; use tempfile::NamedTempFile; @@ -41,12 +39,11 @@ const EXPR: &str = include_str!("eval.nix"); /// Check that the Nixpkgs attribute values corresponding to the packages in pkgs/by-name are /// of the form `callPackage { ... }`. /// See the `eval.nix` file for how this is achieved on the Nix side -pub fn check_values( +pub fn check_values( version: Version, - error_writer: &mut ErrorWriter, nixpkgs: &structure::Nixpkgs, eval_accessible_paths: Vec<&Path>, -) -> anyhow::Result<()> { +) -> CheckResult<()> { // Write the list of packages we need to check into a temporary JSON file. // This can then get read by the Nix evaluation. let attrs_file = NamedTempFile::new().context("Failed to create a temporary file")?; @@ -112,11 +109,11 @@ pub fn check_values( String::from_utf8_lossy(&result.stdout) ))?; - for package_name in &nixpkgs.package_names { + let check_results = nixpkgs.package_names.iter().map(|package_name| { let relative_package_file = structure::Nixpkgs::relative_file_for_package(package_name); let absolute_package_file = nixpkgs.path.join(&relative_package_file); - let check_result = if let Some(attribute_info) = actual_files.get(package_name) { + if let Some(attribute_info) = actual_files.get(package_name) { let valid = match &attribute_info.variant { AttributeVariant::AutoCalled => true, AttributeVariant::CallPackage { path, empty_arg } => { @@ -158,8 +155,7 @@ pub fn check_values( package_name: package_name.clone(), } .into_result() - }; - write_check_result(error_writer, check_result)?; - } - Ok(()) + } + }); + flatten_check_results(check_results, |_| ()) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 80ed29646f72..7de6eadf6564 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -5,7 +5,7 @@ mod structure; mod utils; use anyhow::Context; -use check_result::write_check_result; +use check_result::{flatten_check_results, write_check_result}; use clap::{Parser, ValueEnum}; use colored::Colorize; use std::io; @@ -82,18 +82,24 @@ pub fn check_nixpkgs( // at all. Later used to figure out if the structure was valid or not. let mut error_writer = ErrorWriter::new(error_writer); - if !nixpkgs_path.join(structure::BASE_SUBPATH).exists() { + if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() { eprintln!( "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.", - structure::BASE_SUBPATH + utils::BASE_SUBPATH ); } else { let nixpkgs = Nixpkgs::new(&nixpkgs_path, &mut error_writer)?; if error_writer.empty { // Only if we could successfully parse the structure, we do the semantic checks - eval::check_values(version, &mut error_writer, &nixpkgs, eval_accessible_paths)?; - write_check_result(&mut error_writer, references::check_references(&nixpkgs))?; + let check_result = flatten_check_results( + [ + eval::check_values(version, &nixpkgs, eval_accessible_paths), + references::check_references(&nixpkgs), + ], + |_| (), + ); + write_check_result(&mut error_writer, check_result)?; } } Ok(error_writer.empty) @@ -102,7 +108,7 @@ pub fn check_nixpkgs( #[cfg(test)] mod tests { use crate::check_nixpkgs; - use crate::structure; + use crate::utils; use crate::Version; use anyhow::Context; use std::fs; @@ -147,7 +153,7 @@ mod tests { return Ok(()); } - let base = path.join(structure::BASE_SUBPATH); + let base = path.join(utils::BASE_SUBPATH); fs::create_dir_all(base.join("fo/foo"))?; fs::write(base.join("fo/foo/package.nix"), "{ someDrv }: someDrv")?; diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index ea80128e487a..928d5ea088ec 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,14 +1,12 @@ +use crate::check_result::{write_check_result, CheckError}; use crate::utils; -use crate::utils::ErrorWriter; +use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; use regex::Regex; use std::collections::HashMap; use std::io; use std::path::{Path, PathBuf}; -pub const BASE_SUBPATH: &str = "pkgs/by-name"; -pub const PACKAGE_NIX_FILENAME: &str = "package.nix"; - lazy_static! { static ref SHARD_NAME_REGEX: Regex = Regex::new(r"^[a-z0-9_-]{1,2}$").unwrap(); static ref PACKAGE_NAME_REGEX: Regex = Regex::new(r"^[a-zA-Z0-9_-]+$").unwrap(); @@ -134,10 +132,11 @@ impl Nixpkgs { relative_package_dir.display(), ))?; } else if package_nix_path.is_dir() { - error_writer.write(&format!( - "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", - relative_package_dir.display(), - ))?; + let check_result = CheckError::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result::<()>(); + write_check_result(error_writer, check_result)?; } package_names.push(package_name.clone()); diff --git a/pkgs/test/nixpkgs-check-by-name/src/utils.rs b/pkgs/test/nixpkgs-check-by-name/src/utils.rs index 325c736eca98..728e50e72336 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/utils.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/utils.rs @@ -4,6 +4,9 @@ use std::fs; use std::io; use std::path::Path; +pub const BASE_SUBPATH: &str = "pkgs/by-name"; +pub const PACKAGE_NIX_FILENAME: &str = "package.nix"; + /// Deterministic file listing so that tests are reproducible pub fn read_dir_sorted(base_dir: &Path) -> anyhow::Result> { let listing = base_dir From 64f5eb616e673babd2b54cc07394b04b22242493 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 00:10:29 +0200 Subject: [PATCH 14/52] tests.nixpkgs-check-by-name: Intermediate PackageNixNonExistent error --- .../nixpkgs-check-by-name/src/check_result.rs | 9 +++++++ .../nixpkgs-check-by-name/src/structure.rs | 25 +++++++++++-------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 13d780cfd56e..882ad8c8e2ca 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -7,6 +7,9 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + PackageNixNonExistent { + relative_package_dir: PathBuf, + }, PackageNixDir { relative_package_dir: PathBuf, }, @@ -72,6 +75,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::PackageNixNonExistent { relative_package_dir } => + write!( + f, + "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", + relative_package_dir.display(), + ), CheckError::PackageNixDir { relative_package_dir } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 928d5ea088ec..5a60dc26954b 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,4 +1,4 @@ -use crate::check_result::{write_check_result, CheckError}; +use crate::check_result::{pass, write_check_result, CheckError}; use crate::utils; use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; @@ -126,18 +126,21 @@ impl Nixpkgs { } let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - if !package_nix_path.exists() { - error_writer.write(&format!( - "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", - relative_package_dir.display(), - ))?; - } else if package_nix_path.is_dir() { - let check_result = CheckError::PackageNixDir { + let check_result = if !package_nix_path.exists() { + CheckError::PackageNixNonExistent { relative_package_dir: relative_package_dir.clone(), } - .into_result::<()>(); - write_check_result(error_writer, check_result)?; - } + .into_result() + } else if package_nix_path.is_dir() { + CheckError::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) + }; + + write_check_result(error_writer, check_result)?; package_names.push(package_name.clone()); } From b011d53bda903f70d884ca42a43b65db9b430901 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 00:30:18 +0200 Subject: [PATCH 15/52] tests.nixpkgs-check-by-name: Intermediate IncorrectShard error --- .../nixpkgs-check-by-name/src/check_result.rs | 11 ++++++++ .../nixpkgs-check-by-name/src/structure.rs | 25 ++++++++++++------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 882ad8c8e2ca..e42652039c2c 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -7,6 +7,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + IncorrectShard { + relative_package_dir: PathBuf, + correct_relative_package_dir: PathBuf, + }, PackageNixNonExistent { relative_package_dir: PathBuf, }, @@ -75,6 +79,13 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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(), + ), CheckError::PackageNixNonExistent { relative_package_dir } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 5a60dc26954b..8dc9ca61940a 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,4 +1,4 @@ -use crate::check_result::{pass, write_check_result, CheckError}; +use crate::check_result::{flatten_check_results, pass, write_check_result, CheckError}; use crate::utils; use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; @@ -113,20 +113,24 @@ impl Nixpkgs { } let correct_relative_package_dir = Nixpkgs::relative_dir_for_package(&package_name); - if relative_package_dir != correct_relative_package_dir { + let shard_check_result = if relative_package_dir != correct_relative_package_dir { // Only show this error if we have a valid shard and package name // Because if one of those is wrong, you should fix that first if shard_name_valid && package_name_valid { - error_writer.write(&format!( - "{}: Incorrect directory location, should be {} instead.", - relative_package_dir.display(), - correct_relative_package_dir.display(), - ))?; + CheckError::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: correct_relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) } - } + } else { + pass(()) + }; let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let check_result = if !package_nix_path.exists() { + let package_nix_check_result = if !package_nix_path.exists() { CheckError::PackageNixNonExistent { relative_package_dir: relative_package_dir.clone(), } @@ -140,6 +144,9 @@ impl Nixpkgs { pass(()) }; + let check_result = + flatten_check_results([shard_check_result, package_nix_check_result], |_| ()); + write_check_result(error_writer, check_result)?; package_names.push(package_name.clone()); From e7d9cc96ed0f8edeb52a2dbeefdcf00e4aedf8ee Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 00:33:39 +0200 Subject: [PATCH 16/52] tests.nixpkgs-check-by-name: Intermediate InvalidPackageName error --- .../nixpkgs-check-by-name/src/check_result.rs | 10 ++++++++ .../nixpkgs-check-by-name/src/structure.rs | 25 +++++++++++++------ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index e42652039c2c..ff90ea7c5092 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -7,6 +7,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + InvalidPackageName { + relative_package_dir: PathBuf, + package_name: String, + }, IncorrectShard { relative_package_dir: PathBuf, correct_relative_package_dir: PathBuf, @@ -79,6 +83,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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(), + ), CheckError::IncorrectShard { relative_package_dir, correct_relative_package_dir } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 8dc9ca61940a..e2d03d83a53f 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -105,12 +105,15 @@ impl Nixpkgs { } let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - if !package_name_valid { - error_writer.write(&format!( - "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", - relative_package_dir.display(), - ))?; - } + let name_check_result = if !package_name_valid { + CheckError::InvalidPackageName { + relative_package_dir: relative_package_dir.clone(), + package_name: package_name.clone(), + } + .into_result() + } else { + pass(()) + }; let correct_relative_package_dir = Nixpkgs::relative_dir_for_package(&package_name); let shard_check_result = if relative_package_dir != correct_relative_package_dir { @@ -144,8 +147,14 @@ impl Nixpkgs { pass(()) }; - let check_result = - flatten_check_results([shard_check_result, package_nix_check_result], |_| ()); + let check_result = flatten_check_results( + [ + name_check_result, + shard_check_result, + package_nix_check_result, + ], + |_| (), + ); write_check_result(error_writer, check_result)?; From 935f82267a84a69a7aa66b2b849fc57f15d24685 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 00:55:55 +0200 Subject: [PATCH 17/52] tests.nixpkgs-check-by-name: Intermediate CaseSensitiveDuplicate error --- .../nixpkgs-check-by-name/src/check_result.rs | 12 +++++++ .../nixpkgs-check-by-name/src/structure.rs | 33 ++++++++++++------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index ff90ea7c5092..2bceadbf807c 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -2,11 +2,17 @@ use crate::utils::PACKAGE_NIX_FILENAME; use crate::ErrorWriter; use itertools::{Either, Itertools}; use rnix::parser::ParseError; +use std::ffi::OsString; use std::fmt; use std::io; use std::path::PathBuf; pub enum CheckError { + CaseSensitiveDuplicate { + relative_shard_path: PathBuf, + first: OsString, + second: OsString, + }, InvalidPackageName { relative_package_dir: PathBuf, package_name: String, @@ -83,6 +89,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::CaseSensitiveDuplicate { relative_shard_path, first, second } => + write!( + f, + "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.", + relative_shard_path.display(), + ), CheckError::InvalidPackageName { relative_package_dir, package_name } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index e2d03d83a53f..f64f80d381aa 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -3,7 +3,6 @@ use crate::utils; use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; use regex::Regex; -use std::collections::HashMap; use std::io; use std::path::{Path, PathBuf}; @@ -79,9 +78,28 @@ impl Nixpkgs { ))?; } - let mut unique_package_names = HashMap::new(); + let entries = utils::read_dir_sorted(&shard_path)?; - for package_entry in utils::read_dir_sorted(&shard_path)? { + let duplicate_check_results = entries + .iter() + .zip(entries.iter().skip(1)) + .filter(|(l, r)| { + l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() + }) + .map(|(l, r)| { + CheckError::CaseSensitiveDuplicate { + relative_shard_path: relative_shard_path.clone(), + first: l.file_name(), + second: r.file_name(), + } + .into_result::<()>() + }); + + let duplicate_check_result = flatten_check_results(duplicate_check_results, |_| ()); + + write_check_result(error_writer, duplicate_check_result)?; + + for package_entry in entries { let package_path = package_entry.path(); let package_name = package_entry.file_name().to_string_lossy().into_owned(); let relative_package_dir = @@ -95,15 +113,6 @@ impl Nixpkgs { continue; } - if let Some(duplicate_package_name) = - unique_package_names.insert(package_name.to_lowercase(), package_name.clone()) - { - error_writer.write(&format!( - "{}: Duplicate case-sensitive package directories \"{duplicate_package_name}\" and \"{package_name}\".", - relative_shard_path.display(), - ))?; - } - let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); let name_check_result = if !package_name_valid { CheckError::InvalidPackageName { From 143e267ad24e7872ed8adede446d2a9f95e4c409 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 01:03:48 +0200 Subject: [PATCH 18/52] tests.nixpkgs-check-by-name: Intermediate PackageNonDir error --- .../nixpkgs-check-by-name/src/check_result.rs | 9 ++ .../nixpkgs-check-by-name/src/structure.rs | 103 +++++++++--------- 2 files changed, 62 insertions(+), 50 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 2bceadbf807c..28d48fb59783 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -8,6 +8,9 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + PackageNonDir { + relative_package_dir: PathBuf, + }, CaseSensitiveDuplicate { relative_shard_path: PathBuf, first: OsString, @@ -89,6 +92,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::PackageNonDir { relative_package_dir } => + write!( + f, + "{}: This path is a file, but it should be a directory.", + relative_package_dir.display(), + ), CheckError::CaseSensitiveDuplicate { relative_shard_path, first, second } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index f64f80d381aa..e50f2113e633 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -105,69 +105,72 @@ impl Nixpkgs { let relative_package_dir = PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}")); - if !package_path.is_dir() { - error_writer.write(&format!( - "{}: This path is a file, but it should be a directory.", - relative_package_dir.display(), - ))?; - continue; - } - - let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - let name_check_result = if !package_name_valid { - CheckError::InvalidPackageName { + let check_result = if !package_path.is_dir() { + CheckError::PackageNonDir { relative_package_dir: relative_package_dir.clone(), - package_name: package_name.clone(), } .into_result() } else { - pass(()) - }; - - let correct_relative_package_dir = Nixpkgs::relative_dir_for_package(&package_name); - let shard_check_result = if relative_package_dir != correct_relative_package_dir { - // Only show this error if we have a valid shard and package name - // Because if one of those is wrong, you should fix that first - if shard_name_valid && package_name_valid { - CheckError::IncorrectShard { + let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); + let name_check_result = if !package_name_valid { + CheckError::InvalidPackageName { relative_package_dir: relative_package_dir.clone(), - correct_relative_package_dir: correct_relative_package_dir.clone(), + package_name: package_name.clone(), } .into_result() } else { pass(()) - } - } else { - pass(()) - }; + }; - let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let package_nix_check_result = if !package_nix_path.exists() { - CheckError::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - CheckError::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - pass(()) - }; + let correct_relative_package_dir = + Nixpkgs::relative_dir_for_package(&package_name); + let shard_check_result = if relative_package_dir != correct_relative_package_dir + { + // Only show this error if we have a valid shard and package name + // Because if one of those is wrong, you should fix that first + if shard_name_valid && package_name_valid { + CheckError::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: correct_relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) + } + } else { + pass(()) + }; - let check_result = flatten_check_results( - [ - name_check_result, - shard_check_result, - package_nix_check_result, - ], - |_| (), - ); + let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); + let package_nix_check_result = if !package_nix_path.exists() { + CheckError::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else if package_nix_path.is_dir() { + CheckError::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) + }; + + let check_result = flatten_check_results( + [ + name_check_result, + shard_check_result, + package_nix_check_result, + ], + |_| (), + ); + + package_names.push(package_name.clone()); + + check_result + }; write_check_result(error_writer, check_result)?; - - package_names.push(package_name.clone()); } } From b7ace0198cfc971a887358bdc8871c6f5c31cfb4 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 01:15:18 +0200 Subject: [PATCH 19/52] tests.nixpkgs-check-by-name: Intermediate InvalidShardName error --- .../nixpkgs-check-by-name/src/check_result.rs | 10 +++++ .../nixpkgs-check-by-name/src/structure.rs | 37 +++++++++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 28d48fb59783..ad3d949dcb49 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -8,6 +8,10 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + InvalidShardName { + relative_shard_path: PathBuf, + shard_name: String, + }, PackageNonDir { relative_package_dir: PathBuf, }, @@ -92,6 +96,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::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() + ), CheckError::PackageNonDir { relative_package_dir } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index e50f2113e633..fed70c97b090 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -71,12 +71,17 @@ impl Nixpkgs { } let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); - if !shard_name_valid { - error_writer.write(&format!( - "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", - relative_shard_path.display() - ))?; - } + let shard_name_valid_check_result = if !shard_name_valid { + CheckError::InvalidShardName { + relative_shard_path: relative_shard_path.clone(), + shard_name: shard_name.clone(), + } + .into_result() + } else { + pass(()) + }; + + write_check_result(error_writer, shard_name_valid_check_result)?; let entries = utils::read_dir_sorted(&shard_path)?; @@ -99,13 +104,13 @@ impl Nixpkgs { write_check_result(error_writer, duplicate_check_result)?; - for package_entry in entries { + let check_results = entries.into_iter().map(|package_entry| { 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 check_result = if !package_path.is_dir() { + if !package_path.is_dir() { CheckError::PackageNonDir { relative_package_dir: relative_package_dir.clone(), } @@ -156,21 +161,21 @@ impl Nixpkgs { pass(()) }; - let check_result = flatten_check_results( + flatten_check_results( [ name_check_result, shard_check_result, package_nix_check_result, ], - |_| (), - ); + |_| package_name.clone(), + ) + } + }); - package_names.push(package_name.clone()); + let check_result = flatten_check_results(check_results, |x| x); - check_result - }; - - write_check_result(error_writer, check_result)?; + if let Some(shard_package_names) = write_check_result(error_writer, check_result)? { + package_names.extend(shard_package_names) } } From 571eaed155dced7e29b4e2c6c40f63b0ce521a99 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 01:25:05 +0200 Subject: [PATCH 20/52] tests.nixpkgs-check-by-name: Intermediate ShardNonDir error --- .../nixpkgs-check-by-name/src/check_result.rs | 9 + .../nixpkgs-check-by-name/src/structure.rs | 184 +++++++++--------- 2 files changed, 101 insertions(+), 92 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index ad3d949dcb49..b970c7d63063 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -8,6 +8,9 @@ use std::io; use std::path::PathBuf; pub enum CheckError { + ShardNonDir { + relative_shard_path: PathBuf, + }, InvalidShardName { relative_shard_path: PathBuf, shard_name: String, @@ -96,6 +99,12 @@ impl CheckError { impl fmt::Display for CheckError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { + CheckError::ShardNonDir { relative_shard_path } => + write!( + f, + "{}: This is a file, but it should be a directory.", + relative_shard_path.display(), + ), CheckError::InvalidShardName { relative_shard_path, shard_name } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index fed70c97b090..17eece94d61c 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -61,118 +61,118 @@ impl Nixpkgs { continue; } - if !shard_path.is_dir() { - error_writer.write(&format!( - "{}: This is a file, but it should be a directory.", - relative_shard_path.display(), - ))?; - // we can't check for any other errors if it's a file, since there's no subdirectories to check - continue; - } - - let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); - let shard_name_valid_check_result = if !shard_name_valid { - CheckError::InvalidShardName { + let check_result = if !shard_path.is_dir() { + CheckError::ShardNonDir { relative_shard_path: relative_shard_path.clone(), - shard_name: shard_name.clone(), } .into_result() + // we can't check for any other errors if it's a file, since there's no subdirectories to check } else { - pass(()) - }; - - write_check_result(error_writer, shard_name_valid_check_result)?; - - let entries = utils::read_dir_sorted(&shard_path)?; - - let duplicate_check_results = entries - .iter() - .zip(entries.iter().skip(1)) - .filter(|(l, r)| { - l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() - }) - .map(|(l, r)| { - CheckError::CaseSensitiveDuplicate { + let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); + let shard_name_valid_check_result = if !shard_name_valid { + CheckError::InvalidShardName { relative_shard_path: relative_shard_path.clone(), - first: l.file_name(), - second: r.file_name(), - } - .into_result::<()>() - }); - - let duplicate_check_result = flatten_check_results(duplicate_check_results, |_| ()); - - write_check_result(error_writer, duplicate_check_result)?; - - let check_results = entries.into_iter().map(|package_entry| { - 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}")); - - if !package_path.is_dir() { - CheckError::PackageNonDir { - relative_package_dir: relative_package_dir.clone(), + shard_name: shard_name.clone(), } .into_result() } else { - let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - let name_check_result = if !package_name_valid { - CheckError::InvalidPackageName { + pass(()) + }; + + write_check_result(error_writer, shard_name_valid_check_result)?; + + let entries = utils::read_dir_sorted(&shard_path)?; + + let duplicate_check_results = entries + .iter() + .zip(entries.iter().skip(1)) + .filter(|(l, r)| { + l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() + }) + .map(|(l, r)| { + CheckError::CaseSensitiveDuplicate { + relative_shard_path: relative_shard_path.clone(), + first: l.file_name(), + second: r.file_name(), + } + .into_result::<()>() + }); + + let duplicate_check_result = flatten_check_results(duplicate_check_results, |_| ()); + + write_check_result(error_writer, duplicate_check_result)?; + + let check_results = entries.into_iter().map(|package_entry| { + 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}")); + + if !package_path.is_dir() { + CheckError::PackageNonDir { relative_package_dir: relative_package_dir.clone(), - package_name: package_name.clone(), } .into_result() } else { - pass(()) - }; - - let correct_relative_package_dir = - Nixpkgs::relative_dir_for_package(&package_name); - let shard_check_result = if relative_package_dir != correct_relative_package_dir - { - // Only show this error if we have a valid shard and package name - // Because if one of those is wrong, you should fix that first - if shard_name_valid && package_name_valid { - CheckError::IncorrectShard { + let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); + let name_check_result = if !package_name_valid { + CheckError::InvalidPackageName { relative_package_dir: relative_package_dir.clone(), - correct_relative_package_dir: correct_relative_package_dir.clone(), + package_name: package_name.clone(), } .into_result() } else { pass(()) - } - } else { - pass(()) - }; + }; - let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let package_nix_check_result = if !package_nix_path.exists() { - CheckError::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - CheckError::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - pass(()) - }; + let correct_relative_package_dir = + Nixpkgs::relative_dir_for_package(&package_name); + let shard_check_result = + if relative_package_dir != correct_relative_package_dir { + // Only show this error if we have a valid shard and package name + // Because if one of those is wrong, you should fix that first + if shard_name_valid && package_name_valid { + CheckError::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: correct_relative_package_dir + .clone(), + } + .into_result() + } else { + pass(()) + } + } else { + pass(()) + }; - flatten_check_results( - [ - name_check_result, - shard_check_result, - package_nix_check_result, - ], - |_| package_name.clone(), - ) - } - }); + let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); + let package_nix_check_result = if !package_nix_path.exists() { + CheckError::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else if package_nix_path.is_dir() { + CheckError::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) + }; - let check_result = flatten_check_results(check_results, |x| x); + flatten_check_results( + [ + name_check_result, + shard_check_result, + package_nix_check_result, + ], + |_| package_name.clone(), + ) + } + }); + + flatten_check_results(check_results, |x| x) + }; if let Some(shard_package_names) = write_check_result(error_writer, check_result)? { package_names.extend(shard_package_names) From bb89ca72dfd79d88f7e3de2460c96a0255ebac11 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 01:26:22 +0200 Subject: [PATCH 21/52] tests.nixpkgs-check-by-name: Refactor --- .../nixpkgs-check-by-name/src/structure.rs | 230 +++++++++--------- 1 file changed, 118 insertions(+), 112 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 17eece94d61c..2f225ecd2ec3 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -49,139 +49,145 @@ impl Nixpkgs { ) -> anyhow::Result { let base_dir = path.join(BASE_SUBPATH); - let mut package_names = Vec::new(); + let check_results = utils::read_dir_sorted(&base_dir)? + .into_iter() + .map(|shard_entry| { + let shard_path = shard_entry.path(); + let shard_name = shard_entry.file_name().to_string_lossy().into_owned(); + let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name); - for shard_entry in utils::read_dir_sorted(&base_dir)? { - let shard_path = shard_entry.path(); - let shard_name = shard_entry.file_name().to_string_lossy().into_owned(); - let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name); - - if shard_name == "README.md" { - // README.md is allowed to be a file and not checked - continue; - } - - let check_result = if !shard_path.is_dir() { - CheckError::ShardNonDir { - relative_shard_path: relative_shard_path.clone(), - } - .into_result() - // we can't check for any other errors if it's a file, since there's no subdirectories to check - } else { - let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); - let shard_name_valid_check_result = if !shard_name_valid { - CheckError::InvalidShardName { + if shard_name == "README.md" { + // README.md is allowed to be a file and not checked + pass(vec![]) + } else if !shard_path.is_dir() { + CheckError::ShardNonDir { relative_shard_path: relative_shard_path.clone(), - shard_name: shard_name.clone(), } .into_result() + // we can't check for any other errors if it's a file, since there's no subdirectories to check } else { - pass(()) - }; - - write_check_result(error_writer, shard_name_valid_check_result)?; - - let entries = utils::read_dir_sorted(&shard_path)?; - - let duplicate_check_results = entries - .iter() - .zip(entries.iter().skip(1)) - .filter(|(l, r)| { - l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() - }) - .map(|(l, r)| { - CheckError::CaseSensitiveDuplicate { + let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); + let shard_name_valid_check_result = if !shard_name_valid { + CheckError::InvalidShardName { relative_shard_path: relative_shard_path.clone(), - first: l.file_name(), - second: r.file_name(), - } - .into_result::<()>() - }); - - let duplicate_check_result = flatten_check_results(duplicate_check_results, |_| ()); - - write_check_result(error_writer, duplicate_check_result)?; - - let check_results = entries.into_iter().map(|package_entry| { - 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}")); - - if !package_path.is_dir() { - CheckError::PackageNonDir { - relative_package_dir: relative_package_dir.clone(), + shard_name: shard_name.clone(), } .into_result() } else { - let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - let name_check_result = if !package_name_valid { - CheckError::InvalidPackageName { + pass(()) + }; + + write_check_result(error_writer, shard_name_valid_check_result)?; + + let entries = utils::read_dir_sorted(&shard_path)?; + + let duplicate_check_results = entries + .iter() + .zip(entries.iter().skip(1)) + .filter(|(l, r)| { + l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() + }) + .map(|(l, r)| { + CheckError::CaseSensitiveDuplicate { + relative_shard_path: relative_shard_path.clone(), + first: l.file_name(), + second: r.file_name(), + } + .into_result::<()>() + }); + + let duplicate_check_result = + flatten_check_results(duplicate_check_results, |_| ()); + + write_check_result(error_writer, duplicate_check_result)?; + + let check_results = entries.into_iter().map(|package_entry| { + 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}")); + + if !package_path.is_dir() { + CheckError::PackageNonDir { relative_package_dir: relative_package_dir.clone(), - package_name: package_name.clone(), } .into_result() } else { - pass(()) - }; - - let correct_relative_package_dir = - Nixpkgs::relative_dir_for_package(&package_name); - let shard_check_result = - if relative_package_dir != correct_relative_package_dir { - // Only show this error if we have a valid shard and package name - // Because if one of those is wrong, you should fix that first - if shard_name_valid && package_name_valid { - CheckError::IncorrectShard { - relative_package_dir: relative_package_dir.clone(), - correct_relative_package_dir: correct_relative_package_dir - .clone(), - } - .into_result() - } else { - pass(()) + let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); + let name_check_result = if !package_name_valid { + CheckError::InvalidPackageName { + relative_package_dir: relative_package_dir.clone(), + package_name: package_name.clone(), } + .into_result() } else { pass(()) }; - let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let package_nix_check_result = if !package_nix_path.exists() { - CheckError::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - CheckError::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - pass(()) - }; + let correct_relative_package_dir = + Nixpkgs::relative_dir_for_package(&package_name); + let shard_check_result = + if relative_package_dir != correct_relative_package_dir { + // Only show this error if we have a valid shard and package name + // Because if one of those is wrong, you should fix that first + if shard_name_valid && package_name_valid { + CheckError::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: + correct_relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) + } + } else { + pass(()) + }; - flatten_check_results( - [ - name_check_result, - shard_check_result, - package_nix_check_result, - ], - |_| package_name.clone(), - ) - } - }); + let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); + let package_nix_check_result = if !package_nix_path.exists() { + CheckError::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else if package_nix_path.is_dir() { + CheckError::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + pass(()) + }; - flatten_check_results(check_results, |x| x) - }; + flatten_check_results( + [ + name_check_result, + shard_check_result, + package_nix_check_result, + ], + |_| package_name.clone(), + ) + } + }); - if let Some(shard_package_names) = write_check_result(error_writer, check_result)? { - package_names.extend(shard_package_names) - } + flatten_check_results(check_results, |x| x) + } + }); + + let check_result = flatten_check_results(check_results, |x| { + x.into_iter().flatten().collect::>() + }); + + if let Some(package_names) = write_check_result(error_writer, check_result)? { + Ok(Nixpkgs { + path: path.to_owned(), + package_names, + }) + } else { + Ok(Nixpkgs { + path: path.to_owned(), + package_names: vec![], + }) } - - Ok(Nixpkgs { - path: path.to_owned(), - package_names, - }) } } From 83b887504c9a4aaa4e25f89bbf3dc19074fba29e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 01:54:03 +0200 Subject: [PATCH 22/52] tests.nixpkgs-check-by-name: Support for combining check results --- .../nixpkgs-check-by-name/src/check_result.rs | 12 +++++++++++ .../nixpkgs-check-by-name/src/structure.rs | 21 +++++++++++-------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index b970c7d63063..818ba1a9afd7 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -1,5 +1,6 @@ use crate::utils::PACKAGE_NIX_FILENAME; use crate::ErrorWriter; +use itertools::concat; use itertools::{Either, Itertools}; use rnix::parser::ParseError; use std::ffi::OsString; @@ -245,6 +246,17 @@ pub fn pass(value: A) -> CheckResult { pub type CheckResult = anyhow::Result, A>>; +pub fn sequence_check_results(first: CheckResult<()>, second: CheckResult) -> CheckResult { + match (first?, second?) { + (Either::Right(_), Either::Right(right_value)) => pass(right_value), + (Either::Left(errors), Either::Right(_)) => Ok(Either::Left(errors)), + (Either::Right(_), Either::Left(errors)) => Ok(Either::Left(errors)), + (Either::Left(errors_l), Either::Left(errors_r)) => { + Ok(Either::Left(concat([errors_l, errors_r]))) + } + } +} + pub fn flatten_check_results( check_results: impl IntoIterator>, value_transform: impl Fn(Vec) -> O, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 2f225ecd2ec3..d1655dc2e3fe 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,4 +1,6 @@ -use crate::check_result::{flatten_check_results, pass, write_check_result, CheckError}; +use crate::check_result::{ + flatten_check_results, pass, sequence_check_results, write_check_result, CheckError, +}; use crate::utils; use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; @@ -67,7 +69,7 @@ impl Nixpkgs { // we can't check for any other errors if it's a file, since there's no subdirectories to check } else { let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); - let shard_name_valid_check_result = if !shard_name_valid { + let check_result = if !shard_name_valid { CheckError::InvalidShardName { relative_shard_path: relative_shard_path.clone(), shard_name: shard_name.clone(), @@ -77,8 +79,6 @@ impl Nixpkgs { pass(()) }; - write_check_result(error_writer, shard_name_valid_check_result)?; - let entries = utils::read_dir_sorted(&shard_path)?; let duplicate_check_results = entries @@ -96,10 +96,10 @@ impl Nixpkgs { .into_result::<()>() }); - let duplicate_check_result = - flatten_check_results(duplicate_check_results, |_| ()); - - write_check_result(error_writer, duplicate_check_result)?; + let check_result = sequence_check_results( + check_result, + flatten_check_results(duplicate_check_results, |_| ()), + ); let check_results = entries.into_iter().map(|package_entry| { let package_path = package_entry.path(); @@ -170,7 +170,10 @@ impl Nixpkgs { } }); - flatten_check_results(check_results, |x| x) + sequence_check_results( + check_result, + flatten_check_results(check_results, |x| x), + ) } }); From 0475238ec08c5b903eac37a13efd1b6f4b9617a3 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 01:58:23 +0200 Subject: [PATCH 23/52] tests.nixpkgs-check-by-name: Make structural check a global function --- pkgs/test/nixpkgs-check-by-name/src/main.rs | 6 +- .../nixpkgs-check-by-name/src/structure.rs | 246 ++++++++---------- 2 files changed, 116 insertions(+), 136 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 7de6eadf6564..6138284d2900 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -4,6 +4,7 @@ mod references; mod structure; mod utils; +use crate::structure::check_structure; use anyhow::Context; use check_result::{flatten_check_results, write_check_result}; use clap::{Parser, ValueEnum}; @@ -11,7 +12,6 @@ use colored::Colorize; use std::io; use std::path::{Path, PathBuf}; use std::process::ExitCode; -use structure::Nixpkgs; use utils::ErrorWriter; /// Program to check the validity of pkgs/by-name @@ -88,9 +88,9 @@ pub fn check_nixpkgs( utils::BASE_SUBPATH ); } else { - let nixpkgs = Nixpkgs::new(&nixpkgs_path, &mut error_writer)?; + let check_result = check_structure(&nixpkgs_path); - if error_writer.empty { + if let Some(nixpkgs) = write_check_result(&mut error_writer, check_result)? { // Only if we could successfully parse the structure, we do the semantic checks let check_result = flatten_check_results( [ diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index d1655dc2e3fe..24586c6b533c 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,11 +1,10 @@ use crate::check_result::{ - flatten_check_results, pass, sequence_check_results, write_check_result, CheckError, + flatten_check_results, pass, sequence_check_results, CheckError, CheckResult, }; use crate::utils; -use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME}; +use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; use regex::Regex; -use std::io; use std::path::{Path, PathBuf}; lazy_static! { @@ -42,155 +41,136 @@ impl Nixpkgs { } } -impl Nixpkgs { - /// Read the structure of a Nixpkgs directory, displaying errors on the writer. - /// May return early with I/O errors. - pub fn new( - path: &Path, - error_writer: &mut ErrorWriter, - ) -> anyhow::Result { - let base_dir = path.join(BASE_SUBPATH); +/// Read the structure of a Nixpkgs directory, displaying errors on the writer. +/// May return early with I/O errors. +pub fn check_structure(path: &Path) -> CheckResult { + let base_dir = path.join(BASE_SUBPATH); - let check_results = utils::read_dir_sorted(&base_dir)? - .into_iter() - .map(|shard_entry| { - let shard_path = shard_entry.path(); - let shard_name = shard_entry.file_name().to_string_lossy().into_owned(); - let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name); + let check_results = utils::read_dir_sorted(&base_dir)? + .into_iter() + .map(|shard_entry| { + let shard_path = shard_entry.path(); + let shard_name = shard_entry.file_name().to_string_lossy().into_owned(); + let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name); - if shard_name == "README.md" { - // README.md is allowed to be a file and not checked - pass(vec![]) - } else if !shard_path.is_dir() { - CheckError::ShardNonDir { + if shard_name == "README.md" { + // README.md is allowed to be a file and not checked + pass(vec![]) + } else if !shard_path.is_dir() { + CheckError::ShardNonDir { + relative_shard_path: relative_shard_path.clone(), + } + .into_result() + // we can't check for any other errors if it's a file, since there's no subdirectories to check + } else { + let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); + let check_result = if !shard_name_valid { + CheckError::InvalidShardName { relative_shard_path: relative_shard_path.clone(), + shard_name: shard_name.clone(), } .into_result() - // we can't check for any other errors if it's a file, since there's no subdirectories to check } else { - let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); - let check_result = if !shard_name_valid { - CheckError::InvalidShardName { + pass(()) + }; + + let entries = utils::read_dir_sorted(&shard_path)?; + + let duplicate_check_results = entries + .iter() + .zip(entries.iter().skip(1)) + .filter(|(l, r)| { + l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() + }) + .map(|(l, r)| { + CheckError::CaseSensitiveDuplicate { relative_shard_path: relative_shard_path.clone(), - shard_name: shard_name.clone(), + first: l.file_name(), + second: r.file_name(), + } + .into_result::<()>() + }); + + let check_result = sequence_check_results( + check_result, + flatten_check_results(duplicate_check_results, |_| ()), + ); + + let check_results = entries.into_iter().map(|package_entry| { + 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}")); + + if !package_path.is_dir() { + CheckError::PackageNonDir { + relative_package_dir: relative_package_dir.clone(), } .into_result() } else { - pass(()) - }; - - let entries = utils::read_dir_sorted(&shard_path)?; - - let duplicate_check_results = entries - .iter() - .zip(entries.iter().skip(1)) - .filter(|(l, r)| { - l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() - }) - .map(|(l, r)| { - CheckError::CaseSensitiveDuplicate { - relative_shard_path: relative_shard_path.clone(), - first: l.file_name(), - second: r.file_name(), + let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); + let name_check_result = if !package_name_valid { + CheckError::InvalidPackageName { + relative_package_dir: relative_package_dir.clone(), + package_name: package_name.clone(), } - .into_result::<()>() - }); + .into_result() + } else { + pass(()) + }; - let check_result = sequence_check_results( - check_result, - flatten_check_results(duplicate_check_results, |_| ()), - ); + let correct_relative_package_dir = + Nixpkgs::relative_dir_for_package(&package_name); + let shard_check_result = + if relative_package_dir != correct_relative_package_dir { + // Only show this error if we have a valid shard and package name + // Because if one of those is wrong, you should fix that first + if shard_name_valid && package_name_valid { + CheckError::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: correct_relative_package_dir + .clone(), + } + .into_result() + } else { + pass(()) + } + } else { + pass(()) + }; - let check_results = entries.into_iter().map(|package_entry| { - 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}")); - - if !package_path.is_dir() { - CheckError::PackageNonDir { + let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); + let package_nix_check_result = if !package_nix_path.exists() { + CheckError::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else if package_nix_path.is_dir() { + CheckError::PackageNixDir { relative_package_dir: relative_package_dir.clone(), } .into_result() } else { - let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - let name_check_result = if !package_name_valid { - CheckError::InvalidPackageName { - relative_package_dir: relative_package_dir.clone(), - package_name: package_name.clone(), - } - .into_result() - } else { - pass(()) - }; + pass(()) + }; - let correct_relative_package_dir = - Nixpkgs::relative_dir_for_package(&package_name); - let shard_check_result = - if relative_package_dir != correct_relative_package_dir { - // Only show this error if we have a valid shard and package name - // Because if one of those is wrong, you should fix that first - if shard_name_valid && package_name_valid { - CheckError::IncorrectShard { - relative_package_dir: relative_package_dir.clone(), - correct_relative_package_dir: - correct_relative_package_dir.clone(), - } - .into_result() - } else { - pass(()) - } - } else { - pass(()) - }; + flatten_check_results( + [ + name_check_result, + shard_check_result, + package_nix_check_result, + ], + |_| package_name.clone(), + ) + } + }); - let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let package_nix_check_result = if !package_nix_path.exists() { - CheckError::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - CheckError::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - pass(()) - }; - - flatten_check_results( - [ - name_check_result, - shard_check_result, - package_nix_check_result, - ], - |_| package_name.clone(), - ) - } - }); - - sequence_check_results( - check_result, - flatten_check_results(check_results, |x| x), - ) - } - }); - - let check_result = flatten_check_results(check_results, |x| { - x.into_iter().flatten().collect::>() + sequence_check_results(check_result, flatten_check_results(check_results, |x| x)) + } }); - if let Some(package_names) = write_check_result(error_writer, check_result)? { - Ok(Nixpkgs { - path: path.to_owned(), - package_names, - }) - } else { - Ok(Nixpkgs { - path: path.to_owned(), - package_names: vec![], - }) - } - } + flatten_check_results(check_results, |x| Nixpkgs { + path: path.to_owned(), + package_names: x.into_iter().flatten().collect::>(), + }) } From d65f3ddb890570dea21df41e386d0f6401c9ec3a Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 02:18:21 +0200 Subject: [PATCH 24/52] tests.nixpkgs-check-by-name: Make reference check part of structural check --- pkgs/test/nixpkgs-check-by-name/src/main.rs | 12 ++----- .../nixpkgs-check-by-name/src/references.rs | 31 +++++++++---------- .../nixpkgs-check-by-name/src/structure.rs | 7 +++++ 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 6138284d2900..ff223068697a 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -6,7 +6,7 @@ mod utils; use crate::structure::check_structure; use anyhow::Context; -use check_result::{flatten_check_results, write_check_result}; +use check_result::write_check_result; use clap::{Parser, ValueEnum}; use colored::Colorize; use std::io; @@ -91,14 +91,8 @@ pub fn check_nixpkgs( let check_result = check_structure(&nixpkgs_path); if let Some(nixpkgs) = write_check_result(&mut error_writer, check_result)? { - // Only if we could successfully parse the structure, we do the semantic checks - let check_result = flatten_check_results( - [ - eval::check_values(version, &nixpkgs, eval_accessible_paths), - references::check_references(&nixpkgs), - ], - |_| (), - ); + // Only if we could successfully parse the structure, we do the evaluation checks + let check_result = eval::check_values(version, &nixpkgs, eval_accessible_paths); write_check_result(&mut error_writer, check_result)?; } } diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index b932b556f54f..91f898fe3bfd 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,5 +1,4 @@ use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult}; -use crate::structure::Nixpkgs; use crate::utils; use crate::utils::LineIndex; @@ -19,23 +18,21 @@ struct PackageContext<'a> { /// Check that every package directory in pkgs/by-name doesn't link to outside that directory. /// Both symlinks and Nix path expressions are checked. -pub fn check_references(nixpkgs: &Nixpkgs) -> CheckResult<()> { - // Check the directories for each package separately - let check_results = nixpkgs.package_names.iter().map(|package_name| { - let relative_package_dir = Nixpkgs::relative_dir_for_package(package_name); - let context = PackageContext { - relative_package_dir: &relative_package_dir, - absolute_package_dir: &nixpkgs.path.join(&relative_package_dir), - }; +pub fn check_references( + relative_package_dir: &Path, + absolute_package_dir: &Path, +) -> CheckResult<()> { + let context = PackageContext { + relative_package_dir: &relative_package_dir.to_path_buf(), + absolute_package_dir: &absolute_package_dir.to_path_buf(), + }; - // The empty argument here is the subpath under the package directory to check - // An empty one means the package directory itself - check_path(&context, Path::new("")).context(format!( - "While checking the references in package directory {}", - relative_package_dir.display() - )) - }); - flatten_check_results(check_results, |_| ()) + // The empty argument here is the subpath under the package directory to check + // An empty one means the package directory itself + check_path(&context, Path::new("")).context(format!( + "While checking the references in package directory {}", + relative_package_dir.display() + )) } /// Checks for a specific path to not have references outside diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 24586c6b533c..09ec798e440e 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,6 +1,7 @@ use crate::check_result::{ flatten_check_results, pass, sequence_check_results, CheckError, CheckResult, }; +use crate::references; use crate::utils; use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME}; use lazy_static::lazy_static; @@ -154,11 +155,17 @@ pub fn check_structure(path: &Path) -> CheckResult { pass(()) }; + let reference_check_result = references::check_references( + &relative_package_dir, + &path.join(&relative_package_dir), + ); + flatten_check_results( [ name_check_result, shard_check_result, package_nix_check_result, + reference_check_result, ], |_| package_name.clone(), ) From e58bc75444d5d722a995cf350b0d6c298aeb3808 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 02:22:42 +0200 Subject: [PATCH 25/52] tests.nixpkgs-check-by-name: Remove Nixpkgs struct Isn't necessary anymore with the refactoring --- pkgs/test/nixpkgs-check-by-name/src/eval.rs | 15 +++--- pkgs/test/nixpkgs-check-by-name/src/main.rs | 5 +- .../nixpkgs-check-by-name/src/structure.rs | 47 +++++++------------ 3 files changed, 28 insertions(+), 39 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index eb90a295a577..487ec7cab730 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -41,7 +41,8 @@ const EXPR: &str = include_str!("eval.nix"); /// See the `eval.nix` file for how this is achieved on the Nix side pub fn check_values( version: Version, - nixpkgs: &structure::Nixpkgs, + nixpkgs_path: &Path, + package_names: Vec, eval_accessible_paths: Vec<&Path>, ) -> CheckResult<()> { // Write the list of packages we need to check into a temporary JSON file. @@ -53,7 +54,7 @@ pub fn check_values( // entry is needed. let attrs_file_path = attrs_file.path().canonicalize()?; - serde_json::to_writer(&attrs_file, &nixpkgs.package_names).context(format!( + serde_json::to_writer(&attrs_file, &package_names).context(format!( "Failed to serialise the package names to the temporary path {}", attrs_file_path.display() ))?; @@ -85,9 +86,9 @@ pub fn check_values( .arg(&attrs_file_path) // Same for the nixpkgs to test .args(["--arg", "nixpkgsPath"]) - .arg(&nixpkgs.path) + .arg(nixpkgs_path) .arg("-I") - .arg(&nixpkgs.path); + .arg(nixpkgs_path); // Also add extra paths that need to be accessible for path in eval_accessible_paths { @@ -109,9 +110,9 @@ pub fn check_values( String::from_utf8_lossy(&result.stdout) ))?; - let check_results = nixpkgs.package_names.iter().map(|package_name| { - let relative_package_file = structure::Nixpkgs::relative_file_for_package(package_name); - let absolute_package_file = nixpkgs.path.join(&relative_package_file); + let check_results = package_names.iter().map(|package_name| { + let relative_package_file = structure::relative_file_for_package(package_name); + let absolute_package_file = nixpkgs_path.join(&relative_package_file); if let Some(attribute_info) = actual_files.get(package_name) { let valid = match &attribute_info.variant { diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index ff223068697a..67f6055e7338 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -90,9 +90,10 @@ pub fn check_nixpkgs( } else { let check_result = check_structure(&nixpkgs_path); - if let Some(nixpkgs) = write_check_result(&mut error_writer, check_result)? { + if let Some(package_names) = write_check_result(&mut error_writer, check_result)? { // Only if we could successfully parse the structure, we do the evaluation checks - let check_result = eval::check_values(version, &nixpkgs, eval_accessible_paths); + let check_result = + eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths); write_check_result(&mut error_writer, check_result)?; } } diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 09ec798e440e..c8fbc17fcb39 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -13,38 +13,27 @@ lazy_static! { static ref PACKAGE_NAME_REGEX: Regex = Regex::new(r"^[a-zA-Z0-9_-]+$").unwrap(); } -/// Contains information about the structure of the pkgs/by-name directory of a Nixpkgs -pub struct Nixpkgs { - /// The path to nixpkgs - pub path: PathBuf, - /// The names of all packages declared in pkgs/by-name - pub package_names: Vec, +// Some utility functions for the basic structure + +pub fn shard_for_package(package_name: &str) -> String { + package_name.to_lowercase().chars().take(2).collect() } -impl Nixpkgs { - // Some utility functions for the basic structure +pub fn relative_dir_for_shard(shard_name: &str) -> PathBuf { + PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}")) +} - pub fn shard_for_package(package_name: &str) -> String { - package_name.to_lowercase().chars().take(2).collect() - } +pub fn relative_dir_for_package(package_name: &str) -> PathBuf { + relative_dir_for_shard(&shard_for_package(package_name)).join(package_name) +} - pub fn relative_dir_for_shard(shard_name: &str) -> PathBuf { - PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}")) - } - - pub fn relative_dir_for_package(package_name: &str) -> PathBuf { - Nixpkgs::relative_dir_for_shard(&Nixpkgs::shard_for_package(package_name)) - .join(package_name) - } - - pub fn relative_file_for_package(package_name: &str) -> PathBuf { - Nixpkgs::relative_dir_for_package(package_name).join(PACKAGE_NIX_FILENAME) - } +pub fn relative_file_for_package(package_name: &str) -> PathBuf { + relative_dir_for_package(package_name).join(PACKAGE_NIX_FILENAME) } /// Read the structure of a Nixpkgs directory, displaying errors on the writer. /// May return early with I/O errors. -pub fn check_structure(path: &Path) -> CheckResult { +pub fn check_structure(path: &Path) -> CheckResult> { let base_dir = path.join(BASE_SUBPATH); let check_results = utils::read_dir_sorted(&base_dir)? @@ -52,7 +41,7 @@ pub fn check_structure(path: &Path) -> CheckResult { .map(|shard_entry| { let shard_path = shard_entry.path(); let shard_name = shard_entry.file_name().to_string_lossy().into_owned(); - let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name); + let relative_shard_path = relative_dir_for_shard(&shard_name); if shard_name == "README.md" { // README.md is allowed to be a file and not checked @@ -120,8 +109,7 @@ pub fn check_structure(path: &Path) -> CheckResult { pass(()) }; - let correct_relative_package_dir = - Nixpkgs::relative_dir_for_package(&package_name); + let correct_relative_package_dir = relative_dir_for_package(&package_name); let shard_check_result = if relative_package_dir != correct_relative_package_dir { // Only show this error if we have a valid shard and package name @@ -176,8 +164,7 @@ pub fn check_structure(path: &Path) -> CheckResult { } }); - flatten_check_results(check_results, |x| Nixpkgs { - path: path.to_owned(), - package_names: x.into_iter().flatten().collect::>(), + flatten_check_results(check_results, |x| { + x.into_iter().flatten().collect::>() }) } From 3d60440799721601b04ff39e83374aa684a5300b Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 20 Oct 2023 02:45:54 +0200 Subject: [PATCH 26/52] tests.nixpkgs-check-by-name: Remove error writer --- .../nixpkgs-check-by-name/src/check_result.rs | 16 --------- pkgs/test/nixpkgs-check-by-name/src/main.rs | 35 +++++++++++-------- pkgs/test/nixpkgs-check-by-name/src/utils.rs | 24 ------------- 3 files changed, 21 insertions(+), 54 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index 818ba1a9afd7..e5f14152d5ac 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -1,5 +1,4 @@ use crate::utils::PACKAGE_NIX_FILENAME; -use crate::ErrorWriter; use itertools::concat; use itertools::{Either, Itertools}; use rnix::parser::ParseError; @@ -225,21 +224,6 @@ impl fmt::Display for CheckError { } } -pub fn write_check_result( - error_writer: &mut ErrorWriter, - check_result: CheckResult, -) -> anyhow::Result> { - match check_result? { - Either::Left(errors) => { - for error in errors { - error_writer.write(&error.to_string())? - } - Ok(None) - } - Either::Right(value) => Ok(Some(value)), - } -} - pub fn pass(value: A) -> CheckResult { Ok(Either::Right(value)) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 67f6055e7338..537276a177d5 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -6,13 +6,14 @@ mod utils; use crate::structure::check_structure; use anyhow::Context; -use check_result::write_check_result; +use check_result::pass; use clap::{Parser, ValueEnum}; use colored::Colorize; +use itertools::Either; +use itertools::Either::{Left, Right}; use std::io; use std::path::{Path, PathBuf}; use std::process::ExitCode; -use utils::ErrorWriter; /// Program to check the validity of pkgs/by-name #[derive(Parser, Debug)] @@ -78,26 +79,32 @@ pub fn check_nixpkgs( nixpkgs_path.display() ))?; - // Wraps the error_writer to print everything in red, and tracks whether anything was printed - // at all. Later used to figure out if the structure was valid or not. - let mut error_writer = ErrorWriter::new(error_writer); - - if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() { + let check_result = if !nixpkgs_path.join(utils::BASE_SUBPATH).exists() { eprintln!( "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.", utils::BASE_SUBPATH ); + pass(()) } else { - let check_result = check_structure(&nixpkgs_path); - - if let Some(package_names) = write_check_result(&mut error_writer, check_result)? { + match check_structure(&nixpkgs_path)? { + Left(errors) => Ok(Left(errors)), + Right(package_names) => // Only if we could successfully parse the structure, we do the evaluation checks - let check_result = - eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths); - write_check_result(&mut error_writer, check_result)?; + { + eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths) + } } + }; + + match check_result? { + Either::Left(errors) => { + for error in errors { + writeln!(error_writer, "{}", error.to_string().red())? + } + Ok(false) + } + Either::Right(_) => Ok(true), } - Ok(error_writer.empty) } #[cfg(test)] diff --git a/pkgs/test/nixpkgs-check-by-name/src/utils.rs b/pkgs/test/nixpkgs-check-by-name/src/utils.rs index 728e50e72336..5cc4a0863ba8 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/utils.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/utils.rs @@ -1,5 +1,4 @@ use anyhow::Context; -use colored::Colorize; use std::fs; use std::io; use std::path::Path; @@ -50,26 +49,3 @@ impl LineIndex { } } } - -/// A small wrapper around a generic io::Write specifically for errors: -/// - Print everything in red to signal it's an error -/// - Keep track of whether anything was printed at all, so that -/// it can be queried whether any errors were encountered at all -pub struct ErrorWriter { - pub writer: W, - pub empty: bool, -} - -impl ErrorWriter { - pub fn new(writer: W) -> ErrorWriter { - ErrorWriter { - writer, - empty: true, - } - } - - pub fn write(&mut self, string: &str) -> io::Result<()> { - self.empty = false; - writeln!(self.writer, "{}", string.red()) - } -} From eac0b69063c0706d3a0bc956a1efc04745c14594 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 23 Oct 2023 23:54:02 +0200 Subject: [PATCH 27/52] tests.nixpkgs-check-by-name: Redesign and document check_result functions --- .../nixpkgs-check-by-name/src/check_result.rs | 104 ++++++++++------- pkgs/test/nixpkgs-check-by-name/src/eval.rs | 16 +-- pkgs/test/nixpkgs-check-by-name/src/main.rs | 9 +- .../nixpkgs-check-by-name/src/references.rs | 37 +++--- .../nixpkgs-check-by-name/src/structure.rs | 106 +++++++++--------- 5 files changed, 147 insertions(+), 125 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index e5f14152d5ac..db2377c7204b 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -1,13 +1,17 @@ use crate::utils::PACKAGE_NIX_FILENAME; use itertools::concat; -use itertools::{Either, Itertools}; +use itertools::{ + Either, + Either::{Left, Right}, + Itertools, +}; use rnix::parser::ParseError; use std::ffi::OsString; use std::fmt; use std::io; use std::path::PathBuf; -pub enum CheckError { +pub enum CheckProblem { ShardNonDir { relative_shard_path: PathBuf, }, @@ -90,97 +94,97 @@ pub enum CheckError { }, } -impl CheckError { +impl CheckProblem { pub fn into_result(self) -> CheckResult { - Ok(Either::Left(vec![self])) + Ok(Left(vec![self])) } } -impl fmt::Display for CheckError { +impl fmt::Display for CheckProblem { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - CheckError::ShardNonDir { relative_shard_path } => + CheckProblem::ShardNonDir { relative_shard_path } => write!( f, "{}: This is a file, but it should be a directory.", relative_shard_path.display(), ), - CheckError::InvalidShardName { relative_shard_path, shard_name } => + CheckProblem::InvalidShardName { relative_shard_path, shard_name } => write!( f, "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", relative_shard_path.display() ), - CheckError::PackageNonDir { relative_package_dir } => + CheckProblem::PackageNonDir { relative_package_dir } => write!( f, "{}: This path is a file, but it should be a directory.", relative_package_dir.display(), ), - CheckError::CaseSensitiveDuplicate { relative_shard_path, first, second } => + CheckProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } => write!( f, "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.", relative_shard_path.display(), ), - CheckError::InvalidPackageName { relative_package_dir, package_name } => + CheckProblem::InvalidPackageName { relative_package_dir, package_name } => write!( f, "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", relative_package_dir.display(), ), - CheckError::IncorrectShard { relative_package_dir, correct_relative_package_dir } => + CheckProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } => write!( f, "{}: Incorrect directory location, should be {} instead.", relative_package_dir.display(), correct_relative_package_dir.display(), ), - CheckError::PackageNixNonExistent { relative_package_dir } => + CheckProblem::PackageNixNonExistent { relative_package_dir } => write!( f, "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", relative_package_dir.display(), ), - CheckError::PackageNixDir { relative_package_dir } => + CheckProblem::PackageNixDir { relative_package_dir } => write!( f, "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", relative_package_dir.display(), ), - CheckError::UndefinedAttr { relative_package_file, package_name } => + CheckProblem::UndefinedAttr { relative_package_file, package_name } => write!( f, "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", relative_package_file.display() ), - CheckError::WrongCallPackage { relative_package_file, package_name } => + CheckProblem::WrongCallPackage { relative_package_file, package_name } => write!( f, "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.", relative_package_file.display() ), - CheckError::NonDerivation { relative_package_file, package_name } => + CheckProblem::NonDerivation { relative_package_file, package_name } => write!( f, "pkgs.{package_name}: This attribute defined by {} is not a derivation", relative_package_file.display() ), - CheckError::OutsideSymlink { relative_package_dir, subpath } => + CheckProblem::OutsideSymlink { relative_package_dir, subpath } => write!( f, "{}: Path {} is a symlink pointing to a path outside the directory of that package.", relative_package_dir.display(), subpath.display(), ), - CheckError::UnresolvableSymlink { relative_package_dir, subpath, io_error } => + CheckProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } => write!( f, "{}: Path {} is a symlink which cannot be resolved: {io_error}.", relative_package_dir.display(), subpath.display(), ), - CheckError::CouldNotParseNix { relative_package_dir, subpath, error } => + CheckProblem::CouldNotParseNix { relative_package_dir, subpath, error } => write!( f, "{}: File {} could not be parsed by rnix: {}", @@ -188,7 +192,7 @@ impl fmt::Display for CheckError { subpath.display(), error, ), - CheckError::PathInterpolation { relative_package_dir, subpath, line, text } => + CheckProblem::PathInterpolation { relative_package_dir, subpath, line, text } => write!( f, "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.", @@ -196,7 +200,7 @@ impl fmt::Display for CheckError { subpath.display(), text ), - CheckError::SearchPath { relative_package_dir, subpath, line, text } => + CheckProblem::SearchPath { relative_package_dir, subpath, line, text } => write!( f, "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.", @@ -204,7 +208,7 @@ impl fmt::Display for CheckError { subpath.display(), text ), - CheckError::OutsidePathReference { relative_package_dir, subpath, line, text } => + CheckProblem::OutsidePathReference { relative_package_dir, subpath, line, text } => write!( f, "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.", @@ -212,7 +216,7 @@ impl fmt::Display for CheckError { subpath.display(), text, ), - CheckError::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } => + CheckProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } => write!( f, "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.", @@ -224,27 +228,44 @@ impl fmt::Display for CheckError { } } -pub fn pass(value: A) -> CheckResult { - Ok(Either::Right(value)) +/// A type alias representing the result of a check, either: +/// - Err(anyhow::Error): A fatal failure, typically I/O errors. +/// Such failures are not caused by the files in Nixpkgs. +/// This hints at a bug in the code or a problem with the deployment. +/// - Ok(Left(Vec)): A non-fatal problem with the Nixpkgs files. +/// Further checks can be run even with this result type. +/// Such problems can be fixed by changing the Nixpkgs files. +/// - Ok(Right(A)): A successful (potentially intermediate) result with an arbitrary value. +/// No fatal errors have occurred and no problems have been found with Nixpkgs. +pub type CheckResult = anyhow::Result, A>>; + +/// Map a `CheckResult` to a `CheckResult` by applying a function to the +/// potentially contained value in case of success. +pub fn map(check_result: CheckResult, f: impl FnOnce(I) -> O) -> CheckResult { + Ok(check_result?.map_right(f)) } -pub type CheckResult = anyhow::Result, A>>; +/// Create a successfull `CheckResult` from a value `A`. +pub fn ok(value: A) -> CheckResult { + Ok(Right(value)) +} -pub fn sequence_check_results(first: CheckResult<()>, second: CheckResult) -> CheckResult { +/// Combine two check results, both of which need to be successful for the return value to be successful. +/// The `CheckProblem`s of both sides are returned concatenated. +pub fn and(first: CheckResult<()>, second: CheckResult) -> CheckResult { match (first?, second?) { - (Either::Right(_), Either::Right(right_value)) => pass(right_value), - (Either::Left(errors), Either::Right(_)) => Ok(Either::Left(errors)), - (Either::Right(_), Either::Left(errors)) => Ok(Either::Left(errors)), - (Either::Left(errors_l), Either::Left(errors_r)) => { - Ok(Either::Left(concat([errors_l, errors_r]))) - } + (Right(_), Right(right_value)) => Ok(Right(right_value)), + (Left(errors), Right(_)) => Ok(Left(errors)), + (Right(_), Left(errors)) => Ok(Left(errors)), + (Left(errors_l), Left(errors_r)) => Ok(Left(concat([errors_l, errors_r]))), } } -pub fn flatten_check_results( - check_results: impl IntoIterator>, - value_transform: impl Fn(Vec) -> O, -) -> CheckResult { +/// Combine many checks results into a single one. +/// All given check results need to be successful in order for the returned check result to be successful, +/// in which case the returned check result value contains each a `Vec` of each individual result value. +/// The `CheckProblem`s of all results are returned concatenated. +pub fn sequence(check_results: impl IntoIterator>) -> CheckResult> { let (errors, values): (Vec<_>, Vec<_>) = check_results .into_iter() .collect::>>()? @@ -256,8 +277,13 @@ pub fn flatten_check_results( let flattened_errors = errors.into_iter().flatten().collect::>(); if flattened_errors.is_empty() { - Ok(Either::Right(value_transform(values))) + Ok(Right(values)) } else { - Ok(Either::Left(flattened_errors)) + Ok(Left(flattened_errors)) } } + +/// Like `sequence`, but replace the resulting value with () +pub fn sequence_(check_results: impl IntoIterator>) -> CheckResult<()> { + map(sequence(check_results), |_| ()) +} diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 487ec7cab730..a76abba05d96 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,4 +1,5 @@ -use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult}; +use crate::check_result; +use crate::check_result::{CheckProblem, CheckResult}; use crate::structure; use crate::Version; use std::path::Path; @@ -110,7 +111,7 @@ pub fn check_values( String::from_utf8_lossy(&result.stdout) ))?; - let check_results = package_names.iter().map(|package_name| { + check_result::sequence_(package_names.iter().map(|package_name| { let relative_package_file = structure::relative_file_for_package(package_name); let absolute_package_file = nixpkgs_path.join(&relative_package_file); @@ -136,27 +137,26 @@ pub fn check_values( }; if !valid { - CheckError::WrongCallPackage { + CheckProblem::WrongCallPackage { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), } .into_result() } else if !attribute_info.is_derivation { - CheckError::NonDerivation { + CheckProblem::NonDerivation { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), } .into_result() } else { - pass(()) + check_result::ok(()) } } else { - CheckError::UndefinedAttr { + CheckProblem::UndefinedAttr { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), } .into_result() } - }); - flatten_check_results(check_results, |_| ()) + })) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 537276a177d5..11fedde74130 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -6,11 +6,12 @@ mod utils; use crate::structure::check_structure; use anyhow::Context; -use check_result::pass; use clap::{Parser, ValueEnum}; use colored::Colorize; -use itertools::Either; -use itertools::Either::{Left, Right}; +use itertools::{ + Either, + Either::{Left, Right}, +}; use std::io; use std::path::{Path, PathBuf}; use std::process::ExitCode; @@ -84,7 +85,7 @@ pub fn check_nixpkgs( "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.", utils::BASE_SUBPATH ); - pass(()) + check_result::ok(()) } else { match check_structure(&nixpkgs_path)? { Left(errors) => Ok(Left(errors)), diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 91f898fe3bfd..a6bc53d717d7 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,4 +1,5 @@ -use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult}; +use crate::check_result; +use crate::check_result::{CheckProblem, CheckResult}; use crate::utils; use crate::utils::LineIndex; @@ -46,16 +47,16 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - CheckError::OutsideSymlink { + CheckProblem::OutsideSymlink { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), } .into_result() } else { - pass(()) + check_result::ok(()) } } - Err(io_error) => CheckError::UnresolvableSymlink { + Err(io_error) => CheckProblem::UnresolvableSymlink { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), io_error, @@ -64,12 +65,11 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { } } else if path.is_dir() { // Recursively check each entry - let check_results = utils::read_dir_sorted(&path)?.into_iter().map(|entry| { + check_result::sequence_(utils::read_dir_sorted(&path)?.into_iter().map(|entry| { let entry_subpath = subpath.join(entry.file_name()); check_path(context, &entry_subpath) .context(format!("Error while recursing into {}", subpath.display())) - }); - flatten_check_results(check_results, |_| ()) + })) } else if path.is_file() { // Only check Nix files if let Some(ext) = path.extension() { @@ -79,10 +79,10 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { subpath.display() )) } else { - pass(()) + check_result::ok(()) } } else { - pass(()) + check_result::ok(()) } } else { // This should never happen, git doesn't support other file types @@ -104,7 +104,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let root = Root::parse(&contents); if let Some(error) = root.errors().first() { - return CheckError::CouldNotParseNix { + return CheckProblem::CouldNotParseNix { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), error: error.clone(), @@ -114,17 +114,17 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let line_index = LineIndex::new(&contents); - let check_results = root.syntax().descendants().map(|node| { + check_result::sequence_(root.syntax().descendants().map(|node| { let text = node.text().to_string(); let line = line_index.line(node.text_range().start().into()); if node.kind() != NODE_PATH { // We're only interested in Path expressions - pass(()) + check_result::ok(()) } else if node.children().count() != 0 { // Filters out ./foo/${bar}/baz // TODO: We can just check ./foo - CheckError::PathInterpolation { + CheckProblem::PathInterpolation { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -133,7 +133,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { .into_result() } else if text.starts_with('<') { // Filters out search paths like - CheckError::SearchPath { + CheckProblem::SearchPath { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -149,7 +149,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - CheckError::OutsidePathReference { + CheckProblem::OutsidePathReference { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -157,10 +157,10 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { } .into_result() } else { - pass(()) + check_result::ok(()) } } - Err(e) => CheckError::UnresolvablePathReference { + Err(e) => CheckProblem::UnresolvablePathReference { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -170,6 +170,5 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { .into_result(), } } - }); - flatten_check_results(check_results, |_| ()) + })) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index c8fbc17fcb39..90a552a38863 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,6 +1,5 @@ -use crate::check_result::{ - flatten_check_results, pass, sequence_check_results, CheckError, CheckResult, -}; +use crate::check_result; +use crate::check_result::{CheckProblem, CheckResult}; use crate::references; use crate::utils; use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME}; @@ -36,7 +35,7 @@ pub fn relative_file_for_package(package_name: &str) -> PathBuf { pub fn check_structure(path: &Path) -> CheckResult> { let base_dir = path.join(BASE_SUBPATH); - let check_results = utils::read_dir_sorted(&base_dir)? + let shard_results = utils::read_dir_sorted(&base_dir)? .into_iter() .map(|shard_entry| { let shard_path = shard_entry.path(); @@ -45,35 +44,35 @@ pub fn check_structure(path: &Path) -> CheckResult> { if shard_name == "README.md" { // README.md is allowed to be a file and not checked - pass(vec![]) + check_result::ok(vec![]) } else if !shard_path.is_dir() { - CheckError::ShardNonDir { + CheckProblem::ShardNonDir { relative_shard_path: relative_shard_path.clone(), } .into_result() // we can't check for any other errors if it's a file, since there's no subdirectories to check } else { let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); - let check_result = if !shard_name_valid { - CheckError::InvalidShardName { + let result = if !shard_name_valid { + CheckProblem::InvalidShardName { relative_shard_path: relative_shard_path.clone(), shard_name: shard_name.clone(), } .into_result() } else { - pass(()) + check_result::ok(()) }; let entries = utils::read_dir_sorted(&shard_path)?; - let duplicate_check_results = entries + let duplicate_results = entries .iter() .zip(entries.iter().skip(1)) .filter(|(l, r)| { l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() }) .map(|(l, r)| { - CheckError::CaseSensitiveDuplicate { + CheckProblem::CaseSensitiveDuplicate { relative_shard_path: relative_shard_path.clone(), first: l.file_name(), second: r.file_name(), @@ -81,90 +80,87 @@ pub fn check_structure(path: &Path) -> CheckResult> { .into_result::<()>() }); - let check_result = sequence_check_results( - check_result, - flatten_check_results(duplicate_check_results, |_| ()), - ); + let result = check_result::and(result, check_result::sequence_(duplicate_results)); - let check_results = entries.into_iter().map(|package_entry| { + let package_results = entries.into_iter().map(|package_entry| { 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}")); if !package_path.is_dir() { - CheckError::PackageNonDir { + CheckProblem::PackageNonDir { relative_package_dir: relative_package_dir.clone(), } .into_result() } else { let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - let name_check_result = if !package_name_valid { - CheckError::InvalidPackageName { + let result = if !package_name_valid { + CheckProblem::InvalidPackageName { relative_package_dir: relative_package_dir.clone(), package_name: package_name.clone(), } .into_result() } else { - pass(()) + check_result::ok(()) }; let correct_relative_package_dir = relative_dir_for_package(&package_name); - let shard_check_result = + let result = check_result::and( + result, if relative_package_dir != correct_relative_package_dir { // Only show this error if we have a valid shard and package name // Because if one of those is wrong, you should fix that first if shard_name_valid && package_name_valid { - CheckError::IncorrectShard { + CheckProblem::IncorrectShard { relative_package_dir: relative_package_dir.clone(), correct_relative_package_dir: correct_relative_package_dir .clone(), } .into_result() } else { - pass(()) + check_result::ok(()) } } else { - pass(()) - }; - - let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let package_nix_check_result = if !package_nix_path.exists() { - CheckError::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - CheckError::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - pass(()) - }; - - let reference_check_result = references::check_references( - &relative_package_dir, - &path.join(&relative_package_dir), + check_result::ok(()) + }, ); - flatten_check_results( - [ - name_check_result, - shard_check_result, - package_nix_check_result, - reference_check_result, - ], - |_| package_name.clone(), - ) + let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); + let result = check_result::and( + result, + if !package_nix_path.exists() { + CheckProblem::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else if package_nix_path.is_dir() { + CheckProblem::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + check_result::ok(()) + }, + ); + + let result = check_result::and( + result, + references::check_references( + &relative_package_dir, + &path.join(&relative_package_dir), + ), + ); + + check_result::map(result, |_| package_name.clone()) } }); - sequence_check_results(check_result, flatten_check_results(check_results, |x| x)) + check_result::and(result, check_result::sequence(package_results)) } }); - flatten_check_results(check_results, |x| { + check_result::map(check_result::sequence(shard_results), |x| { x.into_iter().flatten().collect::>() }) } From 8be41ace99f26479ba1d5a7a0c413e0885113158 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 23 Oct 2023 23:58:51 +0200 Subject: [PATCH 28/52] tests.nixpkgs-check-by-name: Separate file for all problems And introduce a function for some smaller indentation --- .../nixpkgs-check-by-name/src/check_result.rs | 239 +----------------- pkgs/test/nixpkgs-check-by-name/src/eval.rs | 9 +- pkgs/test/nixpkgs-check-by-name/src/main.rs | 1 + .../src/nixpkgs_problem.rs | 218 ++++++++++++++++ .../nixpkgs-check-by-name/src/references.rs | 17 +- .../nixpkgs-check-by-name/src/structure.rs | 166 ++++++------ 6 files changed, 331 insertions(+), 319 deletions(-) create mode 100644 pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs index db2377c7204b..a6538d778b61 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -1,243 +1,21 @@ -use crate::utils::PACKAGE_NIX_FILENAME; +use crate::nixpkgs_problem::NixpkgsProblem; use itertools::concat; use itertools::{ Either, Either::{Left, Right}, Itertools, }; -use rnix::parser::ParseError; -use std::ffi::OsString; -use std::fmt; -use std::io; -use std::path::PathBuf; - -pub enum CheckProblem { - ShardNonDir { - relative_shard_path: PathBuf, - }, - InvalidShardName { - relative_shard_path: PathBuf, - shard_name: String, - }, - PackageNonDir { - relative_package_dir: PathBuf, - }, - CaseSensitiveDuplicate { - relative_shard_path: PathBuf, - first: OsString, - second: OsString, - }, - InvalidPackageName { - relative_package_dir: PathBuf, - package_name: String, - }, - IncorrectShard { - relative_package_dir: PathBuf, - correct_relative_package_dir: PathBuf, - }, - PackageNixNonExistent { - relative_package_dir: PathBuf, - }, - PackageNixDir { - relative_package_dir: PathBuf, - }, - UndefinedAttr { - relative_package_file: PathBuf, - package_name: String, - }, - WrongCallPackage { - relative_package_file: PathBuf, - package_name: String, - }, - NonDerivation { - relative_package_file: PathBuf, - package_name: String, - }, - OutsideSymlink { - relative_package_dir: PathBuf, - subpath: PathBuf, - }, - UnresolvableSymlink { - relative_package_dir: PathBuf, - subpath: PathBuf, - io_error: io::Error, - }, - CouldNotParseNix { - relative_package_dir: PathBuf, - subpath: PathBuf, - error: ParseError, - }, - PathInterpolation { - relative_package_dir: PathBuf, - subpath: PathBuf, - line: usize, - text: String, - }, - SearchPath { - relative_package_dir: PathBuf, - subpath: PathBuf, - line: usize, - text: String, - }, - OutsidePathReference { - relative_package_dir: PathBuf, - subpath: PathBuf, - line: usize, - text: String, - }, - UnresolvablePathReference { - relative_package_dir: PathBuf, - subpath: PathBuf, - line: usize, - text: String, - io_error: io::Error, - }, -} - -impl CheckProblem { - pub fn into_result(self) -> CheckResult { - Ok(Left(vec![self])) - } -} - -impl fmt::Display for CheckProblem { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - CheckProblem::ShardNonDir { relative_shard_path } => - write!( - f, - "{}: This is a file, but it should be a directory.", - relative_shard_path.display(), - ), - CheckProblem::InvalidShardName { relative_shard_path, shard_name } => - write!( - f, - "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", - relative_shard_path.display() - ), - CheckProblem::PackageNonDir { relative_package_dir } => - write!( - f, - "{}: This path is a file, but it should be a directory.", - relative_package_dir.display(), - ), - CheckProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } => - write!( - f, - "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.", - relative_shard_path.display(), - ), - CheckProblem::InvalidPackageName { relative_package_dir, package_name } => - write!( - f, - "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", - relative_package_dir.display(), - ), - CheckProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } => - write!( - f, - "{}: Incorrect directory location, should be {} instead.", - relative_package_dir.display(), - correct_relative_package_dir.display(), - ), - CheckProblem::PackageNixNonExistent { relative_package_dir } => - write!( - f, - "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", - relative_package_dir.display(), - ), - CheckProblem::PackageNixDir { relative_package_dir } => - write!( - f, - "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", - relative_package_dir.display(), - ), - CheckProblem::UndefinedAttr { relative_package_file, package_name } => - write!( - f, - "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", - relative_package_file.display() - ), - CheckProblem::WrongCallPackage { relative_package_file, package_name } => - write!( - f, - "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.", - relative_package_file.display() - ), - CheckProblem::NonDerivation { relative_package_file, package_name } => - write!( - f, - "pkgs.{package_name}: This attribute defined by {} is not a derivation", - relative_package_file.display() - ), - CheckProblem::OutsideSymlink { relative_package_dir, subpath } => - write!( - f, - "{}: Path {} is a symlink pointing to a path outside the directory of that package.", - relative_package_dir.display(), - subpath.display(), - ), - CheckProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } => - write!( - f, - "{}: Path {} is a symlink which cannot be resolved: {io_error}.", - relative_package_dir.display(), - subpath.display(), - ), - CheckProblem::CouldNotParseNix { relative_package_dir, subpath, error } => - write!( - f, - "{}: File {} could not be parsed by rnix: {}", - relative_package_dir.display(), - subpath.display(), - error, - ), - CheckProblem::PathInterpolation { relative_package_dir, subpath, line, text } => - write!( - f, - "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.", - relative_package_dir.display(), - subpath.display(), - text - ), - CheckProblem::SearchPath { relative_package_dir, subpath, line, text } => - write!( - f, - "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.", - relative_package_dir.display(), - subpath.display(), - text - ), - CheckProblem::OutsidePathReference { relative_package_dir, subpath, line, text } => - write!( - f, - "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.", - relative_package_dir.display(), - subpath.display(), - text, - ), - CheckProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } => - write!( - f, - "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.", - relative_package_dir.display(), - subpath.display(), - text, - ), - } - } -} /// A type alias representing the result of a check, either: /// - Err(anyhow::Error): A fatal failure, typically I/O errors. /// Such failures are not caused by the files in Nixpkgs. /// This hints at a bug in the code or a problem with the deployment. -/// - Ok(Left(Vec)): A non-fatal problem with the Nixpkgs files. +/// - Ok(Left(Vec)): A non-fatal problem with the Nixpkgs files. /// Further checks can be run even with this result type. /// Such problems can be fixed by changing the Nixpkgs files. /// - Ok(Right(A)): A successful (potentially intermediate) result with an arbitrary value. /// No fatal errors have occurred and no problems have been found with Nixpkgs. -pub type CheckResult = anyhow::Result, A>>; +pub type CheckResult = anyhow::Result, A>>; /// Map a `CheckResult` to a `CheckResult` by applying a function to the /// potentially contained value in case of success. @@ -250,8 +28,15 @@ pub fn ok(value: A) -> CheckResult { Ok(Right(value)) } +impl NixpkgsProblem { + /// Create a `CheckResult` from a single check problem + pub fn into_result(self) -> CheckResult { + Ok(Left(vec![self])) + } +} + /// Combine two check results, both of which need to be successful for the return value to be successful. -/// The `CheckProblem`s of both sides are returned concatenated. +/// The `NixpkgsProblem`s of both sides are returned concatenated. pub fn and(first: CheckResult<()>, second: CheckResult) -> CheckResult { match (first?, second?) { (Right(_), Right(right_value)) => Ok(Right(right_value)), @@ -264,7 +49,7 @@ pub fn and(first: CheckResult<()>, second: CheckResult) -> CheckResult /// Combine many checks results into a single one. /// All given check results need to be successful in order for the returned check result to be successful, /// in which case the returned check result value contains each a `Vec` of each individual result value. -/// The `CheckProblem`s of all results are returned concatenated. +/// The `NixpkgsProblem`s of all results are returned concatenated. pub fn sequence(check_results: impl IntoIterator>) -> CheckResult> { let (errors, values): (Vec<_>, Vec<_>) = check_results .into_iter() diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index a76abba05d96..37fc783f3d34 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,5 +1,6 @@ use crate::check_result; -use crate::check_result::{CheckProblem, CheckResult}; +use crate::check_result::CheckResult; +use crate::nixpkgs_problem::NixpkgsProblem; use crate::structure; use crate::Version; use std::path::Path; @@ -137,13 +138,13 @@ pub fn check_values( }; if !valid { - CheckProblem::WrongCallPackage { + NixpkgsProblem::WrongCallPackage { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), } .into_result() } else if !attribute_info.is_derivation { - CheckProblem::NonDerivation { + NixpkgsProblem::NonDerivation { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), } @@ -152,7 +153,7 @@ pub fn check_values( check_result::ok(()) } } else { - CheckProblem::UndefinedAttr { + NixpkgsProblem::UndefinedAttr { relative_package_file: relative_package_file.clone(), package_name: package_name.clone(), } diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 11fedde74130..1651624f3f40 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -1,5 +1,6 @@ mod check_result; mod eval; +mod nixpkgs_problem; mod references; mod structure; mod utils; diff --git a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs new file mode 100644 index 000000000000..2344a8cc1325 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs @@ -0,0 +1,218 @@ +use crate::utils::PACKAGE_NIX_FILENAME; +use rnix::parser::ParseError; +use std::ffi::OsString; +use std::fmt; +use std::io; +use std::path::PathBuf; + +/// Any problem that can occur when checking Nixpkgs +pub enum NixpkgsProblem { + ShardNonDir { + relative_shard_path: PathBuf, + }, + InvalidShardName { + relative_shard_path: PathBuf, + shard_name: String, + }, + PackageNonDir { + relative_package_dir: PathBuf, + }, + CaseSensitiveDuplicate { + relative_shard_path: PathBuf, + first: OsString, + second: OsString, + }, + InvalidPackageName { + relative_package_dir: PathBuf, + package_name: String, + }, + IncorrectShard { + relative_package_dir: PathBuf, + correct_relative_package_dir: PathBuf, + }, + PackageNixNonExistent { + relative_package_dir: PathBuf, + }, + PackageNixDir { + relative_package_dir: PathBuf, + }, + UndefinedAttr { + relative_package_file: PathBuf, + package_name: String, + }, + WrongCallPackage { + relative_package_file: PathBuf, + package_name: String, + }, + NonDerivation { + relative_package_file: PathBuf, + package_name: String, + }, + OutsideSymlink { + relative_package_dir: PathBuf, + subpath: PathBuf, + }, + UnresolvableSymlink { + relative_package_dir: PathBuf, + subpath: PathBuf, + io_error: io::Error, + }, + CouldNotParseNix { + relative_package_dir: PathBuf, + subpath: PathBuf, + error: ParseError, + }, + PathInterpolation { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + }, + SearchPath { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + }, + OutsidePathReference { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + }, + UnresolvablePathReference { + relative_package_dir: PathBuf, + subpath: PathBuf, + line: usize, + text: String, + io_error: io::Error, + }, +} + +impl fmt::Display for NixpkgsProblem { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + NixpkgsProblem::ShardNonDir { relative_shard_path } => + write!( + f, + "{}: This is a file, but it should be a directory.", + relative_shard_path.display(), + ), + NixpkgsProblem::InvalidShardName { relative_shard_path, shard_name } => + write!( + f, + "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".", + relative_shard_path.display() + ), + NixpkgsProblem::PackageNonDir { relative_package_dir } => + write!( + f, + "{}: This path is a file, but it should be a directory.", + relative_package_dir.display(), + ), + NixpkgsProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } => + write!( + f, + "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.", + relative_shard_path.display(), + ), + NixpkgsProblem::InvalidPackageName { relative_package_dir, package_name } => + write!( + f, + "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".", + relative_package_dir.display(), + ), + NixpkgsProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } => + write!( + f, + "{}: Incorrect directory location, should be {} instead.", + relative_package_dir.display(), + correct_relative_package_dir.display(), + ), + NixpkgsProblem::PackageNixNonExistent { relative_package_dir } => + write!( + f, + "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.", + relative_package_dir.display(), + ), + NixpkgsProblem::PackageNixDir { relative_package_dir } => + write!( + f, + "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.", + relative_package_dir.display(), + ), + NixpkgsProblem::UndefinedAttr { relative_package_file, package_name } => + write!( + f, + "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}", + relative_package_file.display() + ), + NixpkgsProblem::WrongCallPackage { relative_package_file, package_name } => + write!( + f, + "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.", + relative_package_file.display() + ), + NixpkgsProblem::NonDerivation { relative_package_file, package_name } => + write!( + f, + "pkgs.{package_name}: This attribute defined by {} is not a derivation", + relative_package_file.display() + ), + NixpkgsProblem::OutsideSymlink { relative_package_dir, subpath } => + write!( + f, + "{}: Path {} is a symlink pointing to a path outside the directory of that package.", + relative_package_dir.display(), + subpath.display(), + ), + NixpkgsProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } => + write!( + f, + "{}: Path {} is a symlink which cannot be resolved: {io_error}.", + relative_package_dir.display(), + subpath.display(), + ), + NixpkgsProblem::CouldNotParseNix { relative_package_dir, subpath, error } => + write!( + f, + "{}: File {} could not be parsed by rnix: {}", + relative_package_dir.display(), + subpath.display(), + error, + ), + NixpkgsProblem::PathInterpolation { relative_package_dir, subpath, line, text } => + write!( + f, + "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.", + relative_package_dir.display(), + subpath.display(), + text + ), + NixpkgsProblem::SearchPath { relative_package_dir, subpath, line, text } => + write!( + f, + "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.", + relative_package_dir.display(), + subpath.display(), + text + ), + NixpkgsProblem::OutsidePathReference { relative_package_dir, subpath, line, text } => + write!( + f, + "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.", + relative_package_dir.display(), + subpath.display(), + text, + ), + NixpkgsProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } => + write!( + f, + "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.", + relative_package_dir.display(), + subpath.display(), + text, + ), + } + } +} diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index a6bc53d717d7..37837a54ddc2 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,5 +1,6 @@ use crate::check_result; -use crate::check_result::{CheckProblem, CheckResult}; +use crate::check_result::CheckResult; +use crate::nixpkgs_problem::NixpkgsProblem; use crate::utils; use crate::utils::LineIndex; @@ -47,7 +48,7 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - CheckProblem::OutsideSymlink { + NixpkgsProblem::OutsideSymlink { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), } @@ -56,7 +57,7 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { check_result::ok(()) } } - Err(io_error) => CheckProblem::UnresolvableSymlink { + Err(io_error) => NixpkgsProblem::UnresolvableSymlink { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), io_error, @@ -104,7 +105,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let root = Root::parse(&contents); if let Some(error) = root.errors().first() { - return CheckProblem::CouldNotParseNix { + return NixpkgsProblem::CouldNotParseNix { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), error: error.clone(), @@ -124,7 +125,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { } else if node.children().count() != 0 { // Filters out ./foo/${bar}/baz // TODO: We can just check ./foo - CheckProblem::PathInterpolation { + NixpkgsProblem::PathInterpolation { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -133,7 +134,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { .into_result() } else if text.starts_with('<') { // Filters out search paths like - CheckProblem::SearchPath { + NixpkgsProblem::SearchPath { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -149,7 +150,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - CheckProblem::OutsidePathReference { + NixpkgsProblem::OutsidePathReference { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, @@ -160,7 +161,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { check_result::ok(()) } } - Err(e) => CheckProblem::UnresolvablePathReference { + Err(e) => NixpkgsProblem::UnresolvablePathReference { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 90a552a38863..b69f6211c0a0 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,10 +1,13 @@ use crate::check_result; -use crate::check_result::{CheckProblem, CheckResult}; +use crate::check_result::CheckResult; +use crate::nixpkgs_problem::NixpkgsProblem; use crate::references; use crate::utils; use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME}; +use itertools::concat; use lazy_static::lazy_static; use regex::Regex; +use std::fs::DirEntry; use std::path::{Path, PathBuf}; lazy_static! { @@ -30,8 +33,8 @@ pub fn relative_file_for_package(package_name: &str) -> PathBuf { relative_dir_for_package(package_name).join(PACKAGE_NIX_FILENAME) } -/// Read the structure of a Nixpkgs directory, displaying errors on the writer. -/// May return early with I/O errors. +/// Check the structure of Nixpkgs, returning the attribute names that are defined in +/// `pkgs/by-name` pub fn check_structure(path: &Path) -> CheckResult> { let base_dir = path.join(BASE_SUBPATH); @@ -46,7 +49,7 @@ pub fn check_structure(path: &Path) -> CheckResult> { // README.md is allowed to be a file and not checked check_result::ok(vec![]) } else if !shard_path.is_dir() { - CheckProblem::ShardNonDir { + NixpkgsProblem::ShardNonDir { relative_shard_path: relative_shard_path.clone(), } .into_result() @@ -54,7 +57,7 @@ pub fn check_structure(path: &Path) -> CheckResult> { } else { let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); let result = if !shard_name_valid { - CheckProblem::InvalidShardName { + NixpkgsProblem::InvalidShardName { relative_shard_path: relative_shard_path.clone(), shard_name: shard_name.clone(), } @@ -72,7 +75,7 @@ pub fn check_structure(path: &Path) -> CheckResult> { l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase() }) .map(|(l, r)| { - CheckProblem::CaseSensitiveDuplicate { + NixpkgsProblem::CaseSensitiveDuplicate { relative_shard_path: relative_shard_path.clone(), first: l.file_name(), second: r.file_name(), @@ -83,84 +86,87 @@ pub fn check_structure(path: &Path) -> CheckResult> { let result = check_result::and(result, check_result::sequence_(duplicate_results)); let package_results = entries.into_iter().map(|package_entry| { - 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}")); - - if !package_path.is_dir() { - CheckProblem::PackageNonDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); - let result = if !package_name_valid { - CheckProblem::InvalidPackageName { - relative_package_dir: relative_package_dir.clone(), - package_name: package_name.clone(), - } - .into_result() - } else { - check_result::ok(()) - }; - - let correct_relative_package_dir = relative_dir_for_package(&package_name); - let result = check_result::and( - result, - if relative_package_dir != correct_relative_package_dir { - // Only show this error if we have a valid shard and package name - // Because if one of those is wrong, you should fix that first - if shard_name_valid && package_name_valid { - CheckProblem::IncorrectShard { - relative_package_dir: relative_package_dir.clone(), - correct_relative_package_dir: correct_relative_package_dir - .clone(), - } - .into_result() - } else { - check_result::ok(()) - } - } else { - check_result::ok(()) - }, - ); - - let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let result = check_result::and( - result, - if !package_nix_path.exists() { - CheckProblem::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - CheckProblem::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - check_result::ok(()) - }, - ); - - let result = check_result::and( - result, - references::check_references( - &relative_package_dir, - &path.join(&relative_package_dir), - ), - ); - - check_result::map(result, |_| package_name.clone()) - } + check_package(path, &shard_name, shard_name_valid, package_entry) }); check_result::and(result, check_result::sequence(package_results)) } }); - check_result::map(check_result::sequence(shard_results), |x| { - x.into_iter().flatten().collect::>() - }) + // Combine the package names conatained within each shard into a longer list + check_result::map(check_result::sequence(shard_results), concat) +} + +fn check_package( + path: &Path, + shard_name: &str, + shard_name_valid: bool, + package_entry: DirEntry, +) -> CheckResult { + 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}")); + + if !package_path.is_dir() { + NixpkgsProblem::PackageNonDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); + let result = if !package_name_valid { + NixpkgsProblem::InvalidPackageName { + relative_package_dir: relative_package_dir.clone(), + package_name: package_name.clone(), + } + .into_result() + } else { + check_result::ok(()) + }; + + let correct_relative_package_dir = relative_dir_for_package(&package_name); + let result = check_result::and( + result, + if relative_package_dir != correct_relative_package_dir { + // Only show this error if we have a valid shard and package name + // Because if one of those is wrong, you should fix that first + if shard_name_valid && package_name_valid { + NixpkgsProblem::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: correct_relative_package_dir.clone(), + } + .into_result() + } else { + check_result::ok(()) + } + } else { + check_result::ok(()) + }, + ); + + let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); + let result = check_result::and( + result, + if !package_nix_path.exists() { + NixpkgsProblem::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else if package_nix_path.is_dir() { + NixpkgsProblem::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into_result() + } else { + check_result::ok(()) + }, + ); + + let result = check_result::and( + result, + references::check_references(&relative_package_dir, &path.join(&relative_package_dir)), + ); + + check_result::map(result, |_| package_name.clone()) + } } From 03c58ad1d687e8928b2b59f8dc44fb2c4c43300f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 24 Oct 2023 00:20:32 +0200 Subject: [PATCH 29/52] tests.nixpkgs-check-by-name: Minor doc updates --- pkgs/test/nixpkgs-check-by-name/README.md | 4 ++-- pkgs/test/nixpkgs-check-by-name/src/main.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/README.md b/pkgs/test/nixpkgs-check-by-name/README.md index 2d2db9a58bc5..146cea0a64ba 100644 --- a/pkgs/test/nixpkgs-check-by-name/README.md +++ b/pkgs/test/nixpkgs-check-by-name/README.md @@ -1,6 +1,6 @@ # Nixpkgs pkgs/by-name checker -This directory implements a program to check the [validity](#validity-checks) of the `pkgs/by-name` Nixpkgs directory once introduced. +This directory implements a program to check the [validity](#validity-checks) of the `pkgs/by-name` Nixpkgs directory. It is being used by [this GitHub Actions workflow](../../../.github/workflows/check-by-name.yml). This is part of the implementation of [RFC 140](https://github.com/NixOS/rfcs/pull/140). @@ -24,7 +24,7 @@ This API may be changed over time if the CI workflow making use of it is adjuste - `2`: If an unexpected I/O error occurs - Standard error: - Informative messages - - Error messages if validation is not successful + - Detected problems if validation is not successful ## Validity checks diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 1651624f3f40..1f52d81c36cf 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -68,8 +68,8 @@ fn main() -> ExitCode { /// /// # Return value /// - `Err(e)` if an I/O-related error `e` occurred. -/// - `Ok(false)` if the structure is invalid, all the structural errors have been written to `error_writer`. -/// - `Ok(true)` if the structure is valid, nothing will have been written to `error_writer`. +/// - `Ok(false)` if there are problems, all of which will be written to `error_writer`. +/// - `Ok(true)` if there are no problems pub fn check_nixpkgs( nixpkgs_path: &Path, version: Version, From 1d3bc272bc31389c06ae1670d7577cab760ffd25 Mon Sep 17 00:00:00 2001 From: Adam Stephens Date: Tue, 24 Oct 2023 08:09:37 -0400 Subject: [PATCH 30/52] calibre-web: 0.6.20 -> 0.6.21 --- pkgs/servers/calibre-web/default.nix | 6 ++--- pkgs/servers/calibre-web/static_environ.patch | 25 ------------------- 2 files changed, 2 insertions(+), 29 deletions(-) delete mode 100644 pkgs/servers/calibre-web/static_environ.patch diff --git a/pkgs/servers/calibre-web/default.nix b/pkgs/servers/calibre-web/default.nix index f59ebb6fe20f..160435802957 100644 --- a/pkgs/servers/calibre-web/default.nix +++ b/pkgs/servers/calibre-web/default.nix @@ -25,13 +25,13 @@ let in python.pkgs.buildPythonApplication rec { pname = "calibre-web"; - version = "0.6.20"; + version = "0.6.21"; src = fetchFromGitHub { owner = "janeczku"; repo = "calibre-web"; rev = version; - hash = "sha256-0lArY1aTpO4sgIVDSqClYMGlip92f9hE/L2UouTLK8Q="; + hash = "sha256-tRrOquetn3P2NmrXq7DQHRGP1sWnLR7bV2Lw0W/lUPQ="; }; propagatedBuildInputs = with python.pkgs; [ @@ -64,8 +64,6 @@ python.pkgs.buildPythonApplication rec { # and exit. This is gonna be used to configure calibre-web declaratively, as most of its configuration parameters # are stored in the DB. ./db-migrations.patch - # environ in tornado.wsgi.WSGIContainer no longer a static method from 6.3 version - ./static_environ.patch ]; # calibre-web doesn't follow setuptools directory structure. The following is taken from the script diff --git a/pkgs/servers/calibre-web/static_environ.patch b/pkgs/servers/calibre-web/static_environ.patch deleted file mode 100644 index 4f94283a4e66..000000000000 --- a/pkgs/servers/calibre-web/static_environ.patch +++ /dev/null @@ -1,25 +0,0 @@ -diff --git a/cps/tornado_wsgi.py b/cps/tornado_wsgi.py -index af93219c..cf302042 100644 ---- a/cps/tornado_wsgi.py -+++ b/cps/tornado_wsgi.py -@@ -53,7 +53,7 @@ class MyWSGIContainer(WSGIContainer): - return response.append - - app_response = self.wsgi_application( -- MyWSGIContainer.environ(request), start_response -+ self.environ(request), start_response - ) - try: - response.extend(app_response) -@@ -86,9 +86,8 @@ class MyWSGIContainer(WSGIContainer): - request.connection.finish() - self._log(status_code, request) - -- @staticmethod -- def environ(request: httputil.HTTPServerRequest) -> Dict[Text, Any]: -- environ = WSGIContainer.environ(request) -+ def environ(self, request: httputil.HTTPServerRequest) -> Dict[Text, Any]: -+ environ = super().environ(request) - environ['RAW_URI'] = request.path - return environ - From 82e708c19230f77fbb5ea01157fc1226e72119de Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 24 Oct 2023 19:27:48 +0200 Subject: [PATCH 31/52] tests.nixpkgs-check-by-name: Custom Validation type and improvements Co-authored-by: Wanja Hentze --- .../nixpkgs-check-by-name/src/check_result.rs | 74 --------- pkgs/test/nixpkgs-check-by-name/src/eval.rs | 93 +++++------ pkgs/test/nixpkgs-check-by-name/src/main.rs | 22 ++- .../nixpkgs-check-by-name/src/references.rs | 148 +++++++++--------- .../nixpkgs-check-by-name/src/structure.rs | 120 +++++++------- .../nixpkgs-check-by-name/src/validation.rs | 102 ++++++++++++ 6 files changed, 296 insertions(+), 263 deletions(-) delete mode 100644 pkgs/test/nixpkgs-check-by-name/src/check_result.rs create mode 100644 pkgs/test/nixpkgs-check-by-name/src/validation.rs diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs deleted file mode 100644 index a6538d778b61..000000000000 --- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs +++ /dev/null @@ -1,74 +0,0 @@ -use crate::nixpkgs_problem::NixpkgsProblem; -use itertools::concat; -use itertools::{ - Either, - Either::{Left, Right}, - Itertools, -}; - -/// A type alias representing the result of a check, either: -/// - Err(anyhow::Error): A fatal failure, typically I/O errors. -/// Such failures are not caused by the files in Nixpkgs. -/// This hints at a bug in the code or a problem with the deployment. -/// - Ok(Left(Vec)): A non-fatal problem with the Nixpkgs files. -/// Further checks can be run even with this result type. -/// Such problems can be fixed by changing the Nixpkgs files. -/// - Ok(Right(A)): A successful (potentially intermediate) result with an arbitrary value. -/// No fatal errors have occurred and no problems have been found with Nixpkgs. -pub type CheckResult = anyhow::Result, A>>; - -/// Map a `CheckResult` to a `CheckResult` by applying a function to the -/// potentially contained value in case of success. -pub fn map(check_result: CheckResult, f: impl FnOnce(I) -> O) -> CheckResult { - Ok(check_result?.map_right(f)) -} - -/// Create a successfull `CheckResult` from a value `A`. -pub fn ok(value: A) -> CheckResult { - Ok(Right(value)) -} - -impl NixpkgsProblem { - /// Create a `CheckResult` from a single check problem - pub fn into_result(self) -> CheckResult { - Ok(Left(vec![self])) - } -} - -/// Combine two check results, both of which need to be successful for the return value to be successful. -/// The `NixpkgsProblem`s of both sides are returned concatenated. -pub fn and(first: CheckResult<()>, second: CheckResult) -> CheckResult { - match (first?, second?) { - (Right(_), Right(right_value)) => Ok(Right(right_value)), - (Left(errors), Right(_)) => Ok(Left(errors)), - (Right(_), Left(errors)) => Ok(Left(errors)), - (Left(errors_l), Left(errors_r)) => Ok(Left(concat([errors_l, errors_r]))), - } -} - -/// Combine many checks results into a single one. -/// All given check results need to be successful in order for the returned check result to be successful, -/// in which case the returned check result value contains each a `Vec` of each individual result value. -/// The `NixpkgsProblem`s of all results are returned concatenated. -pub fn sequence(check_results: impl IntoIterator>) -> CheckResult> { - let (errors, values): (Vec<_>, Vec<_>) = check_results - .into_iter() - .collect::>>()? - .into_iter() - .partition_map(|r| r); - - // To combine the errors from the results we flatten all the error Vec's into a new Vec - // This is not very efficient, but doesn't matter because generally we should have no errors - let flattened_errors = errors.into_iter().flatten().collect::>(); - - if flattened_errors.is_empty() { - Ok(Right(values)) - } else { - Ok(Left(flattened_errors)) - } -} - -/// Like `sequence`, but replace the resulting value with () -pub fn sequence_(check_results: impl IntoIterator>) -> CheckResult<()> { - map(sequence(check_results), |_| ()) -} diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs index 37fc783f3d34..e4f986748b4f 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs @@ -1,7 +1,6 @@ -use crate::check_result; -use crate::check_result::CheckResult; use crate::nixpkgs_problem::NixpkgsProblem; use crate::structure; +use crate::validation::{self, Validation::Success}; use crate::Version; use std::path::Path; @@ -46,7 +45,7 @@ pub fn check_values( nixpkgs_path: &Path, package_names: Vec, eval_accessible_paths: Vec<&Path>, -) -> CheckResult<()> { +) -> validation::Result<()> { // Write the list of packages we need to check into a temporary JSON file. // This can then get read by the Nix evaluation. let attrs_file = NamedTempFile::new().context("Failed to create a temporary file")?; @@ -112,52 +111,54 @@ pub fn check_values( String::from_utf8_lossy(&result.stdout) ))?; - check_result::sequence_(package_names.iter().map(|package_name| { - let relative_package_file = structure::relative_file_for_package(package_name); - let absolute_package_file = nixpkgs_path.join(&relative_package_file); + Ok(validation::sequence_(package_names.iter().map( + |package_name| { + let relative_package_file = structure::relative_file_for_package(package_name); + let absolute_package_file = nixpkgs_path.join(&relative_package_file); - if let Some(attribute_info) = actual_files.get(package_name) { - let valid = match &attribute_info.variant { - AttributeVariant::AutoCalled => true, - AttributeVariant::CallPackage { path, empty_arg } => { - let correct_file = if let Some(call_package_path) = path { - absolute_package_file == *call_package_path - } else { - false - }; - // Only check for the argument to be non-empty if the version is V1 or - // higher - let non_empty = if version >= Version::V1 { - !empty_arg - } else { - true - }; - correct_file && non_empty - } - AttributeVariant::Other => false, - }; + if let Some(attribute_info) = actual_files.get(package_name) { + let valid = match &attribute_info.variant { + AttributeVariant::AutoCalled => true, + AttributeVariant::CallPackage { path, empty_arg } => { + let correct_file = if let Some(call_package_path) = path { + absolute_package_file == *call_package_path + } else { + false + }; + // Only check for the argument to be non-empty if the version is V1 or + // higher + let non_empty = if version >= Version::V1 { + !empty_arg + } else { + true + }; + correct_file && non_empty + } + AttributeVariant::Other => false, + }; - if !valid { - NixpkgsProblem::WrongCallPackage { - relative_package_file: relative_package_file.clone(), - package_name: package_name.clone(), + if !valid { + NixpkgsProblem::WrongCallPackage { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into() + } else if !attribute_info.is_derivation { + NixpkgsProblem::NonDerivation { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into() + } else { + Success(()) } - .into_result() - } else if !attribute_info.is_derivation { - NixpkgsProblem::NonDerivation { - relative_package_file: relative_package_file.clone(), - package_name: package_name.clone(), - } - .into_result() } else { - check_result::ok(()) + NixpkgsProblem::UndefinedAttr { + relative_package_file: relative_package_file.clone(), + package_name: package_name.clone(), + } + .into() } - } else { - NixpkgsProblem::UndefinedAttr { - relative_package_file: relative_package_file.clone(), - package_name: package_name.clone(), - } - .into_result() - } - })) + }, + ))) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 1f52d81c36cf..4cabf8f446f5 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -1,18 +1,16 @@ -mod check_result; mod eval; mod nixpkgs_problem; mod references; mod structure; mod utils; +mod validation; use crate::structure::check_structure; +use crate::validation::Validation::Failure; +use crate::validation::Validation::Success; use anyhow::Context; use clap::{Parser, ValueEnum}; use colored::Colorize; -use itertools::{ - Either, - Either::{Left, Right}, -}; use std::io; use std::path::{Path, PathBuf}; use std::process::ExitCode; @@ -86,26 +84,26 @@ pub fn check_nixpkgs( "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.", utils::BASE_SUBPATH ); - check_result::ok(()) + Success(()) } else { match check_structure(&nixpkgs_path)? { - Left(errors) => Ok(Left(errors)), - Right(package_names) => + Failure(errors) => Failure(errors), + Success(package_names) => // Only if we could successfully parse the structure, we do the evaluation checks { - eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths) + eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths)? } } }; - match check_result? { - Either::Left(errors) => { + match check_result { + Failure(errors) => { for error in errors { writeln!(error_writer, "{}", error.to_string().red())? } Ok(false) } - Either::Right(_) => Ok(true), + Success(_) => Ok(true), } } diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 37837a54ddc2..c437ebd222f8 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -1,8 +1,7 @@ -use crate::check_result; -use crate::check_result::CheckResult; use crate::nixpkgs_problem::NixpkgsProblem; use crate::utils; use crate::utils::LineIndex; +use crate::validation::{self, ResultIteratorExt, Validation::Success}; use anyhow::Context; use rnix::{Root, SyntaxKind::NODE_PATH}; @@ -23,7 +22,7 @@ struct PackageContext<'a> { pub fn check_references( relative_package_dir: &Path, absolute_package_dir: &Path, -) -> CheckResult<()> { +) -> validation::Result<()> { let context = PackageContext { relative_package_dir: &relative_package_dir.to_path_buf(), absolute_package_dir: &absolute_package_dir.to_path_buf(), @@ -38,10 +37,10 @@ pub fn check_references( } /// Checks for a specific path to not have references outside -fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { +fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<()> { let path = context.absolute_package_dir.join(subpath); - if path.is_symlink() { + Ok(if path.is_symlink() { // Check whether the symlink resolves to outside the package directory match path.canonicalize() { Ok(target) => { @@ -52,9 +51,9 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), } - .into_result() + .into() } else { - check_result::ok(()) + Success(()) } } Err(io_error) => NixpkgsProblem::UnresolvableSymlink { @@ -62,15 +61,20 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { subpath: subpath.to_path_buf(), io_error, } - .into_result(), + .into(), } } else if path.is_dir() { // Recursively check each entry - check_result::sequence_(utils::read_dir_sorted(&path)?.into_iter().map(|entry| { - let entry_subpath = subpath.join(entry.file_name()); - check_path(context, &entry_subpath) - .context(format!("Error while recursing into {}", subpath.display())) - })) + validation::sequence_( + utils::read_dir_sorted(&path)? + .into_iter() + .map(|entry| { + let entry_subpath = subpath.join(entry.file_name()); + check_path(context, &entry_subpath) + .context(format!("Error while recursing into {}", subpath.display())) + }) + .collect_vec()?, + ) } else if path.is_file() { // Only check Nix files if let Some(ext) = path.extension() { @@ -78,22 +82,22 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> { check_nix_file(context, subpath).context(format!( "Error while checking Nix file {}", subpath.display() - )) + ))? } else { - check_result::ok(()) + Success(()) } } else { - check_result::ok(()) + Success(()) } } else { // This should never happen, git doesn't support other file types anyhow::bail!("Unsupported file type for path {}", subpath.display()); - } + }) } /// Check whether a nix file contains path expression references pointing outside the package /// directory -fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { +fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Result<()> { let path = context.absolute_package_dir.join(subpath); let parent_dir = path.parent().context(format!( "Could not get parent of path {}", @@ -105,71 +109,75 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> { let root = Root::parse(&contents); if let Some(error) = root.errors().first() { - return NixpkgsProblem::CouldNotParseNix { + return Ok(NixpkgsProblem::CouldNotParseNix { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), error: error.clone(), } - .into_result(); + .into()); } let line_index = LineIndex::new(&contents); - check_result::sequence_(root.syntax().descendants().map(|node| { - let text = node.text().to_string(); - let line = line_index.line(node.text_range().start().into()); + Ok(validation::sequence_(root.syntax().descendants().map( + |node| { + let text = node.text().to_string(); + let line = line_index.line(node.text_range().start().into()); - if node.kind() != NODE_PATH { - // We're only interested in Path expressions - check_result::ok(()) - } else if node.children().count() != 0 { - // Filters out ./foo/${bar}/baz - // TODO: We can just check ./foo - NixpkgsProblem::PathInterpolation { - relative_package_dir: context.relative_package_dir.clone(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into_result() - } else if text.starts_with('<') { - // Filters out search paths like - NixpkgsProblem::SearchPath { - relative_package_dir: context.relative_package_dir.clone(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into_result() - } else { - // Resolves the reference of the Nix path - // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz` - match parent_dir.join(Path::new(&text)).canonicalize() { - Ok(target) => { - // Then checking if it's still in the package directory - // No need to handle the case of it being inside the directory, since we scan through the - // entire directory recursively anyways - if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { - NixpkgsProblem::OutsidePathReference { - relative_package_dir: context.relative_package_dir.clone(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into_result() - } else { - check_result::ok(()) - } - } - Err(e) => NixpkgsProblem::UnresolvablePathReference { + if node.kind() != NODE_PATH { + // We're only interested in Path expressions + Success(()) + } else if node.children().count() != 0 { + // Filters out ./foo/${bar}/baz + // TODO: We can just check ./foo + NixpkgsProblem::PathInterpolation { relative_package_dir: context.relative_package_dir.clone(), subpath: subpath.to_path_buf(), line, text, - io_error: e, } - .into_result(), + .into() + } else if text.starts_with('<') { + // Filters out search paths like + NixpkgsProblem::SearchPath { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + } + .into() + } else { + // Resolves the reference of the Nix path + // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz` + match parent_dir.join(Path::new(&text)).canonicalize() { + Ok(target) => { + // Then checking if it's still in the package directory + // No need to handle the case of it being inside the directory, since we scan through the + // entire directory recursively anyways + if let Err(_prefix_error) = + target.strip_prefix(context.absolute_package_dir) + { + NixpkgsProblem::OutsidePathReference { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + } + .into() + } else { + Success(()) + } + } + Err(e) => NixpkgsProblem::UnresolvablePathReference { + relative_package_dir: context.relative_package_dir.clone(), + subpath: subpath.to_path_buf(), + line, + text, + io_error: e, + } + .into(), + } } - } - })) + }, + ))) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index b69f6211c0a0..4051ca037c9a 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -1,9 +1,8 @@ -use crate::check_result; -use crate::check_result::CheckResult; use crate::nixpkgs_problem::NixpkgsProblem; use crate::references; use crate::utils; use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME}; +use crate::validation::{self, ResultIteratorExt, Validation::Success}; use itertools::concat; use lazy_static::lazy_static; use regex::Regex; @@ -35,24 +34,25 @@ pub fn relative_file_for_package(package_name: &str) -> PathBuf { /// Check the structure of Nixpkgs, returning the attribute names that are defined in /// `pkgs/by-name` -pub fn check_structure(path: &Path) -> CheckResult> { +pub fn check_structure(path: &Path) -> validation::Result> { let base_dir = path.join(BASE_SUBPATH); let shard_results = utils::read_dir_sorted(&base_dir)? .into_iter() - .map(|shard_entry| { + .map(|shard_entry| -> validation::Result<_> { let shard_path = shard_entry.path(); let shard_name = shard_entry.file_name().to_string_lossy().into_owned(); let relative_shard_path = relative_dir_for_shard(&shard_name); - if shard_name == "README.md" { + Ok(if shard_name == "README.md" { // README.md is allowed to be a file and not checked - check_result::ok(vec![]) + + Success(vec![]) } else if !shard_path.is_dir() { NixpkgsProblem::ShardNonDir { relative_shard_path: relative_shard_path.clone(), } - .into_result() + .into() // we can't check for any other errors if it's a file, since there's no subdirectories to check } else { let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name); @@ -61,9 +61,9 @@ pub fn check_structure(path: &Path) -> CheckResult> { relative_shard_path: relative_shard_path.clone(), shard_name: shard_name.clone(), } - .into_result() + .into() } else { - check_result::ok(()) + Success(()) }; let entries = utils::read_dir_sorted(&shard_path)?; @@ -80,21 +80,25 @@ pub fn check_structure(path: &Path) -> CheckResult> { first: l.file_name(), second: r.file_name(), } - .into_result::<()>() + .into() }); - let result = check_result::and(result, check_result::sequence_(duplicate_results)); + let result = result.and(validation::sequence_(duplicate_results)); - let package_results = entries.into_iter().map(|package_entry| { - check_package(path, &shard_name, shard_name_valid, package_entry) - }); + let package_results = entries + .into_iter() + .map(|package_entry| { + check_package(path, &shard_name, shard_name_valid, package_entry) + }) + .collect_vec()?; - check_result::and(result, check_result::sequence(package_results)) - } - }); + result.and(validation::sequence(package_results)) + }) + }) + .collect_vec()?; // Combine the package names conatained within each shard into a longer list - check_result::map(check_result::sequence(shard_results), concat) + Ok(validation::sequence(shard_results).map(concat)) } fn check_package( @@ -102,16 +106,16 @@ fn check_package( shard_name: &str, shard_name_valid: bool, package_entry: DirEntry, -) -> CheckResult { +) -> validation::Result { 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}")); - if !package_path.is_dir() { + Ok(if !package_path.is_dir() { NixpkgsProblem::PackageNonDir { relative_package_dir: relative_package_dir.clone(), } - .into_result() + .into() } else { let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name); let result = if !package_name_valid { @@ -119,54 +123,48 @@ fn check_package( relative_package_dir: relative_package_dir.clone(), package_name: package_name.clone(), } - .into_result() + .into() } else { - check_result::ok(()) + Success(()) }; let correct_relative_package_dir = relative_dir_for_package(&package_name); - let result = check_result::and( - result, - if relative_package_dir != correct_relative_package_dir { - // Only show this error if we have a valid shard and package name - // Because if one of those is wrong, you should fix that first - if shard_name_valid && package_name_valid { - NixpkgsProblem::IncorrectShard { - relative_package_dir: relative_package_dir.clone(), - correct_relative_package_dir: correct_relative_package_dir.clone(), - } - .into_result() - } else { - check_result::ok(()) + let result = result.and(if relative_package_dir != correct_relative_package_dir { + // Only show this error if we have a valid shard and package name + // Because if one of those is wrong, you should fix that first + if shard_name_valid && package_name_valid { + NixpkgsProblem::IncorrectShard { + relative_package_dir: relative_package_dir.clone(), + correct_relative_package_dir: correct_relative_package_dir.clone(), } + .into() } else { - check_result::ok(()) - }, - ); + Success(()) + } + } else { + Success(()) + }); let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME); - let result = check_result::and( - result, - if !package_nix_path.exists() { - NixpkgsProblem::PackageNixNonExistent { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else if package_nix_path.is_dir() { - NixpkgsProblem::PackageNixDir { - relative_package_dir: relative_package_dir.clone(), - } - .into_result() - } else { - check_result::ok(()) - }, - ); + let result = result.and(if !package_nix_path.exists() { + NixpkgsProblem::PackageNixNonExistent { + relative_package_dir: relative_package_dir.clone(), + } + .into() + } else if package_nix_path.is_dir() { + NixpkgsProblem::PackageNixDir { + relative_package_dir: relative_package_dir.clone(), + } + .into() + } else { + Success(()) + }); - let result = check_result::and( - result, - references::check_references(&relative_package_dir, &path.join(&relative_package_dir)), - ); + let result = result.and(references::check_references( + &relative_package_dir, + &path.join(&relative_package_dir), + )?); - check_result::map(result, |_| package_name.clone()) - } + result.map(|_| package_name.clone()) + }) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/validation.rs b/pkgs/test/nixpkgs-check-by-name/src/validation.rs new file mode 100644 index 000000000000..e72793851521 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/src/validation.rs @@ -0,0 +1,102 @@ +use crate::nixpkgs_problem::NixpkgsProblem; +use itertools::concat; +use itertools::{ + Either::{Left, Right}, + Itertools, +}; +use Validation::*; + +/// The validation result of a check. +/// Instead of exiting at the first failure, +/// this type can accumulate multiple failures. +/// This can be achieved using the functions `and`, `sequence` and `sequence_` +/// +/// This leans on https://hackage.haskell.org/package/validation +pub enum Validation { + Failure(Vec), + Success(A), +} + +impl From for Validation { + /// Create a `Validation` from a single check problem + fn from(value: NixpkgsProblem) -> Self { + Failure(vec![value]) + } +} + +/// A type alias representing the result of a check, either: +/// - Err(anyhow::Error): A fatal failure, typically I/O errors. +/// Such failures are not caused by the files in Nixpkgs. +/// This hints at a bug in the code or a problem with the deployment. +/// - Ok(Failure(Vec)): A non-fatal validation problem with the Nixpkgs files. +/// Further checks can be run even with this result type. +/// Such problems can be fixed by changing the Nixpkgs files. +/// - Ok(Success(A)): A successful (potentially intermediate) result with an arbitrary value. +/// No fatal errors have occurred and no validation problems have been found with Nixpkgs. +pub type Result = anyhow::Result>; + +pub trait ResultIteratorExt: Sized + Iterator> { + fn collect_vec(self) -> std::result::Result, E>; +} + +impl ResultIteratorExt for I +where + I: Sized + Iterator>, +{ + /// A convenience version of `collect` specialised to a vector + fn collect_vec(self) -> std::result::Result, E> { + self.collect() + } +} + +impl Validation { + /// Map a `Validation` to a `Validation` by applying a function to the + /// potentially contained value in case of success. + pub fn map(self, f: impl FnOnce(A) -> B) -> Validation { + match self { + Failure(err) => Failure(err), + Success(value) => Success(f(value)), + } + } +} + +impl Validation<()> { + /// Combine two validations, both of which need to be successful for the return value to be successful. + /// The `NixpkgsProblem`s of both sides are returned concatenated. + pub fn and(self, other: Validation) -> Validation { + match (self, other) { + (Success(_), Success(right_value)) => Success(right_value), + (Failure(errors), Success(_)) => Failure(errors), + (Success(_), Failure(errors)) => Failure(errors), + (Failure(errors_l), Failure(errors_r)) => Failure(concat([errors_l, errors_r])), + } + } +} + +/// Combine many validations into a single one. +/// All given validations need to be successful in order for the returned validation to be successful, +/// in which case the returned validation value contains a `Vec` of each individual value. +/// Otherwise the `NixpkgsProblem`s of all validations are returned concatenated. +pub fn sequence(check_results: impl IntoIterator>) -> Validation> { + let (errors, values): (Vec>, Vec) = check_results + .into_iter() + .partition_map(|validation| match validation { + Failure(err) => Left(err), + Success(value) => Right(value), + }); + + // To combine the errors from the results we flatten all the error Vec's into a new Vec + // This is not very efficient, but doesn't matter because generally we should have no errors + let flattened_errors = errors.into_iter().concat(); + + if flattened_errors.is_empty() { + Success(values) + } else { + Failure(flattened_errors) + } +} + +/// Like `sequence`, but without any containing value, for convenience +pub fn sequence_(validations: impl IntoIterator>) -> Validation<()> { + sequence(validations).map(|_| ()) +} From 7753969628f2d229d2e9fc40a904d5f08dece09b Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 24 Oct 2023 19:37:52 +0200 Subject: [PATCH 32/52] tests.nixpkgs-check-by-name: Remove PackageContext helper Was not really necessary anymore --- .../nixpkgs-check-by-name/src/references.rs | 62 ++++++++----------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index c437ebd222f8..0561a9b22e85 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -7,15 +7,7 @@ use anyhow::Context; use rnix::{Root, SyntaxKind::NODE_PATH}; use std::ffi::OsStr; use std::fs::read_to_string; -use std::path::{Path, PathBuf}; - -/// Small helper so we don't need to pass in the same arguments to all functions -struct PackageContext<'a> { - /// The package directory relative to Nixpkgs, such as `pkgs/by-name/fo/foo` - relative_package_dir: &'a PathBuf, - /// The absolute package directory - absolute_package_dir: &'a PathBuf, -} +use std::path::Path; /// Check that every package directory in pkgs/by-name doesn't link to outside that directory. /// Both symlinks and Nix path expressions are checked. @@ -23,22 +15,21 @@ pub fn check_references( relative_package_dir: &Path, absolute_package_dir: &Path, ) -> validation::Result<()> { - let context = PackageContext { - relative_package_dir: &relative_package_dir.to_path_buf(), - absolute_package_dir: &absolute_package_dir.to_path_buf(), - }; - // The empty argument here is the subpath under the package directory to check // An empty one means the package directory itself - check_path(&context, Path::new("")).context(format!( + check_path(relative_package_dir, absolute_package_dir, Path::new("")).context(format!( "While checking the references in package directory {}", relative_package_dir.display() )) } /// Checks for a specific path to not have references outside -fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<()> { - let path = context.absolute_package_dir.join(subpath); +fn check_path( + relative_package_dir: &Path, + absolute_package_dir: &Path, + subpath: &Path, +) -> validation::Result<()> { + let path = absolute_package_dir.join(subpath); Ok(if path.is_symlink() { // Check whether the symlink resolves to outside the package directory @@ -46,9 +37,9 @@ fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<() Ok(target) => { // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways - if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) { + if let Err(_prefix_error) = target.strip_prefix(absolute_package_dir) { NixpkgsProblem::OutsideSymlink { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), } .into() @@ -57,7 +48,7 @@ fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<() } } Err(io_error) => NixpkgsProblem::UnresolvableSymlink { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), io_error, } @@ -70,7 +61,7 @@ fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<() .into_iter() .map(|entry| { let entry_subpath = subpath.join(entry.file_name()); - check_path(context, &entry_subpath) + check_path(relative_package_dir, absolute_package_dir, &entry_subpath) .context(format!("Error while recursing into {}", subpath.display())) }) .collect_vec()?, @@ -79,10 +70,9 @@ fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<() // Only check Nix files if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { - check_nix_file(context, subpath).context(format!( - "Error while checking Nix file {}", - subpath.display() - ))? + check_nix_file(relative_package_dir, absolute_package_dir, subpath).context( + format!("Error while checking Nix file {}", subpath.display()), + )? } else { Success(()) } @@ -97,8 +87,12 @@ fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<() /// Check whether a nix file contains path expression references pointing outside the package /// directory -fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Result<()> { - let path = context.absolute_package_dir.join(subpath); +fn check_nix_file( + relative_package_dir: &Path, + absolute_package_dir: &Path, + subpath: &Path, +) -> validation::Result<()> { + let path = absolute_package_dir.join(subpath); let parent_dir = path.parent().context(format!( "Could not get parent of path {}", subpath.display() @@ -110,7 +104,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Resul let root = Root::parse(&contents); if let Some(error) = root.errors().first() { return Ok(NixpkgsProblem::CouldNotParseNix { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), error: error.clone(), } @@ -131,7 +125,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Resul // Filters out ./foo/${bar}/baz // TODO: We can just check ./foo NixpkgsProblem::PathInterpolation { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), line, text, @@ -140,7 +134,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Resul } else if text.starts_with('<') { // Filters out search paths like NixpkgsProblem::SearchPath { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), line, text, @@ -154,11 +148,9 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Resul // Then checking if it's still in the package directory // No need to handle the case of it being inside the directory, since we scan through the // entire directory recursively anyways - if let Err(_prefix_error) = - target.strip_prefix(context.absolute_package_dir) - { + if let Err(_prefix_error) = target.strip_prefix(absolute_package_dir) { NixpkgsProblem::OutsidePathReference { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), line, text, @@ -169,7 +161,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Resul } } Err(e) => NixpkgsProblem::UnresolvablePathReference { - relative_package_dir: context.relative_package_dir.clone(), + relative_package_dir: relative_package_dir.to_path_buf(), subpath: subpath.to_path_buf(), line, text, From 1194ff06b0d94851b20b724281963c544754177e Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Thu, 26 Oct 2023 13:40:27 +0200 Subject: [PATCH 33/52] python311Packages.azure-mgmt-netapp: 10.1.0 -> 11.0.0 Changelog: https://github.com/Azure/azure-sdk-for-python/blob/azure-mgmt-netapp_11.0.0/sdk/netapp/azure-mgmt-netapp/CHANGELOG.md --- .../python-modules/azure-mgmt-netapp/default.nix | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkgs/development/python-modules/azure-mgmt-netapp/default.nix b/pkgs/development/python-modules/azure-mgmt-netapp/default.nix index 46061f5a0028..8b5ae2435d0d 100644 --- a/pkgs/development/python-modules/azure-mgmt-netapp/default.nix +++ b/pkgs/development/python-modules/azure-mgmt-netapp/default.nix @@ -4,28 +4,25 @@ , pythonOlder , azure-common , azure-mgmt-core -, msrest -, msrestazure +, isodate }: buildPythonPackage rec { pname = "azure-mgmt-netapp"; - version = "10.1.0"; + version = "11.0.0"; format = "setuptools"; - disabled = pythonOlder "3.7"; + disabled = pythonOlder "3.8"; src = fetchPypi { inherit pname version; - hash = "sha256-eJiWTOCk2C79Jotku9bKlu3vU6H8004hWrX+h76MjQM="; - extension = "zip"; + hash = "sha256-00cDFHpaEciRQLHM+Kt3uOtw/geOn5+onrY7lav6EeU="; }; propagatedBuildInputs = [ azure-common azure-mgmt-core - msrest - msrestazure + isodate ]; # no tests included @@ -39,6 +36,7 @@ buildPythonPackage rec { meta = with lib; { description = "Microsoft Azure NetApp Files Management Client Library for Python"; homepage = "https://github.com/Azure/azure-sdk-for-python"; + changelog = "https://github.com/Azure/azure-sdk-for-python/blob/azure-mgmt-netapp_${version}/sdk/netapp/azure-mgmt-netapp/CHANGELOG.md"; license = licenses.mit; maintainers = with maintainers; [ jonringer ]; }; From 42782fdf6a9dc7e0dd11f9d97deec4a3340034d4 Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Thu, 26 Oct 2023 14:21:48 +0200 Subject: [PATCH 34/52] python311Packages.azure-mgmt-containerregistry: 10.1.0 -> 10.2.0 Changelog: https://github.com/Azure/azure-sdk-for-python/blob/azure-mgmt-containerregistry_10.2.0/sdk/containerregistry/azure-mgmt-containerregistry/CHANGELOG.md --- .../azure-mgmt-containerregistry/default.nix | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pkgs/development/python-modules/azure-mgmt-containerregistry/default.nix b/pkgs/development/python-modules/azure-mgmt-containerregistry/default.nix index f75b262df3a0..bc20dcdfb602 100644 --- a/pkgs/development/python-modules/azure-mgmt-containerregistry/default.nix +++ b/pkgs/development/python-modules/azure-mgmt-containerregistry/default.nix @@ -4,29 +4,25 @@ , pythonOlder , azure-common , azure-mgmt-core -, msrest -, typing-extensions +, isodate }: buildPythonPackage rec { pname = "azure-mgmt-containerregistry"; - version = "10.1.0"; + version = "10.2.0"; format = "setuptools"; - disabled = pythonOlder "3.7"; + disabled = pythonOlder "3.8"; src = fetchPypi { inherit pname version; - hash = "sha256-VrX9YfYNvlA8+eNqHCp35BAeQZzQKakZs7ZZKwT8oYc="; - extension = "zip"; + hash = "sha256-i7i/5ofGxiF9/wTAPnUOaZ6FAgK3EaBqoHeSC8HuXCo="; }; propagatedBuildInputs = [ azure-common azure-mgmt-core - msrest - ] ++ lib.optionals (pythonOlder "3.8") [ - typing-extensions + isodate ]; # no tests included @@ -40,6 +36,7 @@ buildPythonPackage rec { meta = with lib; { description = "Microsoft Azure Container Registry Client Library for Python"; homepage = "https://github.com/Azure/azure-sdk-for-python"; + changelog = "https://github.com/Azure/azure-sdk-for-python/blob/azure-mgmt-containerregistry_${version}/sdk/containerregistry/azure-mgmt-containerregistry/CHANGELOG.md"; license = licenses.mit; maintainers = with maintainers; [ jonringer ]; }; From 11c482767a6849fdfdd09ca095bc180cc52ae286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Romildo?= Date: Fri, 27 Oct 2023 18:31:16 -0300 Subject: [PATCH 35/52] nordic: make sddm a separate output It brings some kde stuff into the environment, and that may not be desirable when one does not need it. --- pkgs/data/themes/nordic/default.nix | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/pkgs/data/themes/nordic/default.nix b/pkgs/data/themes/nordic/default.nix index a64b870d12b9..16eefee7bbb9 100644 --- a/pkgs/data/themes/nordic/default.nix +++ b/pkgs/data/themes/nordic/default.nix @@ -80,14 +80,11 @@ stdenv.mkDerivation rec { sourceRoot = "."; + outputs = [ "out" "sddm" ]; + nativeBuildInputs = [ jdupes ]; - propagatedUserEnvPkgs = [ - gtk-engine-murrine - breeze-icons - plasma-framework - plasma-workspace - ]; + propagatedUserEnvPkgs = [ gtk-engine-murrine ]; dontWrapQtApps = true; @@ -119,15 +116,18 @@ stdenv.mkDerivation rec { rmdir $out/share/themes/Nordic/extras{/wallpapers,} # move kde related contents to appropriate directories - mkdir -p $out/share/{aurorae/themes,color-schemes,Kvantum,plasma,sddm/themes,icons} + mkdir -p $out/share/{aurorae/themes,color-schemes,Kvantum,plasma,icons} mv -v $out/share/themes/Nordic/kde/aurorae/* $out/share/aurorae/themes/ mv -v $out/share/themes/Nordic/kde/colorschemes/* $out/share/color-schemes/ mv -v $out/share/themes/Nordic/kde/konsole $out/share/ mv -v $out/share/themes/Nordic/kde/kvantum/* $out/share/Kvantum/ mv -v $out/share/themes/Nordic/kde/plasma/look-and-feel $out/share/plasma/ - mv -v $out/share/themes/Nordic/kde/sddm/* $out/share/sddm/themes/ mv -v $out/share/themes/Nordic/kde/folders/* $out/share/icons/ mv -v $out/share/themes/Nordic/kde/cursors/*-cursors $out/share/icons/ + + mkdir -p $sddm/share/sddm/themes + mv -v $out/share/themes/Nordic/kde/sddm/* $sddm/share/sddm/themes/ + rm -rf $out/share/themes/Nordic/kde # Replace duplicate files with symbolic links to the first file in @@ -137,6 +137,16 @@ stdenv.mkDerivation rec { runHook postInstall ''; + postFixup = '' + # Propagate sddm theme dependencies to user env otherwise sddm + # does find them. Putting them in buildInputs is not enough. + + mkdir -p $sddm/nix-support + + printWords ${breeze-icons} ${plasma-framework} ${plasma-workspace} \ + >> $sddm/nix-support/propagated-user-env-packages + ''; + meta = with lib; { description = "Gtk and KDE themes using the Nord color pallete"; homepage = "https://github.com/EliverLara/Nordic"; From b553f3a109eac0b79c34dac04637a4d944c9be2d Mon Sep 17 00:00:00 2001 From: Florian Brandes Date: Sun, 22 Oct 2023 11:29:13 +0200 Subject: [PATCH 36/52] pgadmin4: fix build due to Flask update Also updates security-flask-too from 5.3.0 -> 5.3.2 Signed-off-by: Florian Brandes --- .../flask-security-too/default.nix | 24 ++++---- pkgs/tools/admin/pgadmin/default.nix | 57 ++++++++++++++++++- 2 files changed, 67 insertions(+), 14 deletions(-) diff --git a/pkgs/development/python-modules/flask-security-too/default.nix b/pkgs/development/python-modules/flask-security-too/default.nix index 0eae72147262..81abf369a8a4 100644 --- a/pkgs/development/python-modules/flask-security-too/default.nix +++ b/pkgs/development/python-modules/flask-security-too/default.nix @@ -2,6 +2,7 @@ , buildPythonPackage , fetchPypi , pythonOlder +, setuptools # extras: babel , babel @@ -11,7 +12,6 @@ , bcrypt , bleach , flask-mailman -, qrcode # extras: fsqla , flask-sqlalchemy @@ -22,17 +22,17 @@ , cryptography , phonenumbers , webauthn +, qrcode # propagates -, blinker , email-validator , flask , flask-login , flask-principal , flask-wtf -, itsdangerous , passlib , importlib-resources +, wtforms # tests , argon2-cffi @@ -47,32 +47,30 @@ buildPythonPackage rec { pname = "flask-security-too"; - version = "5.3.0"; - format = "pyproject"; + version = "5.3.2"; + pyproject = true; disabled = pythonOlder "3.7"; src = fetchPypi { pname = "Flask-Security-Too"; inherit version; - hash = "sha256-n12DCRPqxm8YhFeVrl99BEvdDYNq6rzP662rain3k1Q="; + hash = "sha256-wLUHXfDWSp7zWwTIjTH79AWlkkNzb21tChpLSEWr8+U="; }; - postPatch = '' - # This should be removed after updating to version 5.3.0. - sed -i '/filterwarnings =/a ignore:pkg_resources is deprecated:DeprecationWarning' pytest.ini - ''; + nativeBuildInputs = [ + setuptools + ]; propagatedBuildInputs = [ - blinker email-validator flask flask-login flask-principal flask-wtf - itsdangerous passlib importlib-resources + wtforms ]; passthru.optional-dependencies = { @@ -84,7 +82,6 @@ buildPythonPackage rec { bcrypt bleach flask-mailman - qrcode ]; fsqla = [ flask-sqlalchemy @@ -95,6 +92,7 @@ buildPythonPackage rec { cryptography phonenumbers webauthn + qrcode ]; }; diff --git a/pkgs/tools/admin/pgadmin/default.nix b/pkgs/tools/admin/pgadmin/default.nix index beecd6412bcf..117f02a593ba 100644 --- a/pkgs/tools/admin/pgadmin/default.nix +++ b/pkgs/tools/admin/pgadmin/default.nix @@ -9,6 +9,7 @@ , yarn , fixup_yarn_lock , nodejs +, fetchpatch , server-mode ? true }: @@ -26,7 +27,61 @@ let # keep the scope, as it is used throughout the derivation and tests # this also makes potential future overrides easier - pythonPackages = python3.pkgs.overrideScope (final: prev: rec { }); + pythonPackages = python3.pkgs.overrideScope (final: prev: rec { + # pgadmin 7.8 is incompatible with Flask >= 2.3 + flask = prev.flask.overridePythonAttrs (oldAttrs: rec { + version = "2.2.5"; + src = oldAttrs.src.override { + pname = "Flask"; + inherit version; + hash = "sha256-7e6bCn/yZiG9WowQ/0hK4oc3okENmbC7mmhQx/uXeqA="; + }; + format = "setuptools"; + }); + # downgrade needed for older Flask + httpbin = prev.httpbin.overridePythonAttrs (oldAttrs: rec { + version = "0.7.0"; + src = oldAttrs.src.override { + inherit version; + hash = "sha256-y7N3kMkVdfTxV1f0KtQdn3KesifV7b6J5OwXVIbbjfo="; + }; + format = "setuptools"; + patches = [ + (fetchpatch { + # Replaces BaseResponse class with Response class for Werkezug 2.1.0 compatibility + # https://github.com/postmanlabs/httpbin/pull/674 + url = "https://github.com/postmanlabs/httpbin/commit/5cc81ce87a3c447a127e4a1a707faf9f3b1c9b6b.patch"; + hash = "sha256-SbEWjiqayMFYrbgAPZtSsXqSyCDUz3z127XgcKOcrkE="; + }) + ]; + pytestFlagsArray = [ + "test_httpbin.py" + ]; + propagatedBuildInputs = oldAttrs.propagatedBuildInputs ++ [ final.pythonPackages.brotlipy ]; + }); + # downgrade needed for older httpbin + werkzeug = prev.werkzeug.overridePythonAttrs (oldAttrs: rec { + version = "2.2.3"; + format = "setuptools"; + src = oldAttrs.src.override { + pname = "Werkzeug"; + inherit version; + hash = "sha256-LhzMlBfU2jWLnebxdOOsCUOR6h1PvvLWZ4ZdgZ39Cv4="; + }; + }); + # Downgrade needed for older Flask + flask-security-too = prev.flask-security-too.overridePythonAttrs (oldAttrs: rec { + version = "5.1.2"; + src = oldAttrs.src.override { + inherit version; + hash = "sha256-lZzm43m30y+2qjxNddFEeg9HDlQP9afq5VtuR25zaLc="; + }; + postPatch = '' + # This should be removed after updating to version 5.3.0. + sed -i '/filterwarnings =/a ignore:pkg_resources is deprecated:DeprecationWarning' pytest.ini + ''; + }); + }); offlineCache = fetchYarnDeps { yarnLock = ./yarn.lock; From c3a57194282506295398036a4abd504bd4d19527 Mon Sep 17 00:00:00 2001 From: Atemu Date: Sat, 28 Oct 2023 19:39:46 +0200 Subject: [PATCH 37/52] linux_xanmod: 6.1.58 -> 6.1.60 --- pkgs/os-specific/linux/kernel/xanmod-kernels.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/os-specific/linux/kernel/xanmod-kernels.nix b/pkgs/os-specific/linux/kernel/xanmod-kernels.nix index 336c8c7fc325..7a4a618a8d61 100644 --- a/pkgs/os-specific/linux/kernel/xanmod-kernels.nix +++ b/pkgs/os-specific/linux/kernel/xanmod-kernels.nix @@ -3,8 +3,8 @@ let # These names are how they are designated in https://xanmod.org. ltsVariant = { - version = "6.1.58"; - hash = "sha256-Lnp1CSh1jLbIkEx9hLfxhdIA12iQZmywhOec9uZ7UjI="; + version = "6.1.60"; + hash = "sha256-KYCeONJxyFPee4pvBLRw/MBTzPU7D2oZCrAVr3t/yPM="; variant = "lts"; }; From c3bf00cd574e5856a29fae15b1b4eacf1d3e397a Mon Sep 17 00:00:00 2001 From: Atemu Date: Sat, 28 Oct 2023 19:55:03 +0200 Subject: [PATCH 38/52] linux_xanmod_latest: 6.5.8 -> 6.5.9 --- pkgs/os-specific/linux/kernel/xanmod-kernels.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/os-specific/linux/kernel/xanmod-kernels.nix b/pkgs/os-specific/linux/kernel/xanmod-kernels.nix index 7a4a618a8d61..43969919fb0f 100644 --- a/pkgs/os-specific/linux/kernel/xanmod-kernels.nix +++ b/pkgs/os-specific/linux/kernel/xanmod-kernels.nix @@ -9,8 +9,8 @@ let }; mainVariant = { - version = "6.5.8"; - hash = "sha256-lHi+O7RE6YdiqPmuxHajGkc7jS9F5cB89+JbTVKkB/c="; + version = "6.5.9"; + hash = "sha256-5SFPBsDTmq7tA6pyM7rbIjBPAtPbqhUl6VfA2z5baPA="; variant = "main"; }; From eb42414431d5f61ec18f779e5d2eaaeff7ebfafb Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Sun, 29 Oct 2023 08:01:07 +0100 Subject: [PATCH 39/52] python311Packages.withings-sync: init at 4.2.1 Synchronisation of Withings weight https://github.com/jaroslawhartman/withings-sync --- .../python-modules/withings-sync/default.nix | 48 +++++++++++++++++++ pkgs/top-level/python-packages.nix | 2 + 2 files changed, 50 insertions(+) create mode 100644 pkgs/development/python-modules/withings-sync/default.nix diff --git a/pkgs/development/python-modules/withings-sync/default.nix b/pkgs/development/python-modules/withings-sync/default.nix new file mode 100644 index 000000000000..60cce387fa62 --- /dev/null +++ b/pkgs/development/python-modules/withings-sync/default.nix @@ -0,0 +1,48 @@ +{ lib +, buildPythonPackage +, fetchFromGitHub +, garth +, lxml +, pythonOlder +, requests +, setuptools +, wheel +}: + +buildPythonPackage rec { + pname = "withings-sync"; + version = "4.2.1"; + pyproject = true; + + disabled = pythonOlder "3.10"; + + src = fetchFromGitHub { + owner = "jaroslawhartman"; + repo = "withings-sync"; + rev = "refs/tags/v${version}"; + hash = "sha256-6igjUmgIA077/1SQMt10tRpnLVKxGFNJN1GeLhQLROg="; + }; + + nativeBuildInputs = [ + setuptools + wheel + ]; + + propagatedBuildInputs = [ + garth + lxml + requests + ]; + + pythonImportsCheck = [ + "withings_sync" + ]; + + meta = with lib; { + description = "Synchronisation of Withings weight"; + homepage = "https://github.com/jaroslawhartman/withings-sync"; + changelog = "https://github.com/jaroslawhartman/withings-sync/releases/tag/v${version}"; + license = licenses.mit; + maintainers = with maintainers; [ fab ]; + }; +} diff --git a/pkgs/top-level/python-packages.nix b/pkgs/top-level/python-packages.nix index 73bd3bb17c1f..857327a50007 100644 --- a/pkgs/top-level/python-packages.nix +++ b/pkgs/top-level/python-packages.nix @@ -15664,6 +15664,8 @@ self: super: with self; { withings-api = callPackage ../development/python-modules/withings-api { }; + withings-sync = callPackage ../development/python-modules/withings-sync { }; + wktutils = callPackage ../development/python-modules/wktutils { }; wled = callPackage ../development/python-modules/wled { }; From 8314e5baa6dc32641b38773118a834f228ff4b85 Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Sun, 29 Oct 2023 08:03:02 +0100 Subject: [PATCH 40/52] python311Packages.garminconnect: 0.2.8 -> 0.2.9 Diff: https://github.com/cyberjunky/python-garminconnect/compare/refs/tags/0.2.8...0.2.9 Changelog: https://github.com/cyberjunky/python-garminconnect/releases/tag/0.2.9 --- .../python-modules/garminconnect/default.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/development/python-modules/garminconnect/default.nix b/pkgs/development/python-modules/garminconnect/default.nix index aff899a18878..423cfd34a70f 100644 --- a/pkgs/development/python-modules/garminconnect/default.nix +++ b/pkgs/development/python-modules/garminconnect/default.nix @@ -1,25 +1,25 @@ { lib , buildPythonPackage -, cloudscraper , fetchFromGitHub , garth , pdm-backend , pythonOlder , requests +, withings-sync }: buildPythonPackage rec { pname = "garminconnect"; - version = "0.2.8"; + version = "0.2.9"; format = "pyproject"; - disabled = pythonOlder "3.7"; + disabled = pythonOlder "3.10"; src = fetchFromGitHub { owner = "cyberjunky"; repo = "python-garminconnect"; rev = "refs/tags/${version}"; - hash = "sha256-jNDFSA6Mz0+7UhEVrCKcKDEX3B7yk6igBf59A6YlW2M="; + hash = "sha256-wQWOksI0nfzIMdxgZehMmNytuXWD22GLUNoI7Ki0C3s="; }; nativeBuildInputs = [ @@ -27,9 +27,9 @@ buildPythonPackage rec { ]; propagatedBuildInputs = [ - cloudscraper garth requests + withings-sync ]; # Tests require a token From 9ea4504f6b746d40690f76ef1997fa7ddd8eec20 Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Sun, 29 Oct 2023 08:11:53 +0100 Subject: [PATCH 41/52] python311Packages.google-cloud-vision: 3.4.4 -> 3.4.5 Changelog: https://github.com/googleapis/python-vision/blob/v3.4.5/CHANGELOG.md --- .../python-modules/google-cloud-vision/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/development/python-modules/google-cloud-vision/default.nix b/pkgs/development/python-modules/google-cloud-vision/default.nix index 91d97e68b096..95c2ed0662ca 100644 --- a/pkgs/development/python-modules/google-cloud-vision/default.nix +++ b/pkgs/development/python-modules/google-cloud-vision/default.nix @@ -12,14 +12,14 @@ buildPythonPackage rec { pname = "google-cloud-vision"; - version = "3.4.4"; + version = "3.4.5"; format = "setuptools"; disabled = pythonOlder "3.7"; src = fetchPypi { inherit pname version; - hash = "sha256-QFdErlCFIDTMR7MqmxuuUNP7Cc0eIWABQYKJHvV2ZpU="; + hash = "sha256-DfgkGrJ3GZuRnKODen3oUFk2P+oOPWYAYIcL587/wEc="; }; propagatedBuildInputs = [ From 55d04ca0df7485ec681f95236b8c9c758e436729 Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Sun, 29 Oct 2023 16:11:22 +0100 Subject: [PATCH 42/52] python311Packages.hap-python: 4.9.0 -> 4.9.1 Diff: https://github.com/ikalchev/HAP-python/compare/refs/tags/4.9.0...4.9.1 Changelog: https://github.com/ikalchev/HAP-python/blob/4.9.1/CHANGELOG.md --- pkgs/development/python-modules/hap-python/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/development/python-modules/hap-python/default.nix b/pkgs/development/python-modules/hap-python/default.nix index cb7e154887e0..0bb7f35e288f 100644 --- a/pkgs/development/python-modules/hap-python/default.nix +++ b/pkgs/development/python-modules/hap-python/default.nix @@ -17,7 +17,7 @@ buildPythonPackage rec { pname = "hap-python"; - version = "4.9.0"; + version = "4.9.1"; format = "setuptools"; disabled = pythonOlder "3.6"; @@ -26,7 +26,7 @@ buildPythonPackage rec { owner = "ikalchev"; repo = "HAP-python"; rev = "refs/tags/${version}"; - hash = "sha256-bFSqMAZWE3xTfnc7FSQMfAhxhKlYm65VFpm+q3yrqpE="; + hash = "sha256-nnh8PSEcuPN1qGuInJ7uYe83zdne8axbTrHd4g1xoJs="; }; propagatedBuildInputs = [ From a771cbe75e0122bb8bd2da5156ff6837b2fa2872 Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Sun, 29 Oct 2023 19:55:45 +0100 Subject: [PATCH 43/52] python311Packages.peaqevcore: 19.5.10 -> 19.5.12 Changelog: https://github.com/elden1337/peaqev-core/releases/tag/19.5.12 --- pkgs/development/python-modules/peaqevcore/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/development/python-modules/peaqevcore/default.nix b/pkgs/development/python-modules/peaqevcore/default.nix index 9fe9539bf810..cc2d71914dc6 100644 --- a/pkgs/development/python-modules/peaqevcore/default.nix +++ b/pkgs/development/python-modules/peaqevcore/default.nix @@ -6,14 +6,14 @@ buildPythonPackage rec { pname = "peaqevcore"; - version = "19.5.10"; + version = "19.5.12"; format = "setuptools"; disabled = pythonOlder "3.7"; src = fetchPypi { inherit pname version; - hash = "sha256-izw41TUmqKOy34/RMHjBROQr88SChheKJVpPMaOubnE="; + hash = "sha256-NsQrfJQ1+WZ4wNBH8ZGGo9IMJ+yvWrVQmesDBQrfRKg="; }; postPatch = '' From 1152e785222f616929a181380d9e04f80c2299c6 Mon Sep 17 00:00:00 2001 From: rnhmjoj Date: Mon, 30 Oct 2023 01:04:03 +0100 Subject: [PATCH 44/52] cie-middleware-linux: 1.4.4.0 -> 1.5.0 --- .../security/cie-middleware-linux/default.nix | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkgs/tools/security/cie-middleware-linux/default.nix b/pkgs/tools/security/cie-middleware-linux/default.nix index 7af3e9240f1e..fa5ec2d2af83 100644 --- a/pkgs/tools/security/cie-middleware-linux/default.nix +++ b/pkgs/tools/security/cie-middleware-linux/default.nix @@ -20,13 +20,13 @@ let pname = "cie-middleware-linux"; - version = "1.4.4.0"; + version = "1.5.0"; src = fetchFromGitHub { owner = "M0rf30"; repo = pname; - rev = "${version}-podofo"; - sha256 = "sha256-Kyr9OTiY6roJ/wVJS/1aWfrrzDNQbuRTJQqo0akbMUU="; + rev = version; + sha256 = "sha256-Z8K2Ibg5bBfSql5HEapKgdfiCf/EIKTTD15oVeysQGk="; }; gradle = gradle_7; @@ -44,6 +44,7 @@ let buildPhase = '' # Run the fetchDeps task export GRADLE_USER_HOME=$(mktemp -d) + ls -l gradle --no-daemon -b cie-java/build.gradle fetchDeps ''; @@ -60,7 +61,7 @@ let outputHashAlgo = "sha256"; outputHashMode = "recursive"; - outputHash = "sha256-WzT5vYF9yCMU2A7EkLZyjgWrN3gD7pnkPXc3hDFqpD8="; + outputHash = "sha256-jtaH8dBpnx8KMJe+jzJfkvcx1NO4nL5jsRO4+GI+d0c="; }; in @@ -84,7 +85,7 @@ stdenv.mkDerivation { buildInputs = [ cryptopp fontconfig - podofo + podofo.dev openssl pcsclite curl @@ -95,6 +96,10 @@ stdenv.mkDerivation { # substitute the cieid command with this $out/bin/cieid substituteInPlace libs/pkcs11/src/CSP/AbilitaCIE.cpp \ --replace 'file = "cieid"' 'file = "'$out'/bin/cieid"' + + # revert https://github.com/M0Rf30/cie-middleware-linux/commit/1a389d8 + sed -i libs/meson.build \ + -e "s@podofo_dep = .\+@podofo_dep = dependency('libpodofo')@g" ''; # Note: we use pushd/popd to juggle between the From 9c8d9a7af3ae0f2e4abc0851d5938e0c7847a781 Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Mon, 30 Oct 2023 07:58:50 +0100 Subject: [PATCH 45/52] python311Packages.publicsuffixlist: 0.10.0.20231026 -> 0.10.0.20231030 --- pkgs/development/python-modules/publicsuffixlist/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/development/python-modules/publicsuffixlist/default.nix b/pkgs/development/python-modules/publicsuffixlist/default.nix index 12e67d554eb4..44648cbbff72 100644 --- a/pkgs/development/python-modules/publicsuffixlist/default.nix +++ b/pkgs/development/python-modules/publicsuffixlist/default.nix @@ -9,14 +9,14 @@ buildPythonPackage rec { pname = "publicsuffixlist"; - version = "0.10.0.20231026"; + version = "0.10.0.20231030"; format = "setuptools"; disabled = pythonOlder "3.7"; src = fetchPypi { inherit pname version; - hash = "sha256-q2rUBjbue3I3VnRLTF7UscBs51bGxUGjMYwAkgX5UMs="; + hash = "sha256-1yRv6zg9mKJTinR57QHvCx/0mi0b2O3CkcoH1v4QuNo="; }; passthru.optional-dependencies = { From 0532c14154f0e4989546516adfac3099dce9fe2a Mon Sep 17 00:00:00 2001 From: Fabian Affolter Date: Mon, 30 Oct 2023 08:02:39 +0100 Subject: [PATCH 46/52] python311Packages.publicsuffixlist: switch to pyproject --- .../python-modules/publicsuffixlist/default.nix | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkgs/development/python-modules/publicsuffixlist/default.nix b/pkgs/development/python-modules/publicsuffixlist/default.nix index 44648cbbff72..00edd1433813 100644 --- a/pkgs/development/python-modules/publicsuffixlist/default.nix +++ b/pkgs/development/python-modules/publicsuffixlist/default.nix @@ -5,12 +5,13 @@ , pytestCheckHook , pythonOlder , requests +, setuptools }: buildPythonPackage rec { pname = "publicsuffixlist"; version = "0.10.0.20231030"; - format = "setuptools"; + pyproject = true; disabled = pythonOlder "3.7"; @@ -19,6 +20,10 @@ buildPythonPackage rec { hash = "sha256-1yRv6zg9mKJTinR57QHvCx/0mi0b2O3CkcoH1v4QuNo="; }; + nativeBuildInputs = [ + setuptools + ]; + passthru.optional-dependencies = { update = [ requests From 419eba9ab1a86c5fb01cee802ee47120c8efa78f Mon Sep 17 00:00:00 2001 From: K900 Date: Mon, 30 Oct 2023 10:46:11 +0300 Subject: [PATCH 47/52] linux_6_6: init at 6.6 --- pkgs/os-specific/linux/kernel/kernels-org.json | 4 ++++ pkgs/top-level/aliases.nix | 2 ++ pkgs/top-level/linux-kernels.nix | 11 ++++++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pkgs/os-specific/linux/kernel/kernels-org.json b/pkgs/os-specific/linux/kernel/kernels-org.json index 926a172a7240..ab2775ab922d 100644 --- a/pkgs/os-specific/linux/kernel/kernels-org.json +++ b/pkgs/os-specific/linux/kernel/kernels-org.json @@ -34,5 +34,9 @@ "4.14": { "version": "4.14.328", "hash": "sha256:1igcpvnhwwrczfdsafmszvi0456k7f6j4cgpfw6v6afw09p95d8x" + }, + "6.6": { + "version": "6.6", + "hash": "sha256:1l2nisx9lf2vdgkq910n5ldbi8z25ky1zvl67zgwg2nxcdna09nr" } } diff --git a/pkgs/top-level/aliases.nix b/pkgs/top-level/aliases.nix index a6986c88419f..85ac74fae7db 100644 --- a/pkgs/top-level/aliases.nix +++ b/pkgs/top-level/aliases.nix @@ -493,6 +493,7 @@ mapAliases ({ linuxPackages_6_3 = linuxKernel.packages.linux_6_3; linuxPackages_6_4 = linuxKernel.packages.linux_6_4; linuxPackages_6_5 = linuxKernel.packages.linux_6_5; + linuxPackages_6_6 = linuxKernel.packages.linux_6_6; linuxPackages_rpi0 = linuxKernel.packages.linux_rpi1; linuxPackages_rpi02w = linuxKernel.packages.linux_rpi3; linuxPackages_rpi1 = linuxKernel.packages.linux_rpi1; @@ -517,6 +518,7 @@ mapAliases ({ linux_6_3 = linuxKernel.kernels.linux_6_3; linux_6_4 = linuxKernel.kernels.linux_6_4; linux_6_5 = linuxKernel.kernels.linux_6_5; + linux_6_6 = linuxKernel.kernels.linux_6_6; linux_rpi0 = linuxKernel.kernels.linux_rpi1; linux_rpi02w = linuxKernel.kernels.linux_rpi3; linux_rpi1 = linuxKernel.kernels.linux_rpi1; diff --git a/pkgs/top-level/linux-kernels.nix b/pkgs/top-level/linux-kernels.nix index d796fa7164d0..e4b1133109f9 100644 --- a/pkgs/top-level/linux-kernels.nix +++ b/pkgs/top-level/linux-kernels.nix @@ -178,6 +178,14 @@ in { ]; }; + linux_6_6 = callPackage ../os-specific/linux/kernel/mainline.nix { + branch = "6.6"; + kernelPatches = [ + kernelPatches.bridge_stp_helper + kernelPatches.request_key_helper + ]; + }; + linux_testing = let testing = callPackage ../os-specific/linux/kernel/mainline.nix { # A special branch that tracks the kernel under the release process @@ -573,6 +581,7 @@ in { linux_5_15 = recurseIntoAttrs (packagesFor kernels.linux_5_15); linux_6_1 = recurseIntoAttrs (packagesFor kernels.linux_6_1); linux_6_5 = recurseIntoAttrs (packagesFor kernels.linux_6_5); + linux_6_6 = recurseIntoAttrs (packagesFor kernels.linux_6_6); } // lib.optionalAttrs config.allowAliases { linux_4_9 = throw "linux 4.9 was removed because it will reach its end of life within 22.11"; # Added 2022-11-08 linux_4_14 = throw "linux 4.14 was removed because it will reach its end of life within 23.11"; # Added 2023-10-11 @@ -633,7 +642,7 @@ in { packageAliases = { linux_default = packages.linux_6_1; # Update this when adding the newest kernel major version! - linux_latest = packages.linux_6_5; + linux_latest = packages.linux_6_6; linux_mptcp = throw "'linux_mptcp' has been moved to https://github.com/teto/mptcp-flake"; linux_rt_default = packages.linux_rt_5_4; linux_rt_latest = packages.linux_rt_6_1; From be33098cfffff918ac527888058436ee193b6cd6 Mon Sep 17 00:00:00 2001 From: K900 Date: Mon, 30 Oct 2023 11:11:15 +0300 Subject: [PATCH 48/52] linux/common-config: enable new security features for 6.6 --- pkgs/os-specific/linux/kernel/common-config.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/os-specific/linux/kernel/common-config.nix b/pkgs/os-specific/linux/kernel/common-config.nix index ceb34fe0c76f..0f59d3ac7aad 100644 --- a/pkgs/os-specific/linux/kernel/common-config.nix +++ b/pkgs/os-specific/linux/kernel/common-config.nix @@ -558,6 +558,8 @@ let PERSISTENT_KEYRINGS = yes; # enable temporary caching of the last request_key() result KEYS_REQUEST_CACHE = whenAtLeast "5.3" yes; + # randomized slab caches + RANDOM_KMALLOC_CACHES = whenAtLeast "6.6" yes; } // optionalAttrs stdenv.hostPlatform.isx86_64 { # Enable Intel SGX X86_SGX = whenAtLeast "5.11" yes; @@ -572,6 +574,8 @@ let KVM_AMD_SEV = yes; # AMD SEV-SNP SEV_GUEST = whenAtLeast "5.19" module; + # Shadow stacks + X86_USER_SHADOW_STACK = whenAtLeast "6.6" yes; }; microcode = { From ed449a1f058053e5926e29033f868129730b229b Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Sat, 28 Oct 2023 13:25:03 +0200 Subject: [PATCH 49/52] pr-tracker: fetchurl -> fetchzip The tarballs are dynamically generated by cgit, so we should use fetchzip (which extracts the tarball) to ensure determinism. --- pkgs/servers/pr-tracker/default.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/servers/pr-tracker/default.nix b/pkgs/servers/pr-tracker/default.nix index 33ce80cdaaf5..41828e209bc0 100644 --- a/pkgs/servers/pr-tracker/default.nix +++ b/pkgs/servers/pr-tracker/default.nix @@ -1,6 +1,6 @@ { rustPlatform , lib -, fetchurl +, fetchzip , openssl , pkg-config , systemd @@ -10,9 +10,9 @@ rustPlatform.buildRustPackage rec { pname = "pr-tracker"; version = "1.2.0"; - src = fetchurl { + src = fetchzip { url = "https://git.qyliss.net/pr-tracker/snapshot/pr-tracker-${version}.tar.xz"; - sha256 = "sha256-Tru9DsitRQLiO4Ln70J9LvkEqcj2i4A+eArBvIhd/ls="; + hash = "sha256-fUEmxD50Ymnql5vnDt8DUlIztAJ9XNeKxA+FLY68Fkw="; }; cargoSha256 = "0q3ibxnzw8gngvrgfkv4m64dr411c511xkvb6j9k63vhy9vwarz7"; From 423b31f1b24ec8d82baec9a5bb969da892010e6d Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Sat, 28 Oct 2023 13:26:56 +0200 Subject: [PATCH 50/52] pr-tracker: 1.2.0 -> 1.3.0 --- pkgs/servers/pr-tracker/default.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/servers/pr-tracker/default.nix b/pkgs/servers/pr-tracker/default.nix index 41828e209bc0..1c07a56a7e3a 100644 --- a/pkgs/servers/pr-tracker/default.nix +++ b/pkgs/servers/pr-tracker/default.nix @@ -8,14 +8,14 @@ rustPlatform.buildRustPackage rec { pname = "pr-tracker"; - version = "1.2.0"; + version = "1.3.0"; src = fetchzip { url = "https://git.qyliss.net/pr-tracker/snapshot/pr-tracker-${version}.tar.xz"; - hash = "sha256-fUEmxD50Ymnql5vnDt8DUlIztAJ9XNeKxA+FLY68Fkw="; + hash = "sha256-JetfcA7Pn6nsCxCkgxP4jS6tijx89any/0GrmLa+DR0="; }; - cargoSha256 = "0q3ibxnzw8gngvrgfkv4m64dr411c511xkvb6j9k63vhy9vwarz7"; + cargoSha256 = "sha256-QUr0IHmzbhFNd6rBDEX8RZul/d1TLv0t+ySCQYMlpmE="; nativeBuildInputs = [ pkg-config ]; buildInputs = [ openssl systemd ]; From caf70776a2b500e0e010519b1ebf136d81df130e Mon Sep 17 00:00:00 2001 From: kashw2 Date: Sun, 29 Oct 2023 16:08:26 +1000 Subject: [PATCH 51/52] qmplay2: 23.08.22 -> 23.10.22 --- pkgs/applications/video/qmplay2/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/applications/video/qmplay2/default.nix b/pkgs/applications/video/qmplay2/default.nix index e78472ee29c1..b6b577790cfb 100644 --- a/pkgs/applications/video/qmplay2/default.nix +++ b/pkgs/applications/video/qmplay2/default.nix @@ -26,14 +26,14 @@ stdenv.mkDerivation (finalAttrs: { pname = "qmplay2"; - version = "23.08.22"; + version = "23.10.22"; src = fetchFromGitHub { owner = "zaps166"; repo = "QMPlay2"; rev = finalAttrs.version; fetchSubmodules = true; - hash = "sha256-Ug7WAqZ+BxspQUXweL/OnVBGCsU60DOWNexbi0GpDo0="; + hash = "sha256-yDymUXuILgT4AFTt302GniPi/WNwrTCOuOfdUiKOIyk="; }; nativeBuildInputs = [ From 7b0a2b8d874928fb56782fb35efb8745995b90c3 Mon Sep 17 00:00:00 2001 From: "R. Ryantm" Date: Sun, 29 Oct 2023 14:53:49 +0000 Subject: [PATCH 52/52] brogue-ce: 1.12 -> 1.13 --- pkgs/games/brogue-ce/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/games/brogue-ce/default.nix b/pkgs/games/brogue-ce/default.nix index 0691d82f175a..7532be8d1957 100644 --- a/pkgs/games/brogue-ce/default.nix +++ b/pkgs/games/brogue-ce/default.nix @@ -9,13 +9,13 @@ stdenv.mkDerivation rec { pname = "brogue-ce"; - version = "1.12"; + version = "1.13"; src = fetchFromGitHub { owner = "tmewett"; repo = "BrogueCE"; rev = "v${version}"; - hash = "sha256-bGAE0hRiKBo3ikyObGxAiPRRO24KtC+upO3XLj+f4yo="; + hash = "sha256-FUIdi1Ytn+INeD9550MW41qXtLb6in0QS3Snt8QaXUA="; }; postPatch = ''