Some changes, also run fmt

Author: mdaniels5757

.gitignore

@@ -1,3 +1,4 @@
 target
 .direnv
 result*
+by-name-config-generated.json

by-name-config.json

@@ -0,0 +1,22 @@
+# Configure by-name directories in this file.
+# Then build with `nix-build -A build` to automatically generate by-name-config-generated.json
+# (Or, to generate manually, `nix-instantiate --eval --json --strict by-name-config.nix > by-name-config-generated.json`)
+{
+  by_name_dirs = [
+    {
+      path = "pkgs/development/python-modules/by-name";
+      attr_path_regex = "^(python3\\d*Packages|python3\\d*.pkgs)\\..*";
+      unversioned_attr_prefix = "python3Packages";
+    }
+    {
+      path = "pkgs/development/tcl-modules/by-name";
+      attr_path_regex = "^(tcl\\d*Packages)\\..*";
+      unversioned_attr_prefix = "tclPackages";
+    }
+    {
+      path = "pkgs/by-name";
+      attr_path_regex = ".*"; # There must be exactly one wildcard. All non-wildcard regexes must be mutually exclusive.
+      unversioned_attr_prefix = ""; # Ditto for this field.
+    }
+  ];
+}

by-name-config.nix

@@ -50,6 +50,10 @@ rustPlatform.buildRustPackage {
   env.NIXPKGS_VET_NIX_PACKAGE = lib.getBin nix;
   env.NIXPKGS_VET_NIXPKGS_LIB = "${path}/lib";
 
+  postBuild = ''
+    nix-instantiate --eval --json --strict ${./by-name-config.nix} > by-name-config-generated.json
+  '';
+
   checkPhase = ''
     # This path will be symlinked to the current version that is being tested
     nixPackage=$(mktemp -d)/nix
@@ -60,8 +64,6 @@ rustPlatform.buildRustPackage {
     # This is what nixpkgs-vet uses
     export NIXPKGS_VET_NIX_PACKAGE=$nixPackage
 
-    cp ${./by-name-config.json} by-name-config.json
-
     ${lib.concatMapStringsSep "\n" (nix: ''
       ln -s ${lib.getBin nix} "$nixPackage"
       echo "Testing with $(nix --version)"

package.nix

@@ -6,15 +6,15 @@ use anyhow::Context;
 use relative_path::RelativePathBuf;
 use serde::Deserialize;
 
-use crate::NixFileStore;
 use crate::nix_file::CallPackageArgumentInfo;
 use crate::problem::{
     npv_100, npv_101, npv_102, npv_103, npv_104, npv_105, npv_106, npv_107, npv_108, npv_120,
 };
 use crate::ratchet::RatchetState::{Loose, Tight};
-use crate::structure::{self, Config};
+use crate::structure::{self, ByNameDir, Config};
 use crate::validation::ResultIteratorExt as _;
 use crate::validation::{self, Validation::Success};
+use crate::NixFileStore;
 use crate::{location, ratchet};
 
 const EVAL_NIX: &[u8] = include_bytes!("eval.nix");
@@ -159,6 +159,7 @@ pub fn check_values(
     nixpkgs_path: &Path,
     nix_file_store: &mut NixFileStore,
     package_names: &[String],
+    by_name_dir: &ByNameDir,
     config: &Config,
 ) -> validation::Result<BTreeMap<String, ratchet::Package>> {
     let work_dir = tempfile::Builder::new()
@@ -169,6 +170,8 @@ pub fn check_values(
     // Canonicalize the path so that if a symlink were returned, we wouldn't ask Nix to follow it.
     let work_dir_path = work_dir.path().canonicalize()?;
 
+    println!("package_names: {}", package_names.join(", "));
+
     // Write the list of packages we need to check into a temporary JSON file.
     let package_names_path = work_dir_path.join("package-names.json");
     let package_names_file = fs::File::create(&package_names_path)?;
@@ -224,7 +227,11 @@ pub fn check_values(
 
     if !result.status.success() {
         // Early return in case evaluation fails
-        return Ok(npv_120::NixEvalError::new(String::from_utf8_lossy(&result.stderr)).into());
+        return Ok(npv_120::NixEvalError::new(
+            String::from_utf8_lossy(&result.stderr),
+            by_name_dir.clone(),
+        )
+        .into());
     }
 
     // Parse the resulting JSON value
@@ -247,13 +254,15 @@ pub fn check_values(
                         &attribute_name,
                         non_by_name_attribute,
                         config,
+                        by_name_dir,
                     )?,
                     Attribute::ByName(by_name_attribute) => by_name(
                         nix_file_store,
                         nixpkgs_path,
                         &attribute_name,
                         by_name_attribute,
                         config,
+                        by_name_dir,
                     )?,
                 };
                 Ok::<_, anyhow::Error>(check_result.map(|value| (attribute_name.clone(), value)))
@@ -271,16 +280,18 @@ fn by_name(
     attribute_name: &str,
     by_name_attribute: ByNameAttribute,
     config: &Config,
+    by_name_dir: &ByNameDir,
 ) -> validation::Result<ratchet::Package> {
-    // At this point we know that `pkgs/by-name/fo/foo/package.nix` has to exists.  This match
+    println!("attribute_name: {attribute_name}");
+    // At this point we know that `pkgs/by-name/fo/foo/package.nix` has to exist.  This match
     // decides whether the attribute `foo` is defined accordingly and whether a legacy manual
     // definition could be removed.
     let manual_definition_result = match by_name_attribute {
         // The attribute is missing.
         ByNameAttribute::Missing => {
             // This indicates a bug in the `pkgs/by-name` overlay, because it's supposed to
             // automatically define attributes in a `by-name` directory
-            npv_100::ByNameUndefinedAttribute::new(attribute_name).into()
+            npv_100::ByNameUndefinedAttribute::new(attribute_name, by_name_dir.clone()).into()
         }
         // The attribute exists
         ByNameAttribute::Existing(AttributeInfo {
@@ -294,7 +305,7 @@ fn by_name(
             //
             // We can't know whether the attribute is automatically or manually defined for sure,
             // and while we could check the location, the error seems clear enough as is.
-            npv_101::ByNameNonDerivation::new(attribute_name).into()
+            npv_101::ByNameNonDerivation::new(attribute_name, by_name_dir.clone()).into()
         }
         // The attribute exists
         ByNameAttribute::Existing(AttributeInfo {
@@ -310,7 +321,7 @@ fn by_name(
             let is_derivation_result = if is_derivation {
                 Success(())
             } else {
-                npv_101::ByNameNonDerivation::new(attribute_name).into()
+                npv_101::ByNameNonDerivation::new(attribute_name, by_name_dir.clone()).into()
             };
 
             // If the definition looks correct
@@ -366,6 +377,7 @@ fn by_name(
                             definition,
                             location,
                             config,
+                            by_name_dir,
                         )
                     } else {
                         // If manual definitions don't have a location, it's likely `mapAttrs`'d
@@ -401,13 +413,15 @@ fn by_name_override(
     definition: String,
     location: location::Location,
     config: &Config,
+    by_name_dir: &ByNameDir,
 ) -> validation::Validation<ratchet::RatchetState<ratchet::ManualDefinition>> {
     let Some(syntactic_call_package) = optional_syntactic_call_package else {
         // Something like `<attr> = foo`
         return npv_104::ByNameOverrideOfNonSyntacticCallPackage::new(
             attribute_name,
             location,
             definition,
+            by_name_dir.clone(),
         )
         .into();
     };
@@ -418,13 +432,19 @@ fn by_name_override(
             attribute_name,
             location,
             definition,
+            by_name_dir.clone(),
         )
         .into();
     }
 
     let Some(actual_package_path) = syntactic_call_package.relative_path else {
-        return npv_108::ByNameOverrideContainsEmptyPath::new(attribute_name, location, definition)
-            .into();
+        return npv_108::ByNameOverrideContainsEmptyPath::new(
+            attribute_name,
+            location,
+            definition,
+            by_name_dir.clone(),
+        )
+        .into();
     };
 
     let expected_by_name_dir = structure::expected_by_name_dir_for_package(attribute_name, config);
@@ -436,6 +456,7 @@ fn by_name_override(
             attribute_name,
             actual_package_path,
             location,
+            by_name_dir.clone(),
         )
         .into();
     }
@@ -444,8 +465,13 @@ fn by_name_override(
     // continue to be allowed. This is the state to migrate away from.
     if syntactic_call_package.empty_arg {
         Success(Loose(
-            npv_107::ByNameOverrideContainsEmptyArgument::new(attribute_name, location, definition)
-                .into(),
+            npv_107::ByNameOverrideContainsEmptyArgument::new(
+                attribute_name,
+                location,
+                definition,
+                by_name_dir.clone(),
+            )
+            .into(),
         ))
     } else {
         // This is the state to migrate to.
@@ -461,9 +487,10 @@ fn handle_non_by_name_attribute(
     attribute_name: &str,
     non_by_name_attribute: NonByNameAttribute,
     config: &Config,
+    by_name_dir: &ByNameDir,
 ) -> validation::Result<ratchet::Package> {
-    use NonByNameAttribute::EvalSuccess;
     use ratchet::RatchetState::{Loose, NonApplicable, Tight};
+    use NonByNameAttribute::EvalSuccess;
 
     // The ratchet state whether this attribute uses a `by-name` directory
     //
@@ -573,7 +600,7 @@ fn handle_non_by_name_attribute(
                     _ => {
                         // Otherwise, the path is outside a `by-name` directory, which means it can be
                         // migrated.
-                        Loose((syntactic_call_package, location.file))
+                        Loose((syntactic_call_package, location.file, by_name_dir.to_owned()))
                     }
                 }
             }

src/eval.rs

@@ -22,7 +22,6 @@ mod validation;
 
 use anyhow::Context as _;
 use clap::Parser;
-use relative_path::{RelativePath, RelativePathBuf};
 use std::collections::BTreeMap;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
@@ -32,7 +31,7 @@ use std::{panic, thread};
 use crate::nix_file::NixFileStore;
 use crate::problem::Problem;
 use crate::status::{ColoredStatus, Status};
-use crate::structure::{Config, check_structure, read_config};
+use crate::structure::{check_structure, read_config, ByNameDir, Config};
 use crate::validation::Validation::{Failure, Success};
 
 /// Program to check the validity of pkgs/by-name
@@ -77,15 +76,16 @@ fn main() -> ExitCode {
 /// - `base_nixpkgs`: Path to the base Nixpkgs to run ratchet checks against.
 /// - `main_nixpkgs`: Path to the main Nixpkgs to check.
 fn process(base_nixpkgs: &Path, main_nixpkgs: &Path, config: &Config) -> Status {
-    let by_name_dir_paths: Vec<RelativePathBuf> =
-        config.by_name_dirs.iter().map(|x| x.path.clone()).collect();
+    let by_name_dirs: &Vec<ByNameDir> = &config.by_name_dirs;
     let mut thread_results: Vec<Status> = vec![];
     thread::scope(|s| {
         let mut threads: Vec<ScopedJoinHandle<Status>> = vec![];
-        for path in by_name_dir_paths {
-            let new_thread =
-                s.spawn(move || process_by_name_dir(base_nixpkgs, main_nixpkgs, &path, config));
-            threads.push(new_thread);
+        for dir in by_name_dirs {
+            if dir.path != "pkgs/by-name" {
+                let new_thread =
+                    s.spawn(move || process_by_name_dir(base_nixpkgs, main_nixpkgs, dir, config));
+                threads.push(new_thread);
+            }
         }
         for thread in threads {
             thread_results.push(thread.join().unwrap())
@@ -158,7 +158,7 @@ fn process(base_nixpkgs: &Path, main_nixpkgs: &Path, config: &Config) -> Status
 fn process_by_name_dir(
     base_nixpkgs: &Path,
     main_nixpkgs: &Path,
-    by_name_dir: &RelativePath,
+    by_name_dir: &ByNameDir,
     config: &Config,
 ) -> Status {
     let (main_result, base_result) = thread::scope(|s| {
@@ -206,7 +206,7 @@ fn process_by_name_dir(
 /// ratchet check against another result.
 fn check_nixpkgs(
     nixpkgs_path: &Path,
-    by_name_subpath: &RelativePath,
+    by_name_dir: &ByNameDir,
     config: &Config,
 ) -> validation::Result<ratchet::Nixpkgs> {
     let nixpkgs_path = nixpkgs_path.canonicalize().with_context(|| {
@@ -219,15 +219,21 @@ fn check_nixpkgs(
     let mut nix_file_store = NixFileStore::default();
 
     let package_result = {
-        if !nixpkgs_path.join(by_name_subpath.as_str()).exists() {
+        if !nixpkgs_path.join(by_name_dir.path.as_str()).exists() {
             // No pkgs/by-name directory, always valid
             Success(BTreeMap::new())
         } else {
-            let structure = check_structure(&nixpkgs_path, &mut nix_file_store, by_name_subpath)?;
+            let structure = check_structure(&nixpkgs_path, &mut nix_file_store, by_name_dir)?;
 
             // Only if we could successfully parse the structure, we do the evaluation checks
             structure.result_map(|package_names| {
-                eval::check_values(&nixpkgs_path, &mut nix_file_store, package_names.as_slice(), config)
+                eval::check_values(
+                    &nixpkgs_path,
+                    &mut nix_file_store,
+                    package_names.as_slice(),
+                    by_name_dir,
+                    config,
+                )
             })?
         }
     };
@@ -249,7 +255,7 @@ mod tests {
 
     use anyhow::Context;
     use pretty_assertions::StrComparison;
-    use tempfile::{TempDir, tempdir_in};
+    use tempfile::{tempdir_in, TempDir};
 
     use crate::structure;
 
@@ -369,7 +375,7 @@ mod tests {
             process(
                 &base_nixpkgs,
                 &main_path,
-                &structure::read_config(Path::new("by-name-config.json")),
+                &structure::read_config(Path::new("by-name-config-generated.json")),
             )
         });
 

src/main.rs

@@ -7,11 +7,11 @@ use relative_path::RelativePathBuf;
 use rnix::ast;
 use rnix::ast::Expr;
 use rnix::ast::HasEntry;
+use rowan::ast::AstNode;
 use rowan::TextSize;
 use rowan::TokenAtOffset;
-use rowan::ast::AstNode;
-use std::collections::HashMap;
 use std::collections::hash_map::Entry;
+use std::collections::HashMap;
 use std::fs::read_to_string;
 use std::path::Path;
 use std::path::PathBuf;