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 5b90d1d
Show file tree
Hide file tree
Showing 19 changed files with 75 additions and 75 deletions.
8 changes: 4 additions & 4 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@
/pkgs/build-support/setup-hooks/auto-patchelf.py @layus
/pkgs/pkgs-lib @infinisil

# Unit directories
/pkgs/top-level/unit-files.nix @infinisil @roberth
/pkgs/unit/Readme.md @infinisil @roberth
/pkgs/test/units @infinisil @roberth
# Named package hierarchy
/pkgs/top-level/named-hierarchy-files.nix @infinisil @roberth
/pkgs/by-name/README.md @infinisil @roberth
/pkgs/test/named-hierarchy @infinisil @roberth

# Nixpkgs build-support
/pkgs/build-support/writers @lassulus @Profpatsch
Expand Down
8 changes: 4 additions & 4 deletions doc/contributing/coding-conventions.chapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,14 @@ Names of files and directories should be in lowercase, with dashes between words

### New top-level package hierarchy {#sec-named-hierarchy}

New top-level packages may be declared in a `package.nix` file in a corresponding `pkgs/unit/$prefix/$name` directory, where `$name` is the attribute name the package should be exposed as, and `$prefix` is the lowercased two-letter prefix of the package name.
New top-level packages may be declared in a `package.nix` file in a corresponding `pkgs/by-name/$prefix/$name` directory, where `$name` is the attribute name the package should be exposed as, and `$prefix` is the lowercased two-letter prefix of the package name.
The `package.nix` file must be a function taking other top-level attributes or configuration flags as arguments, and returning a derivation.
Arbitrary other files may be created in the directory if desired.
Package files declared in this way are automatically called with `callPackage` and added to the top-level package set at `pkgs.$name`.

If there is a need customize the second `callPackage` attribute argument, this can be done be redefining the same package attribute with `callPackage` and the custom arguments in `pkgs/top-level/all-packages.nix`.

For example, `pkgs.libfoo` may be declared in `pkgs/unit/li/libfoo/package.nix` with a Nix expression as follows:
For example, `pkgs.libfoo` may be declared in `pkgs/by-name/li/libfoo/package.nix` with a Nix expression as follows:
```nix
{ lib, stdenv, libbar, enableBaz ? false }:
stdenv.mkDerivation {
Expand All @@ -254,11 +254,11 @@ stdenv.mkDerivation {
}
```

This automatically declares `pkgs.libfoo` as `pkgs.callPackage pkgs/unit/li/libfoo/package.nix { }`.
This automatically declares `pkgs.libfoo` as `pkgs.callPackage pkgs/by-name/li/libfoo/package.nix { }`.

If `libfoo` doesn't compile with the default `libbar` version, and `pkgs.libbar_2` is available, you can change the default `libbar` version by adding a definition like this to `pkgs/top-level/all-packages.nix`:
```nix
libfoo = callPackage ../unit/li/libfoo/package.nix {
libfoo = callPackage ../by-name/li/libfoo/package.nix {
libbar = libbar_2;
};
```
Expand Down
10 changes: 5 additions & 5 deletions doc/contributing/quick-start.chapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@ To add a top-level package `pkgs.pkgname` to Nixpkgs:
$ cd nixpkgs
```

2. Create a package directory `pkgs/unit/$prefix/$name` where `$name` is the name of the package and `$prefix` is the lowercased 2-letter prefix of the package name (for more information see [](#sec-named-hierarchy)).
2. Create a package directory `pkgs/by-name/$prefix/$name` where `$name` is the name of the package and `$prefix` is the lowercased 2-letter prefix of the package name (for more information see [](#sec-named-hierarchy)).

```ShellSession
$ mkdir -p pkgs/unit/li/libfoo
$ mkdir -p pkgs/by-name/li/libfoo
```

3. Create a `package.nix` file in the package directory, containing a Nix expression — a piece of code that describes how to build the package. In this case, it should be a _function_ that is called with the package dependencies as arguments, and returns a build of the package in the Nix store.

```ShellSession
$ emacs pkgs/unit/li/libfoo/package.nix
$ git add pkgs/unit/li/libfoo/package.nix
$ emacs pkgs/by-name/li/libfoo/package.nix
$ git add pkgs/by-name/li/libfoo/package.nix
```

You can have a look at the existing Nix expressions under `pkgs/` to see how it’s done. Here are some good ones:

- GNU Hello: [`pkgs/unit/he/hello/package.nix`](https://github.com/NixOS/nixpkgs/blob/master/pkgs/unit/he/hello/package.nix). Trivial package, which specifies some `meta` attributes which is good practice.
- GNU Hello: [`pkgs/by-name/he/hello/package.nix`](https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/he/hello/package.nix). Trivial package, which specifies some `meta` attributes which is good practice.

- GNU cpio: [`pkgs/tools/archivers/cpio/default.nix`](https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/archivers/cpio/default.nix). Also a simple package. The generic builder in `stdenv` does everything for you. It has no dependencies beyond `stdenv`.

Expand Down
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
Loading

0 comments on commit 5b90d1d

Please sign in to comment.