gador/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tests.nixpkgs-check-by-name: Re-use nixFileCache more

Browse Source 
Makes the reference check use the nixFileCache instead of separately
parsing and resolving paths
This commit is contained in:
Silvan Mosberger
2024-01-30 21:54:04 +01:00
parent 0bcb052284
commit db435ae516
3 changed files with 41 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkgs/test/nixpkgs-check-by-name/src/main.rs
+1 -1
View File
@@ -136,7 +136,7 @@ pub fn check_nixpkgs<W: io::Write>(
)?;
Success(ratchet::Nixpkgs::default())
} else {
check_structure(&nixpkgs_path)?.result_map(|package_names|
check_structure(&nixpkgs_path, &mut nix_file_cache)?.result_map(|package_names|
// Only if we could successfully parse the structure, we do the evaluation checks
eval::check_values(&nixpkgs_path, &mut nix_file_cache, package_names, keep_nix_path))?
}
pkgs/test/nixpkgs-check-by-name/src/references.rs
+35 -45
View File
@@ -1,23 +1,23 @@
use crate::nixpkgs_problem::NixpkgsProblem;
use crate::utils;
use crate::utils::LineIndex;
use crate::validation::{self, ResultIteratorExt, Validation::Success};
use crate::NixFileCache;
use rowan::ast::AstNode;
use anyhow::Context;
use rnix::{Root, SyntaxKind::NODE_PATH};
use std::ffi::OsStr;
use std::fs::read_to_string;
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.
pub fn check_references(
nix_file_cache: &mut NixFileCache,
relative_package_dir: &Path,
absolute_package_dir: &Path,
) -> validation::Result<()> {
// The empty argument here is the subpath under the package directory to check
// An empty one means the package directory itself
check_path(relative_package_dir, absolute_package_dir, Path::new("")).with_context(|| {
check_path(nix_file_cache, relative_package_dir, absolute_package_dir, Path::new("")).with_context(|| {
format!(
"While checking the references in package directory {}",
relative_package_dir.display()
@@ -27,6 +27,7 @@ pub fn check_references(
/// Checks for a specific path to not have references outside
fn check_path(
nix_file_cache: &mut NixFileCache,
relative_package_dir: &Path,
absolute_package_dir: &Path,
subpath: &Path,
@@ -63,7 +64,7 @@ fn check_path(
.into_iter()
.map(|entry| {
let entry_subpath = subpath.join(entry.file_name());
check_path(relative_package_dir, absolute_package_dir, &entry_subpath)
check_path(nix_file_cache, relative_package_dir, absolute_package_dir, &entry_subpath)
.with_context(|| {
format!("Error while recursing into {}", subpath.display())
})
@@ -74,7 +75,7 @@ fn check_path(
// Only check Nix files
if let Some(ext) = path.extension() {
if ext == OsStr::new("nix") {
check_nix_file(relative_package_dir, absolute_package_dir, subpath).with_context(
check_nix_file(nix_file_cache, relative_package_dir, absolute_package_dir, subpath).with_context(
|| format!("Error while checking Nix file {}", subpath.display()),
)?
} else {
@@ -92,20 +93,16 @@ fn check_path(
/// Check whether a nix file contains path expression references pointing outside the package
/// directory
fn check_nix_file(
nix_file_cache: &mut NixFileCache,
relative_package_dir: &Path,
absolute_package_dir: &Path,
subpath: &Path,
) -> validation::Result<()> {
let path = absolute_package_dir.join(subpath);
let parent_dir = path
.parent()
.with_context(|| format!("Could not get parent of path {}", subpath.display()))?;
let contents = read_to_string(&path)
.with_context(|| format!("Could not read file {}", subpath.display()))?;
let root = Root::parse(&contents);
if let Some(error) = root.errors().first() {
let nix_file = match nix_file_cache.get(&path)? {
Ok(nix_file) => nix_file,
Err(error) =>
// NOTE: There's now another Nixpkgs CI check to make sure all changed Nix files parse
// correctly, though that uses mainline Nix instead of rnix, so it doesn't give the same
// errors. In the future we should unify these two checks, ideally moving the other CI
@@ -115,20 +112,23 @@ fn check_nix_file(
subpath: subpath.to_path_buf(),
error: error.clone(),
}
.into());
}
.into())
};
let line_index = LineIndex::new(&contents);
Ok(validation::sequence_(root.syntax().descendants().map(
Ok(validation::sequence_(nix_file.syntax_root.syntax().descendants().map(
|node| {
let text = node.text().to_string();
let line = line_index.line(node.text_range().start().into());
let line = nix_file.line_index.line(node.text_range().start().into());
if node.kind() != NODE_PATH {
// We're only interested in Path expressions
Success(())
} else if node.children().count() != 0 {
let Some(path) = rnix::ast::Path::cast(node) else {
return Success(())
};
use crate::nix_file::ResolvedPath::*;
match nix_file.static_resolve_path(path, absolute_package_dir) {
Interpolated =>
// Filters out ./foo/${bar}/baz
// TODO: We can just check ./foo
NixpkgsProblem::PathInterpolation {
@@ -137,8 +137,8 @@ fn check_nix_file(
line,
text,
}
.into()
} else if text.starts_with('<') {
.into(),
SearchPath =>
// Filters out search paths like <nixpkgs>
NixpkgsProblem::SearchPath {
relative_package_dir: relative_package_dir.to_path_buf(),
@@ -146,28 +146,15 @@ fn check_nix_file(
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(absolute_package_dir) {
NixpkgsProblem::OutsidePathReference {
.into(),
Outside => NixpkgsProblem::OutsidePathReference {
relative_package_dir: relative_package_dir.to_path_buf(),
subpath: subpath.to_path_buf(),
line,
text,
}
.into()
} else {
Success(())
}
}
Err(e) => NixpkgsProblem::UnresolvablePathReference {
.into(),
Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference {
relative_package_dir: relative_package_dir.to_path_buf(),
subpath: subpath.to_path_buf(),
line,
@@ -175,8 +162,11 @@ fn check_nix_file(
io_error: e,
}
.into(),
Within(_p) =>
// No need to handle the case of it being inside the directory, since we scan through the
// entire directory recursively anyways
Success(())
}
}
},
)))
}),
))
}
pkgs/test/nixpkgs-check-by-name/src/structure.rs
+5 -2
View File
@@ -3,6 +3,7 @@ use crate::references;
use crate::utils;
use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME};
use crate::validation::{self, ResultIteratorExt, Validation::Success};
use crate::NixFileCache;
use itertools::concat;
use lazy_static::lazy_static;
use regex::Regex;
@@ -34,7 +35,7 @@ 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) -> validation::Result<Vec<String>> {
pub fn check_structure(path: &Path, nix_file_cache: &mut NixFileCache) -> validation::Result<Vec<String>> {
let base_dir = path.join(BASE_SUBPATH);
let shard_results = utils::read_dir_sorted(&base_dir)?
@@ -88,7 +89,7 @@ pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
let package_results = entries
.into_iter()
.map(|package_entry| {
check_package(path, &shard_name, shard_name_valid, package_entry)
check_package(nix_file_cache, path, &shard_name, shard_name_valid, package_entry)
})
.collect_vec()?;
@@ -102,6 +103,7 @@ pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
}
fn check_package(
nix_file_cache: &mut NixFileCache,
path: &Path,
shard_name: &str,
shard_name_valid: bool,
@@ -161,6 +163,7 @@ fn check_package(
});
let result = result.and(references::check_references(
nix_file_cache,
&relative_package_dir,
&path.join(&relative_package_dir),
)?);