Skip to content

Commit

Permalink
unit -> by-name
Browse files Browse the repository at this point in the history
And Readme.md -> README.md
  • Loading branch information
infinisil committed Jun 20, 2023
1 parent 7930b80 commit 7e88811
Show file tree
Hide file tree
Showing 16 changed files with 61 additions and 61 deletions.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion pkgs/test/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,5 @@ with pkgs;

pkgs-lib = recurseIntoAttrs (import ../pkgs-lib/tests { inherit pkgs; });

units = callPackage ./units { };
namedHierarchy = callPackage ./named-hierarchy { };
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
callPackage,
}:
let
unitBaseDirectory = ../../unit;
baseDirectory = ../../by-name;

/*
Directory structure:
pkgs/unit
pkgs/by-name
- bar
- bar (regular)
- fo
Expand All @@ -20,17 +20,17 @@ let
Expected output:
Structural checks for pkgs/unit failed:
- pkgs/unit/bar: Invalid name "bar", must be at most 2 ASCII characters consisting of lowercase letters, numbers or `-`/`_`
- pkgs/unit/fo: Duplicate case-sensitive entries:
- pkgs/unit/fo/foo
- pkgs/unit/fo/FOO
- pkgs/unit/fo/@:
Structural checks for pkgs/by-name failed:
- pkgs/by-name/bar: Invalid name "bar", must be at most 2 ASCII characters consisting of lowercase letters, numbers or `-`/`_`
- pkgs/by-name/fo: Duplicate case-sensitive entries:
- pkgs/by-name/fo/foo
- pkgs/by-name/fo/FOO
- pkgs/by-name/fo/@:
- Is a file, but should be a directory
- Invalid name "@": Must be ASCII characters consisting of letters, numbers or `-`/`_`
- Missing required `package.nix` file
- pkgs/unit/fo/bar:
- Incorrect directory: Should be in pkgs/unit/ba instead
- pkgs/by-name/fo/bar:
- Incorrect directory: Should be in pkgs/by-name/ba instead
Ensuring the shards are directories is done in stage.nix
Expand All @@ -40,27 +40,27 @@ let
let
shardNameIsValid = builtins.match "[a-z0-9_-]{1,2}" shard != null;
optionalShardNameError = lib.optional (! shardNameIsValid)
"- pkgs/unit/${shard}: Invalid name \"${shard}\", must be at most 2 ASCII characters consisting of lowercase letters, numbers or `-`/`_`\n";
"- pkgs/by-name/${shard}: Invalid name \"${shard}\", must be at most 2 ASCII characters consisting of lowercase letters, numbers or `-`/`_`\n";

entries = builtins.readDir (lib.path.append unitBaseDirectory shard);
entries = builtins.readDir (lib.path.append baseDirectory shard);
duplicates = lib.filterAttrs (name: values: lib.length values > 1) (lib.groupBy lib.toLower (lib.attrNames entries));
duplicateErrors = lib.mapAttrsToList (name: values: ''
- pkgs/unit/${shard}: Duplicate case-sensitive entries:
${lib.concatMapStrings (value: " - pkgs/unit/${shard}/${value}\n") values}''
- pkgs/by-name/${shard}: Duplicate case-sensitive entries:
${lib.concatMapStrings (value: " - pkgs/by-name/${shard}/${value}\n") values}''
) duplicates;

unitErrors = lib.concatLists (lib.mapAttrsToList (unit: type:
packageErrors = lib.concatLists (lib.mapAttrsToList (attr: type:
let
unitNameIsValid = builtins.match "[a-zA-Z0-9_-]+" unit != null;
optionalUnitNameError = lib.optional (! unitNameIsValid)
" - Invalid name \"${unit}\": Must be ASCII characters consisting of letters, numbers or `-`/`_`\n";
attrNameIsValid = builtins.match "[a-zA-Z0-9_-]+" attr != null;
optionalAttrNameError = lib.optional (! attrNameIsValid)
" - Invalid name \"${attr}\": Must be ASCII characters consisting of letters, numbers or `-`/`_`\n";

correctShard = lib.toLower (lib.substring 0 2 unit);
correctShard = lib.toLower (lib.substring 0 2 attr);
directoryIsCorrect = shard == correctShard;
optionalDirectoryError = lib.optional (shardNameIsValid && unitNameIsValid && ! directoryIsCorrect)
" - Incorrect directory: Should be pkgs/unit/${correctShard}/${unit} instead\n";
optionalDirectoryError = lib.optional (shardNameIsValid && attrNameIsValid && ! directoryIsCorrect)
" - Incorrect directory: Should be pkgs/by-name/${correctShard}/${attr} instead\n";

packageNixPath = (lib.path.append unitBaseDirectory (lib.path.subpath.join [ shard unit "package.nix" ]));
packageNixPath = (lib.path.append baseDirectory (lib.path.subpath.join [ shard attr "package.nix" ]));
hasPackageNix = builtins.pathExists packageNixPath;
optionalPackageNixError = lib.optional (! hasPackageNix)
" - Missing required `package.nix` file\n";
Expand All @@ -69,37 +69,37 @@ let

errors =
if type == "directory" then
optionalUnitNameError
optionalAttrNameError
++ optionalDirectoryError
++ optionalPackageNixError
else
[ fileError ];
in
lib.optional (errors != []) ''
- pkgs/unit/${shard}/${unit}:
- pkgs/by-name/${shard}/${attr}:
${lib.concatStrings errors}''
) entries);
in
if type == "directory" then
optionalShardNameError
++ duplicateErrors
++ unitErrors
++ packageErrors
else
[ "- pkgs/unit/${shard}: Is a file, but should be a directory.\n" ];
[ "- pkgs/by-name/${shard}: Is a file, but should be a directory.\n" ];

shards = builtins.removeAttrs (builtins.readDir unitBaseDirectory) [ "Readme.md" ];
shards = builtins.removeAttrs (builtins.readDir baseDirectory) [ "README.md" ];

structureErrors = lib.concatLists (lib.mapAttrsToList errorPartsForShard shards);

structureResult =
if structureErrors == [] then
builtins.trace "Structural evaluation checks for pkgs/unit successfull" null
builtins.trace "Structural evaluation checks for pkgs/by-name successfull" null
else
throw ''
Structural evaluation checks for pkgs/unit failed:
Structural evaluation checks for pkgs/by-name failed:
${lib.concatStrings structureErrors}'';

unitFiles = import ../../top-level/unit-files.nix;
packageFiles = import ../../top-level/named-hierarchy-files.nix;

overlay = self: super: {
callPackage = fn: args:
Expand Down Expand Up @@ -140,21 +140,21 @@ let
[ "${prefix}: Not a derivation" ]
else
[ ]
) unitFiles);
) packageFiles);


# Excludes the reference check
semanticResult =
if semanticErrors == [] then
builtins.trace "Semantic evaluation checks for pkgs/unit successfull" null
builtins.trace "Semantic evaluation checks for pkgs/by-name successfull" null
else
throw ''
Semantic evaluation checks for pkgs/unit failed:
Semantic evaluation checks for pkgs/by-name failed:
${lib.concatStrings semanticErrors}'';

refCheck = callPackage ./reference { };
in
runCommand "units-test" {
runCommand "named-hierarchy-tests" {
nativeBuildInputs = [
structureResult
semanticResult
Expand All @@ -163,21 +163,21 @@ runCommand "units-test" {
passthru.reference = refCheck;
} ''
mkdir -p pkgs
cp -r ${unitBaseDirectory} pkgs/unit
cp -r ${baseDirectory} pkgs/by-name
failed=
for unitDir in pkgs/unit/*/*; do
# echo "Check that $unitDir doesn't refer to files outside itself"
if ! nix-reference-check "$unitDir"; then
for packageDir in pkgs/by-name/*/*; do
# echo "Check that $packageDir doesn't refer to files outside itself"
if ! nix-reference-check "$packageDir"; then
failed=1
fi
done
if [[ -n "$failed" ]]; then
echo "Reference checks for pkgs/unit failed"
echo "Reference checks for pkgs/by-name failed"
exit 1
else
echo "Reference checks for pkgs/unit successfull"
echo "Reference checks for pkgs/by-name successfull"
touch $out
fi
''
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
1 change: 1 addition & 0 deletions pkgs/test/named-hierarchy/reference/shell.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(import ../../../.. {}).tests.namedHierarchy.reference
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn resolve_reference(
source: &PathBuf,
line: usize,
reference: PathBuf,
unit_dir: &PathBuf,
base_dir: &PathBuf,
) -> Result<PathBuf> {
let mut rel_to_root = source.parent().unwrap().to_path_buf();
let mut ascending = true;
Expand All @@ -35,11 +35,11 @@ fn resolve_reference(
)));
}
rel_to_root = match rel_to_root.parent() {
Some(parent) if parent.starts_with(unit_dir) => parent.to_path_buf(),
Some(parent) if parent.starts_with(base_dir) => parent.to_path_buf(),
_ => {
return Err(Error::msg(format!(
"File {:?} on line {:?} refers to a path that escapes the unit directory {:?}: {:?}",
source, line, unit_dir, reference
"File {:?} on line {:?} refers to a path that escapes the package directory {:?}: {:?}",
source, line, base_dir, reference
)));
}
};
Expand All @@ -66,7 +66,7 @@ fn resolve_reference(
Ok(rel_to_root)
}

fn check_path(unit_dir: &PathBuf, file: &PathBuf, already_checked: &mut HashSet<PathBuf>) -> Result<()> {
fn check_path(base_dir: &PathBuf, file: &PathBuf, already_checked: &mut HashSet<PathBuf>) -> Result<()> {

if ! already_checked.insert(file.canonicalize()?) {
//println!("Skipping check for file {:?} because it was already checked before", file);
Expand All @@ -75,7 +75,7 @@ fn check_path(unit_dir: &PathBuf, file: &PathBuf, already_checked: &mut HashSet<

if file.is_dir() {
for entry in file.read_dir()? {
check_path(&unit_dir, &entry?.path(), already_checked)?
check_path(&base_dir, &entry?.path(), already_checked)?
}
return Ok(())
}
Expand Down Expand Up @@ -119,11 +119,11 @@ fn check_path(unit_dir: &PathBuf, file: &PathBuf, already_checked: &mut HashSet<
)));
}

let resolved = resolve_reference(&file, line, PathBuf::from(&text), &unit_dir)?;
let resolved = resolve_reference(&file, line, PathBuf::from(&text), &base_dir)?;

//println!("Resolved reference {:?} in file {:?} line {:?} to {:?}", text, &file, line, resolved);

check_path(&unit_dir, &resolved, already_checked)?;
check_path(&base_dir, &resolved, already_checked)?;
}

Ok(())
Expand All @@ -138,10 +138,10 @@ fn main() -> Result<()> {
if args.len() != 2 {
return usage();
}
let unit_dir = PathBuf::from(args[1].clone());
if ! unit_dir.is_dir() {
let base_dir = PathBuf::from(args[1].clone());
if ! base_dir.is_dir() {
return usage();
}
let mut already_checked = HashSet::new();
check_path(&unit_dir, &unit_dir, &mut already_checked)
check_path(&base_dir, &base_dir, &mut already_checked)
}
1 change: 0 additions & 1 deletion pkgs/test/units/reference/shell.nix

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/*
Internal Nixpkgs file, it may be removed or changed at any point without warning
This file contains the mapping from attribute name to the file in pkgs/unit that contains a definition of it.
This file contains the mapping from attribute name to the file in pkgs/by-name that contains a definition of it.
No validity checks are performed, instead this file is optimized for performance.
TODO: Link to manual and validity checks
Example: If `pkgs/unit/he/hello` exists, the result will have a `hello = pkgs/unit/he/hello/package.nix` entry.
Example: If `pkgs/by-name/he/hello` exists, the result will have a `hello = pkgs/by-name/he/hello/package.nix` entry.
Type: AttrsOf Path
*/
Expand All @@ -20,7 +20,7 @@ let
inherit (builtins) attrNames elemAt length mapAttrs readDir;

# Warning: Do not make this configurable, otherwise the `import` evaluation cache won't work!
baseDirectory = ../unit;
baseDirectory = ../by-name;

entries = readDir baseDirectory;
shards = attrNames entries;
Expand All @@ -33,7 +33,7 @@ let
# - Assume that Readme.md is the only file and change the condition to `shard == "Readme.md"` for a minor performance improvement.
# This would however cause very poor error messages if there's other files.
# - Ensure that Readme.md is the only file, throwing a better error message if that's not the case.
# However for good code architecture, all error messages relating to `pkgs/unit` should be centralized in (TODO: Link to validity checks)
# However for good code architecture, all error messages relating to `pkgs/by-name` should be centralized in (TODO: Link to validity checks)
# Additionally in either of those alternatives, we would have to duplicate the hardcoding of "Readme.md"
{ }
else
Expand Down Expand Up @@ -87,7 +87,7 @@ let

# We benchmark the three approaches with the following command with 300 runs:
#
# NIX_SHOW_STATS=1 nix-instantiate --eval --strict 'builtins.seq (import pkgs/top-level/unit-files.nix) null'
# NIX_SHOW_STATS=1 nix-instantiate --eval --strict 'builtins.seq (import pkgs/top-level/named-hierarchy-files.nix) null'
# Approach | cpuTime [ms] | gc.totalBytes [MB]
# linear merge | 33.4 ± 4.8 | 126.075
# concatMap | 21.9 ± 2.6 | 5.824
Expand Down
6 changes: 3 additions & 3 deletions pkgs/top-level/stage.nix
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,10 @@ let
};

# Auto-calls packages declared in the named package hierarchy in pkgs/unit
unitPackages = self: super:
namedHierarchyPackages = self: super:
lib.mapAttrs (name: file:
self.callPackage file { }
) (import ./unit-files.nix);
) (import ./named-hierarchy-files.nix);

# The complete chain of package set builders, applied from top to bottom.
# stdenvOverlays must be last as it brings package forward from the
Expand All @@ -283,7 +283,7 @@ let
stdenvAdapters
trivialBuilders
splice
unitPackages
namedHierarchyPackages
allPackages
otherPackageSets
aliases
Expand Down

0 comments on commit 7e88811

Please sign in to comment.