From dcb50e9a3fffc88283bf5986d784890c34eb32f5 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 22 Nov 2022 00:19:51 +0100 Subject: [PATCH 1/9] Sharding and other updates - Describe sharding as discussed in the last NAT meeting - Refines which packages are eligible for this transition - Exclude functions and packages that aren't self-contained regarding references - Add some TODO's - General text improvements - Add an example tree --- README.md | 161 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 98 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index c906a20..75293e2 100644 --- a/README.md +++ b/README.md @@ -11,51 +11,61 @@ related-issues: (will contain links to implementation PRs) # Summary [summary]: #summary -Make most attribute definitions in `pkgs/top-level/all-packages.nix` be auto-generated from a single flat directory instead, where the subdirectory corresponds to the attribute name. The ad-hoc category-based structure of packages will be gotten rid of. +Make trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` be auto-generated from a predictable attribute-based file hierarchy. +This makes it much easier to contribute new packages packages, since there's no more guessing needed as to where the package should go, both in the ad-hoc directory categories and in `pkgs/top-level/all-packages.nix`. + # Motivation [motivation]: #motivation -- (Especially new) package contributors are having a hard time figuring out which files to add and edit. These are very common questions: - - Which directory should my package definition go in? What are all the categories and do they matter? What if the package has multiple matching categories? - - Why can't I build my package after adding the package file? [introduced to all-packages.nix] Where in all-packages.nix should my package go? -- Figuring out where an attribute is defined is very tricky: +- (Especially new) package contributors are having a hard time figuring out which files to add or edit. These are very common questions: + - Which directory should my package definition go in? + - What are all the categories and do they matter? + - What if the package has multiple matching categories? + - Why can't I build my package after adding the package file? + - Where in all-packages.nix should my package go? +- Figuring out where an attribute is defined is a bit tricky: - First one has to find the definition of it in all-packages.nix to see what file it refers to - - Especially on GitHub this is even more problematic, as the `all-packages.nix` file is [too big to be displayed by GitHub](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/top-level/all-packages.nix) + - On GitHub this is even more problematic, as the `all-packages.nix` file is [too big to be displayed by GitHub](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/top-level/all-packages.nix) - Then go to that file's definition, which takes quite some time for navigation (unless you have a plugin that can jump to it directly) - `all-packages.nix` frequently causes merge conflicts. It's a point of contention for all new packages # Detailed design [design]: #detailed-design -All attributes at the root of nixpkgs (`pkgs.`) whose definition is of the form +For all attributes at the root of nixpkgs `pkgs.` which: +1. Are defined in `pkgs/top-level/all-packages.nix` + (necessary so that the overlay containing the automatically discovered packages can be ordered directly before the `all-packages.nix` overlay without changing any behavior) +2. Are defined to be equal to `pkgs.callPackage { }` such that all transitively referenced paths from the default Nix file of ``: + 1. Are under the directory of `` + (necessary so that moving these files to a new directory doesn't break references in this package) + 2. Are not referenced from any paths outside of these transitive references + (necessary so that moving these files to a new directory doesn't break references in other packages) +3. Evaluate to a derivation + (necessary because using `pkg-fun.nix` for a non-package would be counter-intuitive) -```nix -{ - = pkgs.callPackage ../some/dir { }; -} -``` +These will be become eligible to be transformed as follows: +- Move the default Nix file from `` to `pkgs/unit/<4-prefix name>//pkg-fun.nix` ([TODO: Justify why `unit`](https://github.com/nixpkgs-architecture/simple-package-paths/issues/16)) + - Where `<4-prefix name>` is the 4-letter prefix of ``. + If `` has less than 4 characters, append `-` to the `` until it's 4 characters long and use that as `<4-prefix name>` +- Additionally also move all paths transitively referenced by the default Nix file to `pkgs/unit/<4-prefix name>//???` [TODO](https://github.com/nixpkgs-architecture/simple-package-paths/issues/19) +- Remove the definition of that attribute in `pkgs/top-level/all-packages.nix` +- For each moved path, create a compatibility layer from the old to the new path, potentially using a symlink. See [compatibility] for more details -will be become eligible to be transformed as follows: -- Move `pkgs/some/dir` to `pkgs/auto/` -- Fix any references to that folder (e.g. in update scripts) -- Remove the original definition +These attributes will newly be added to `pkgs` by automatically calling `pkgs.callPackage pkgs/unit/<4-prefix name>//pkg-fun.nix { }` on all entries in `pkgs/unit`. In order to make this more efficient, `builtins.readDir` should be optimized as described [here](https://github.com/NixOS/nix/issues/7314). -These attributes will newly be defined by listing all directories in `pkgs/auto` using `builtins.readDir` and calling `pkgs.callPackage` on all of them, which then gets added to the `pkgs` scope. +## Compatibility layer +[compatibility]: #compatibility-layer + +TODO: Nix files should use `import` to act like a symlink while also giving a warning with `builtins.trace`. Something like -Attributes whose definition aren't of the above form won't be changed, so e.g. the following definition in `all-packages.nix` won't be changed: ```nix -{ - syncplay-nogui = syncplay.override { enableGUI = false; }; -} +builtins.trace "warning: Using deprecated path ${./.}, use pkgs/unit/ instead, this will be removed after NixOS 22.05" + (import ../../pkgs/unit/name) ``` -However, if such definitions can be refactored into the above form they will become eligible for the transformation. - -## Backwards compatibility symlinks -[symlinks]: #backwards-compatibility-symlinks -When moving `pkgs/some/dir/default.nix` to the new `pkgs/unit//pkg-fun.nix`, a symlink will be created pointing from `pkgs/unit/some/dir/default.nix` to `pkgs/unit//pkg-fun.nix`. Reasoning: +When moving `pkgs/some/dir/default.nix` to the new `pkgs/unit/<4-prefix name>//pkg-fun.nix`, a symlink will be created pointing from the old to the new location. Reasoning: - Current community discussions referencing old files from the `master` branch are still valid for some time. While GitHub doesn't provide an easy way to navigate to a symlink, seeing the path to where the file has moved is better than getting an error. - It provides an opportunity for code referencing old paths to be updated. While it's not possible to give a deprecation warning with symlinks, users will at least be able to read it in the NixOS release notes. This doesn't occur often in practice. @@ -71,13 +81,48 @@ This RFC makes no requirement as to how the transition should happen, but here a # Examples [examples]: #examples -- `pkgs.hello`: - - Move from `pkgs/applications/misc/hello/default.nix` to `pkgs/unit/hello/pkg-fun.nix` - - Move from `pkgs/applications/misc/hello/test.nix` to `pkgs/unit/hello/test.nix` -- `pkgs.gnumake`: Move from `pkgs/development/tools/build-managers/gnumake` to `pkgs/unit/gnumake` -- `pkgs.gnumake42`: Move from `pkgs/development/tools/build-managers/gnumake/4.2` to `pkgs/unit/gnumake42` -- `pkgs.buildEnv`: Move from `pkgs/build-support/buildenv` to `pkgs/unit/buildEnv` -- `pkgs.fetchFromGitHub`: Move from `pkgs/build-support/fetchgithub` to `pkgs/unit/fetchFromGitHub` +- `pkgs.hello` matches all criteria: + The default Nix file [`pkgs/applications/misc/hello/default.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/applications/misc/hello/default.nix) only transitively [references `test.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/applications/misc/hello/default.nix#L31) in the same directory. + Neither the `default.nix` nor `test.nix` is referenced by any other file in nixpkgs, so we can do the transformation: + - Move `pkgs/applications/misc/hello/default.nix` to `pkgs/unit/hell/hello/pkg-fun.nix` + - Move `pkgs/applications/misc/hello/test.nix` to `pkgs/unit/hell/hello/???` [TODO](https://github.com/nixpkgs-architecture/simple-package-paths/issues/19) +- `pkgs.gnumake` matches all criteria: + The default Nix file [`pkgs/development/tools/build-managers/gnumake/default.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/tools/build-managers/gnumake/default.nix) transitively references only files in its own directory and no other files in nixpkgs reference `gnumake`'s files, so we can do the transformation by moving all the files from `pkgs/development/tools/build-managers/gnumake` to `pkgs/unit/gnum/gnumake`, the default Nix file ending up in `pkgs/unit/gnum/gnumake/pkg-fun.nix`. +- Similarly `pkgs.gnumake42` in [`pkgs/development/tools/build-managers/gnumake/4.2/default.nix`](https://github.com/NixOS/nixpkgs/tree/nixos-22.05/pkgs/development/tools/build-managers/gnumake/4.2/default.nix) fulfils all criteria, even though its directory is nested in `pkgs.gnumake`'s directory, they don't reference each others files. +- `pkgs.zsh` matches all criteria, its default Nix file is moved to `pkgs/unit/zsh-/zsh/pkg-fun.nix` +- `pkgs.emptyFile` doesn't fulfil criteria 1 (it's defined in `pkgs/build-support/trivial-builders.nix`) and 2 (it's defined inline), so it can't be moved into `pkgs/unit` +- `pkgs.fetchFromGitHub` doesn't fulfil the criteria 3 (it evaluates to a function), so it can't be moved into `pkgs/unit` + +Here's how such a `pkgs/unit` directory structure would look like, note how all attribute names have the same level of nesting: +``` +pkgs +└── unit + ├── acpi + │ ├── acpi + │ ├── acpica-tools + │ ├── acpid + │ ┊ + ┊ + ├── auto + │ ├── autossh + │ ├── automirror + │ ├── autosuspend + │ ┊ + ┊ + ├── sl-- + │ └── sl + ┊ + ├── slac + │ ├── slack + │ ├── slack-cli + │ └── slack-term + ┊ + ├── zsh- + │ ├── zsh + │ ├── zsh-autocomplete + │ ├── zsh-completions + ┊ ┊ +``` # Interactions [interactions]: #interactions @@ -93,8 +138,6 @@ This RFC makes no requirement as to how the transition should happen, but here a - It was an inconvenient user interface, requiring a checkout or browsing through GitHub - Many packages fit multiple categories, leading to multiple locations to search through instead of one - There's other better ways of discovering similar packages, e.g. [Repology](https://repology.org/) -- GitHub's UI can't display more than 1000 items in a directory. Counter-arguments: - - It's still possible to open a subdirectory using the "Go to file" button, or pressing the `T` key - Creating [symlinks](#symlinks) for the old paths has the potential to create merge conflicts between the symlink and the changed original file there. If the symlink wasn't there, GitHub could perform auto-merging. Counter-arguments: - There's not too many such merge conflicts - This breaks `builtins.unsafeGetAttrPos "hello" pkgs`. Counter-arguments: @@ -104,38 +147,30 @@ This RFC makes no requirement as to how the transition should happen, but here a # Alternatives [alternatives]: #alternatives -- Create a prefix-based hierarchy of directories, e.g. `pkgs/unit/he/hello`, similar to `.git/objects`, so that no directory has more than 1000 entries, enabling GitHub to display the entries, therefore improving navigation on GitHub. Downsides are: - - Slower evaluation, since a lot more directories need to be traversed - - Increased end-user complexity: - - Creating the package files often requires the creating of 2 directories, not just one - - Referencing the files requires knowing the prefix schema -- Improve deprecation signalling by creating `.nix` files that act like a symlink, but with a warning. Something like this: - ```nix - builtins.trace "warning: Using deprecated path ${./.}, use pkgs/unit/ instead, this will be removed after NixOS 22.05" - (import ../../pkgs/unit/name) - ``` - The main downside of this is the increased complexity of implementation - - - Use `package.nix` instead of `pkg-fun.nix` - - Makes the migration to a non-function form of overridable packages harder in the future. We'd like to use `package.nix` for a package format that's based on a fixpoint rather than a function, because that will make overriding simpler. - - - Use `default.nix` instead of `pkg-fun.nix` - - `default.nix`'s only benefits do not apply - - removing the need to specify the file name in expressions, but this does not apply because we have to do this at most once in the code that replaces definitions from `all-packages.nix`. - - removing the need to specify the file name on the command line, but this does not apply because a package function must be imported into an expression before it can be used, making `nix build -f pkgs/unit/hello` equally broken regardless of file name. - - Choosing `default.nix` would bias the purpose of the `pkg` directory to serve only as package definitions, whereas we could make the tree more human friendly by grouping files together by "topic" rather than by technical delineations. For instance, having a package definition, changelog, package-specific config generator and perhaps even NixOS module in one directory makes work on the package in a broad sense easier. This is not a goal of this RFC, but a motivation to make this a future possibility. - - `pkg-fun.nix` frees up `default.nix` for a short expression that is actually buildable, e.g. `(import ../..).hello`. - - - Use `unit/` instead of `pkgs/unit`. This is future proof in case we want to - make the directory structure more general purpose, but this is out of scope - and we want to improve tooling to make renames easy. - +- Use a flat directory `pkgs/unit/*/pkg-fun.nix` instead, arguments: + - Good because it speeds up Nix evaluation since there's only a single directory to call `builtins.readDir` on instead of many + - With an optimized `readDir` this isn't much of a problem + - Good because it's simpler, both for the user and for the code + - Bad because it causes GitHub to limit the rendering of that directory to 1'000 entries (and we have about 10'000 that benefit from this transition for a start) + - Bad because it makes `git` slower ([TODO: By how much?](https://github.com/nixpkgs-architecture/simple-package-paths/issues/18)) + +- Don't use `pkg-fun.nix` but another file name: + - `package.nix`/`pkg.nix`: Makes the migration to a non-function form of overridable packages harder in the future. + - `default.nix`: + - Doesn't have its main benefits in this case: + - Removing the need to specify the file name in expressions, but this does not apply because this file will be imported automatically by the code that replaces definitions from `all-packages.nix`. + - Removing the need to specify the file name on the command line, but this does not apply because a package function must be imported into an expression before it can be used, making `nix build -f pkgs/unit/hell/hello` equally broken regardless of file name. + - Not using `default.nix` frees up `default.nix` for a short expression that is actually buildable, e.g. `(import ../..).hello`. + - Choosing `default.nix` would bias the purpose of the `unit` directory to serve only as package definitions, whereas we could make the tree more human friendly by grouping files together by "topic" rather than by technical delineations. + For instance, having a package definition, changelog, package-specific config generator and perhaps even NixOS module in one directory makes work on the package in a broad sense easier. + This is not a goal of this RFC, but a motivation to make this a future possibility. + + - Use `unit` (at the nixpkgs root) instead of `pkgs/unit`. + This is future proof in case we want to make the directory structure more general purpose, but this is out of scope # Unresolved questions [unresolved]: #unresolved-questions -- Is it really okay to not be able to list the package directory in GitHub? Will the "Go to file" function be good enough? - # Future work [future]: #future-work From e01ede667fed52406d06e4049bd2463e771646d1 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 14 Dec 2022 23:02:34 +0100 Subject: [PATCH 2/9] Updates with reviews, meetings and other stuff --- README.md | 87 ++++++++++++++++++++++++++----------------------------- 1 file changed, 41 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 75293e2..8740d43 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ feature: simple-package-paths start-date: 2022-09-02 author: Silvan Mosberger -co-authors: (find a buddy later to help out with the RFC) +co-authors: Nixpkgs Architecture Team shepherd-team: (names, to be nominated and accepted by RFC steering committee) shepherd-leader: (name to be appointed by RFC steering committee) related-issues: (will contain links to implementation PRs) @@ -28,55 +28,39 @@ This makes it much easier to contribute new packages packages, since there's no - First one has to find the definition of it in all-packages.nix to see what file it refers to - On GitHub this is even more problematic, as the `all-packages.nix` file is [too big to be displayed by GitHub](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/top-level/all-packages.nix) - Then go to that file's definition, which takes quite some time for navigation (unless you have a plugin that can jump to it directly) + - It also slows down or even deadlocks editors due to the file size + - In some cases `nix edit` works, though that's not yet stable (it relies on Flakes being enabled) and comes with some problems ([doesn't allow the file to be edited](https://github.com/NixOS/nix/issues/3347), doesn't work with packages that don't set `meta.position` correctly). - `all-packages.nix` frequently causes merge conflicts. It's a point of contention for all new packages # Detailed design [design]: #detailed-design -For all attributes at the root of nixpkgs `pkgs.` which: -1. Are defined in `pkgs/top-level/all-packages.nix` +For all attributes at the root of nixpkgs `pkgs.` which satisfy these criteria: +1. Is defined in `pkgs/top-level/all-packages.nix` (necessary so that the overlay containing the automatically discovered packages can be ordered directly before the `all-packages.nix` overlay without changing any behavior) -2. Are defined to be equal to `pkgs.callPackage { }` such that all transitively referenced paths from the default Nix file of ``: - 1. Are under the directory of `` - (necessary so that moving these files to a new directory doesn't break references in this package) - 2. Are not referenced from any paths outside of these transitive references - (necessary so that moving these files to a new directory doesn't break references in other packages) -3. Evaluate to a derivation +2. Is defined to be equal to `pkgs.callPackage { }` +3. All transitively referenced paths from the default Nix file of `` are under the same directory as the default Nix file and can be moved around together without breaking any references in other Nix files (except the one reference in `pkgs/top-level/all-packages.nix`). + This means that the Nix code should neither reference code outside, nor be referenced from outside. + (This is necessary so that no Nix code needs to be updated in the transition below) +4. Evaluates to a derivation (necessary because using `pkg-fun.nix` for a non-package would be counter-intuitive) These will be become eligible to be transformed as follows: - Move the default Nix file from `` to `pkgs/unit/<4-prefix name>//pkg-fun.nix` ([TODO: Justify why `unit`](https://github.com/nixpkgs-architecture/simple-package-paths/issues/16)) - - Where `<4-prefix name>` is the 4-letter prefix of ``. - If `` has less than 4 characters, append `-` to the `` until it's 4 characters long and use that as `<4-prefix name>` -- Additionally also move all paths transitively referenced by the default Nix file to `pkgs/unit/<4-prefix name>//???` [TODO](https://github.com/nixpkgs-architecture/simple-package-paths/issues/19) + - Where `<4-prefix name>` is the 4-letter prefix of ``, equal to `substring 0 4 name`. + If `` has less than or exactly 4 characters, `<4-prefix name>` is equal to just ``. +- Additionally also move all paths transitively referenced by the default Nix file to `pkgs/unit/<4-prefix name>/` [TODO](https://github.com/nixpkgs-architecture/simple-package-paths/issues/19) - Remove the definition of that attribute in `pkgs/top-level/all-packages.nix` -- For each moved path, create a compatibility layer from the old to the new path, potentially using a symlink. See [compatibility] for more details These attributes will newly be added to `pkgs` by automatically calling `pkgs.callPackage pkgs/unit/<4-prefix name>//pkg-fun.nix { }` on all entries in `pkgs/unit`. In order to make this more efficient, `builtins.readDir` should be optimized as described [here](https://github.com/NixOS/nix/issues/7314). -## Compatibility layer -[compatibility]: #compatibility-layer - -TODO: Nix files should use `import` to act like a symlink while also giving a warning with `builtins.trace`. Something like - -```nix -builtins.trace "warning: Using deprecated path ${./.}, use pkgs/unit/ instead, this will be removed after NixOS 22.05" - (import ../../pkgs/unit/name) -``` - - -When moving `pkgs/some/dir/default.nix` to the new `pkgs/unit/<4-prefix name>//pkg-fun.nix`, a symlink will be created pointing from the old to the new location. Reasoning: -- Current community discussions referencing old files from the `master` branch are still valid for some time. While GitHub doesn't provide an easy way to navigate to a symlink, seeing the path to where the file has moved is better than getting an error. -- It provides an opportunity for code referencing old paths to be updated. While it's not possible to give a deprecation warning with symlinks, users will at least be able to read it in the NixOS release notes. This doesn't occur often in practice. - -These symlinks need to be present for at least one NixOS release. - ## Transitioning -This RFC makes no requirement as to how the transition should happen, but here are some possible ways: -- Rip-off-the-bandaid approach: Do a big transition, causing a lot of merge conflicts with existing PR's creating and updating packages. Inform PR authors on how to progress -- Incremental CI approach: Introduce a CI action that complains when new packages (and maybe updated packages as well) use the old package paths -- Transparent and sneaky approach: Transition only packages that can be done without causing merge conflicts for all existing PRs. Repeat every once in a while until all packages are done +This RFC comes with [a reference tool](https://github.com/nixpkgs-architecture/simple-package-paths/pull/22) to make the above transition in an automated way. +If this RFC is accepted, the result of that tool will be PR'd to nixpkgs. +The tool itself will also be added to nixpkgs so that it can easily be ran again in the future. +For at least one release cycle, the legacy way of declaring packages should still be accepted, but the tool can be ran again at any point, thereby moving those new packages from the legacy paths to the new `pkgs/unit` paths. +A CI action may also be implemented to help with this if deemed necessary. # Examples [examples]: #examples @@ -85,13 +69,15 @@ This RFC makes no requirement as to how the transition should happen, but here a The default Nix file [`pkgs/applications/misc/hello/default.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/applications/misc/hello/default.nix) only transitively [references `test.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/applications/misc/hello/default.nix#L31) in the same directory. Neither the `default.nix` nor `test.nix` is referenced by any other file in nixpkgs, so we can do the transformation: - Move `pkgs/applications/misc/hello/default.nix` to `pkgs/unit/hell/hello/pkg-fun.nix` - - Move `pkgs/applications/misc/hello/test.nix` to `pkgs/unit/hell/hello/???` [TODO](https://github.com/nixpkgs-architecture/simple-package-paths/issues/19) + - Move `pkgs/applications/misc/hello/test.nix` to `pkgs/unit/hell/hello/test.nix` - `pkgs.gnumake` matches all criteria: The default Nix file [`pkgs/development/tools/build-managers/gnumake/default.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/tools/build-managers/gnumake/default.nix) transitively references only files in its own directory and no other files in nixpkgs reference `gnumake`'s files, so we can do the transformation by moving all the files from `pkgs/development/tools/build-managers/gnumake` to `pkgs/unit/gnum/gnumake`, the default Nix file ending up in `pkgs/unit/gnum/gnumake/pkg-fun.nix`. - Similarly `pkgs.gnumake42` in [`pkgs/development/tools/build-managers/gnumake/4.2/default.nix`](https://github.com/NixOS/nixpkgs/tree/nixos-22.05/pkgs/development/tools/build-managers/gnumake/4.2/default.nix) fulfils all criteria, even though its directory is nested in `pkgs.gnumake`'s directory, they don't reference each others files. -- `pkgs.zsh` matches all criteria, its default Nix file is moved to `pkgs/unit/zsh-/zsh/pkg-fun.nix` -- `pkgs.emptyFile` doesn't fulfil criteria 1 (it's defined in `pkgs/build-support/trivial-builders.nix`) and 2 (it's defined inline), so it can't be moved into `pkgs/unit` -- `pkgs.fetchFromGitHub` doesn't fulfil the criteria 3 (it evaluates to a function), so it can't be moved into `pkgs/unit` +- `pkgs.zsh` matches all criteria, its default Nix file is moved to `pkgs/unit/zsh/zsh/pkg-fun.nix` +- `pkgs.emptyFile` doesn't fulfil [criteria 1](#user-content-criteria-1) (it's defined in `pkgs/build-support/trivial-builders.nix`), so it can't be moved into `pkgs/unit` +- `pkgs.readline` doesn't fulfil [criteria 2](#user-content-criteria-2) (it's defined as an alias to `readline6`, which is itself defined as an alias to `readline63`) +- `pkgs.readline63` doesn't fulfil [criteria 3](#user-content-criteria-3) (it [transitively references](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/libraries/readline/6.3.nix#L23) `link-against-ncurses.patch`, which is [also referenced](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/libraries/readline/7.0.nix#L30) by the definition for `pkgs.readline70`) +- `pkgs.fetchFromGitHub` doesn't fulfil the [criteria 4](#user-content-criteria-4) (it evaluates to a function), so it can't be moved into `pkgs/unit` Here's how such a `pkgs/unit` directory structure would look like, note how all attribute names have the same level of nesting: ``` @@ -109,7 +95,7 @@ pkgs │ ├── autosuspend │ ┊ ┊ - ├── sl-- + ├── sl │ └── sl ┊ ├── slac @@ -117,7 +103,7 @@ pkgs │ ├── slack-cli │ └── slack-term ┊ - ├── zsh- + ├── zsh │ ├── zsh │ ├── zsh-autocomplete │ ├── zsh-completions @@ -127,7 +113,8 @@ pkgs # Interactions [interactions]: #interactions -- `nix edit` is unaffected, since it uses a packages `meta.position` to get the file to edit +- `nix edit` is unaffected, since it uses a packages `meta.position` to get the file to edit. + Though with this RFC `nix edit` could be updated to not have to rely on that anymore for the packages in the new hierarchy in nixpkgs. # Drawbacks [drawbacks]: #drawbacks @@ -138,8 +125,6 @@ pkgs - It was an inconvenient user interface, requiring a checkout or browsing through GitHub - Many packages fit multiple categories, leading to multiple locations to search through instead of one - There's other better ways of discovering similar packages, e.g. [Repology](https://repology.org/) -- Creating [symlinks](#symlinks) for the old paths has the potential to create merge conflicts between the symlink and the changed original file there. If the symlink wasn't there, GitHub could perform auto-merging. Counter-arguments: - - There's not too many such merge conflicts - This breaks `builtins.unsafeGetAttrPos "hello" pkgs`. Counter-arguments: - This functionality is unsafe and therefore breakages can be expected - Support for this can be added to Nix (make `builtins.readDir` propagate file as a position) @@ -148,9 +133,9 @@ pkgs [alternatives]: #alternatives - Use a flat directory `pkgs/unit/*/pkg-fun.nix` instead, arguments: + - Good because it's simpler, both for the user and for the code - Good because it speeds up Nix evaluation since there's only a single directory to call `builtins.readDir` on instead of many - With an optimized `readDir` this isn't much of a problem - - Good because it's simpler, both for the user and for the code - Bad because it causes GitHub to limit the rendering of that directory to 1'000 entries (and we have about 10'000 that benefit from this transition for a start) - Bad because it makes `git` slower ([TODO: By how much?](https://github.com/nixpkgs-architecture/simple-package-paths/issues/18)) @@ -165,8 +150,16 @@ pkgs For instance, having a package definition, changelog, package-specific config generator and perhaps even NixOS module in one directory makes work on the package in a broad sense easier. This is not a goal of this RFC, but a motivation to make this a future possibility. - - Use `unit` (at the nixpkgs root) instead of `pkgs/unit`. - This is future proof in case we want to make the directory structure more general purpose, but this is out of scope +- Use `unit` (at the nixpkgs root) instead of `pkgs/unit`. + This is future proof in case we want to make the directory structure more general purpose, but this is out of scope +- Additionally have a backwards-compatibility layer for moved paths, such as a symlink pointing from the old to the new location, or for Nix files even a `builtins.trace "deprecated" (import ../new/path)`. + We are not doing this because it would give precedent to file paths being a stable API interface, which definitely shouldn't be the case (bar some exceptions). + It would also lead to worse merge conflicts as the transition is happening, since Git would have to resolve a merge conflict between a symlink and a changed file. +- Loosen [criteria 3](#user-content-criteria-3), allowing certain packages to be moved to the new structure even if it requires updating references to paths in Nix files. + This isn't done because it [turns out](https://github.com/nixpkgs-architecture/simple-package-paths/issues/14) that this criteria indicates the file structure being used as an API interface. + By manually refactoring the Nix code to not rely on this anymore, you can increase code quality/reusability/clarity and then do the transition described in the RFC. +- Use a different sharding scheme than `<4-prefix name>`. + Discussions regarding this can be seen [here](https://github.com/nixpkgs-architecture/simple-package-paths/issues/1), [NAT meeting #18](https://github.com/nixpkgs-architecture/meetings/blob/6282b0c6bbc47b6f1becd155586c79728eddefc9/2022-11-21.md) and [here](https://github.com/nixpkgs-architecture/simple-package-paths/pull/20#discussion_r1029004083) # Unresolved questions [unresolved]: #unresolved-questions @@ -174,6 +167,8 @@ pkgs # Future work [future]: #future-work +All of these questions are in scope to be addressed in future discussions in the [Nixpkgs Architecture Team](https://nixos.org/community/teams/nixpkgs-architecture.html): + - This RFC only addresses the top-level attribute namespace, aka packages in `pkgs.`, it doesn't do anything about package sets like `pkgs.python3Packages.`, `pkgs.haskell.packages.ghc942.`, which could also benefit from a similar auto-calling - While this RFC doesn't address expressions where the second `callPackage` argument isn't `{}`, there is an easy way to transition to an argument of `{}`: For every attribute of the form `name = attrs.value;` in the argument, make sure `attrs` is in the arguments of the file, then add `name ? attrs.value` to the arguments. Then the expression in `all-packages.nix` can too be auto-called - Don't do this for `name = value` pairs though, that's an alias-like thing From 39062ac29c8f8300a0ece8c9c574d9f37254ee5f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 14 Dec 2022 23:18:33 +0100 Subject: [PATCH 3/9] More updates --- README.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 8740d43..f0de548 100644 --- a/README.md +++ b/README.md @@ -29,13 +29,16 @@ This makes it much easier to contribute new packages packages, since there's no - On GitHub this is even more problematic, as the `all-packages.nix` file is [too big to be displayed by GitHub](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/top-level/all-packages.nix) - Then go to that file's definition, which takes quite some time for navigation (unless you have a plugin that can jump to it directly) - It also slows down or even deadlocks editors due to the file size - - In some cases `nix edit` works, though that's not yet stable (it relies on Flakes being enabled) and comes with some problems ([doesn't allow the file to be edited](https://github.com/NixOS/nix/issues/3347), doesn't work with packages that don't set `meta.position` correctly). + - In some cases `nix edit` works, though that's not yet stable (it relies on Flakes being enabled) and comes with some problems ([doesn't yet open a writable file](https://github.com/NixOS/nix/issues/3347), doesn't work with packages that don't set `meta.position` correctly). - `all-packages.nix` frequently causes merge conflicts. It's a point of contention for all new packages # Detailed design [design]: #detailed-design -For all attributes at the root of nixpkgs `pkgs.` which satisfy these criteria: +Make a large part of `pkgs.` definitions in `all-packages.nix` eligible to be moved to `pkgs/unit/<4-letter name>/`. +The definition in `all-packages.nix` won't be necessary anymore, as all directories in `pkgs/unit/*/*` are automatically added to the `pkgs` set. + +The criteria for `pkgs.` becoming eligible are as follows: 1. Is defined in `pkgs/top-level/all-packages.nix` (necessary so that the overlay containing the automatically discovered packages can be ordered directly before the `all-packages.nix` overlay without changing any behavior) 2. Is defined to be equal to `pkgs.callPackage { }` @@ -45,14 +48,15 @@ For all attributes at the root of nixpkgs `pkgs.` which satisfy these crit 4. Evaluates to a derivation (necessary because using `pkg-fun.nix` for a non-package would be counter-intuitive) -These will be become eligible to be transformed as follows: -- Move the default Nix file from `` to `pkgs/unit/<4-prefix name>//pkg-fun.nix` ([TODO: Justify why `unit`](https://github.com/nixpkgs-architecture/simple-package-paths/issues/16)) +If all criteria are satisfied, the package becomes eligible for the following changes: +- Move the default Nix file from `` to `pkgs/unit/<4-prefix name>//pkg-fun.nix` - Where `<4-prefix name>` is the 4-letter prefix of ``, equal to `substring 0 4 name`. If `` has less than or exactly 4 characters, `<4-prefix name>` is equal to just ``. -- Additionally also move all paths transitively referenced by the default Nix file to `pkgs/unit/<4-prefix name>/` [TODO](https://github.com/nixpkgs-architecture/simple-package-paths/issues/19) + - The directory `unit` [was chosen](https://github.com/nixpkgs-architecture/simple-package-paths/issues/16) for a future vision where it could be its own top-level directory, not only containing package definitions for software components, but also related NixOS modules, library components, etc. +- Move all paths transitively referenced by the default Nix file to `pkgs/unit/<4-prefix name>/` - Remove the definition of that attribute in `pkgs/top-level/all-packages.nix` -These attributes will newly be added to `pkgs` by automatically calling `pkgs.callPackage pkgs/unit/<4-prefix name>//pkg-fun.nix { }` on all entries in `pkgs/unit`. In order to make this more efficient, `builtins.readDir` should be optimized as described [here](https://github.com/NixOS/nix/issues/7314). +These attributes will newly be added to `pkgs` by automatically calling `pkgs.callPackage pkgs/unit/<4-prefix name>//pkg-fun.nix { }` on all entries in `pkgs/unit`. In order to ensure efficiency of this operation, `builtins.readDir` should be optimized as described [here](https://github.com/NixOS/nix/issues/7314). ## Transitioning From 31fad775d64ba6960d3015d4a78911228c78c027 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 14 Dec 2022 23:21:06 +0100 Subject: [PATCH 4/9] Add mention of slow directory listings for a flat directory --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index f0de548..109990a 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,7 @@ pkgs - With an optimized `readDir` this isn't much of a problem - Bad because it causes GitHub to limit the rendering of that directory to 1'000 entries (and we have about 10'000 that benefit from this transition for a start) - Bad because it makes `git` slower ([TODO: By how much?](https://github.com/nixpkgs-architecture/simple-package-paths/issues/18)) + - Bad because directory listing slows down with many files - Don't use `pkg-fun.nix` but another file name: - `package.nix`/`pkg.nix`: Makes the migration to a non-function form of overridable packages harder in the future. From 41f087efac16ed37fdc54ae0de42cd10916f719f Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 16 Dec 2022 20:52:41 +0100 Subject: [PATCH 5/9] Apply suggestions from code review Co-authored-by: Robert Hensing --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 109990a..0a081ee 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ related-issues: (will contain links to implementation PRs) # Summary [summary]: #summary -Make trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` be auto-generated from a predictable attribute-based file hierarchy. +Auto-generate trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` from a sharded directory that matches the attribute name. This makes it much easier to contribute new packages packages, since there's no more guessing needed as to where the package should go, both in the ad-hoc directory categories and in `pkgs/top-level/all-packages.nix`. @@ -29,7 +29,7 @@ This makes it much easier to contribute new packages packages, since there's no - On GitHub this is even more problematic, as the `all-packages.nix` file is [too big to be displayed by GitHub](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/top-level/all-packages.nix) - Then go to that file's definition, which takes quite some time for navigation (unless you have a plugin that can jump to it directly) - It also slows down or even deadlocks editors due to the file size - - In some cases `nix edit` works, though that's not yet stable (it relies on Flakes being enabled) and comes with some problems ([doesn't yet open a writable file](https://github.com/NixOS/nix/issues/3347), doesn't work with packages that don't set `meta.position` correctly). + - `nix edit -f . package-attr` works, though that's not yet stable (it relies on the `nix-command` feature being enabled) and doesn't work with packages that don't set `meta.position` correctly). - `all-packages.nix` frequently causes merge conflicts. It's a point of contention for all new packages # Detailed design @@ -58,9 +58,9 @@ If all criteria are satisfied, the package becomes eligible for the following ch These attributes will newly be added to `pkgs` by automatically calling `pkgs.callPackage pkgs/unit/<4-prefix name>//pkg-fun.nix { }` on all entries in `pkgs/unit`. In order to ensure efficiency of this operation, `builtins.readDir` should be optimized as described [here](https://github.com/NixOS/nix/issues/7314). -## Transitioning +## Transition -This RFC comes with [a reference tool](https://github.com/nixpkgs-architecture/simple-package-paths/pull/22) to make the above transition in an automated way. +This RFC comes with [a tool](https://github.com/nixpkgs-architecture/simple-package-paths/pull/22) to make the above transition in an automated way. If this RFC is accepted, the result of that tool will be PR'd to nixpkgs. The tool itself will also be added to nixpkgs so that it can easily be ran again in the future. For at least one release cycle, the legacy way of declaring packages should still be accepted, but the tool can be ran again at any point, thereby moving those new packages from the legacy paths to the new `pkgs/unit` paths. From ba04b42ffa563b1db1791a07155f23b321b9ec70 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 19 Dec 2022 16:16:11 +0100 Subject: [PATCH 6/9] Updates from review Co-Authored-By: Robert Hensing --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0a081ee..dd15148 100644 --- a/README.md +++ b/README.md @@ -60,10 +60,10 @@ These attributes will newly be added to `pkgs` by automatically calling `pkgs.ca ## Transition -This RFC comes with [a tool](https://github.com/nixpkgs-architecture/simple-package-paths/pull/22) to make the above transition in an automated way. -If this RFC is accepted, the result of that tool will be PR'd to nixpkgs. -The tool itself will also be added to nixpkgs so that it can easily be ran again in the future. -For at least one release cycle, the legacy way of declaring packages should still be accepted, but the tool can be ran again at any point, thereby moving those new packages from the legacy paths to the new `pkgs/unit` paths. +This RFC comes with [a reference tool](https://github.com/nixpkgs-architecture/simple-package-paths/pull/22) to make the above transition in an automated way. +If this RFC is accepted, the result of that tool will be used to create a pull request to nixpkgs. +The tool itself will also be added to nixpkgs so that it can easily be run again in the future. +For at least one release cycle, the legacy way of declaring packages should still be accepted, but the tool can be run again at any point, thereby moving those new packages from the legacy paths to the new `pkgs/unit` paths. A CI action may also be implemented to help with this if deemed necessary. # Examples @@ -140,13 +140,13 @@ pkgs - Good because it's simpler, both for the user and for the code - Good because it speeds up Nix evaluation since there's only a single directory to call `builtins.readDir` on instead of many - With an optimized `readDir` this isn't much of a problem - - Bad because it causes GitHub to limit the rendering of that directory to 1'000 entries (and we have about 10'000 that benefit from this transition for a start) + - Bad because the GitHub web interface only renders the first 1'000 entries (and we have about 10'000 that benefit from this transition, even given the restrictions) - Bad because it makes `git` slower ([TODO: By how much?](https://github.com/nixpkgs-architecture/simple-package-paths/issues/18)) - Bad because directory listing slows down with many files - Don't use `pkg-fun.nix` but another file name: - - `package.nix`/`pkg.nix`: Makes the migration to a non-function form of overridable packages harder in the future. - - `default.nix`: + - `package.nix`/`pkg.nix`: Bad, because it makes the migration to a non-function form of overridable packages harder in the future. + - `default.nix`: Bad because: - Doesn't have its main benefits in this case: - Removing the need to specify the file name in expressions, but this does not apply because this file will be imported automatically by the code that replaces definitions from `all-packages.nix`. - Removing the need to specify the file name on the command line, but this does not apply because a package function must be imported into an expression before it can be used, making `nix build -f pkgs/unit/hell/hello` equally broken regardless of file name. @@ -174,7 +174,7 @@ pkgs All of these questions are in scope to be addressed in future discussions in the [Nixpkgs Architecture Team](https://nixos.org/community/teams/nixpkgs-architecture.html): -- This RFC only addresses the top-level attribute namespace, aka packages in `pkgs.`, it doesn't do anything about package sets like `pkgs.python3Packages.`, `pkgs.haskell.packages.ghc942.`, which could also benefit from a similar auto-calling +- This RFC only addresses the top-level attribute namespace, aka packages in `pkgs.`, it doesn't do anything about package sets like `pkgs.python3Packages.`, `pkgs.haskell.packages.ghc942.`, which may or may not also benefit from a similar auto-calling - While this RFC doesn't address expressions where the second `callPackage` argument isn't `{}`, there is an easy way to transition to an argument of `{}`: For every attribute of the form `name = attrs.value;` in the argument, make sure `attrs` is in the arguments of the file, then add `name ? attrs.value` to the arguments. Then the expression in `all-packages.nix` can too be auto-called - Don't do this for `name = value` pairs though, that's an alias-like thing - What to do with different versions, e.g. `wlroots = wlroots_0_14`? This goes into version resolution, a different problem to fix From 66540f0b6928793d84f850c151ffd4abb38bf1ae Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 19 Dec 2022 22:21:55 +0100 Subject: [PATCH 7/9] Rewrite detailed design without implementation details --- README.md | 65 +++++++++++++++---------------------------------------- 1 file changed, 17 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index dd15148..4384fce 100644 --- a/README.md +++ b/README.md @@ -35,55 +35,25 @@ This makes it much easier to contribute new packages packages, since there's no # Detailed design [design]: #detailed-design -Make a large part of `pkgs.` definitions in `all-packages.nix` eligible to be moved to `pkgs/unit/<4-letter name>/`. -The definition in `all-packages.nix` won't be necessary anymore, as all directories in `pkgs/unit/*/*` are automatically added to the `pkgs` set. - -The criteria for `pkgs.` becoming eligible are as follows: -1. Is defined in `pkgs/top-level/all-packages.nix` - (necessary so that the overlay containing the automatically discovered packages can be ordered directly before the `all-packages.nix` overlay without changing any behavior) -2. Is defined to be equal to `pkgs.callPackage { }` -3. All transitively referenced paths from the default Nix file of `` are under the same directory as the default Nix file and can be moved around together without breaking any references in other Nix files (except the one reference in `pkgs/top-level/all-packages.nix`). - This means that the Nix code should neither reference code outside, nor be referenced from outside. - (This is necessary so that no Nix code needs to be updated in the transition below) -4. Evaluates to a derivation - (necessary because using `pkg-fun.nix` for a non-package would be counter-intuitive) - -If all criteria are satisfied, the package becomes eligible for the following changes: -- Move the default Nix file from `` to `pkgs/unit/<4-prefix name>//pkg-fun.nix` - - Where `<4-prefix name>` is the 4-letter prefix of ``, equal to `substring 0 4 name`. - If `` has less than or exactly 4 characters, `<4-prefix name>` is equal to just ``. - - The directory `unit` [was chosen](https://github.com/nixpkgs-architecture/simple-package-paths/issues/16) for a future vision where it could be its own top-level directory, not only containing package definitions for software components, but also related NixOS modules, library components, etc. -- Move all paths transitively referenced by the default Nix file to `pkgs/unit/<4-prefix name>/` -- Remove the definition of that attribute in `pkgs/top-level/all-packages.nix` - -These attributes will newly be added to `pkgs` by automatically calling `pkgs.callPackage pkgs/unit/<4-prefix name>//pkg-fun.nix { }` on all entries in `pkgs/unit`. In order to ensure efficiency of this operation, `builtins.readDir` should be optimized as described [here](https://github.com/NixOS/nix/issues/7314). - -## Transition - -This RFC comes with [a reference tool](https://github.com/nixpkgs-architecture/simple-package-paths/pull/22) to make the above transition in an automated way. -If this RFC is accepted, the result of that tool will be used to create a pull request to nixpkgs. -The tool itself will also be added to nixpkgs so that it can easily be run again in the future. -For at least one release cycle, the legacy way of declaring packages should still be accepted, but the tool can be run again at any point, thereby moving those new packages from the legacy paths to the new `pkgs/unit` paths. -A CI action may also be implemented to help with this if deemed necessary. - -# Examples +This RFC establishes the convention of `pkgs/unit/${substring 0 4 name}/${name}` "unit" directories for the definitions of the Nix packages `pkgs.${name}` in nixpkgs. +The `pkg-fun.nix` files in all unit directories are automatically discovered, called using `pkgs.callPackage` and added to the `pkgs` set. + +These requirements will be checked using CI: +1. The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${substring 0 4 name}/${name}`. +2. Files outside a unit directory must not reference files inside that unit directory, and the other way around. +3. Each unit directory must have a `pkg-fun.nix` file such that `pkgs.callPackage ./pkg-fun.nix {}` evaluates to a derivation. +4. Packages defined by unit directories must not be defined or overridden anywhere else, such as in `pkgs/top-level/all-packages.nix`. + +This convention is optional, but it will be applied to all existing packages where possible. Nixpkgs reviewers may encourage contributors to use this convention without enforcing it. + +## Examples [examples]: #examples -- `pkgs.hello` matches all criteria: - The default Nix file [`pkgs/applications/misc/hello/default.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/applications/misc/hello/default.nix) only transitively [references `test.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/applications/misc/hello/default.nix#L31) in the same directory. - Neither the `default.nix` nor `test.nix` is referenced by any other file in nixpkgs, so we can do the transformation: - - Move `pkgs/applications/misc/hello/default.nix` to `pkgs/unit/hell/hello/pkg-fun.nix` - - Move `pkgs/applications/misc/hello/test.nix` to `pkgs/unit/hell/hello/test.nix` -- `pkgs.gnumake` matches all criteria: - The default Nix file [`pkgs/development/tools/build-managers/gnumake/default.nix`](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/tools/build-managers/gnumake/default.nix) transitively references only files in its own directory and no other files in nixpkgs reference `gnumake`'s files, so we can do the transformation by moving all the files from `pkgs/development/tools/build-managers/gnumake` to `pkgs/unit/gnum/gnumake`, the default Nix file ending up in `pkgs/unit/gnum/gnumake/pkg-fun.nix`. -- Similarly `pkgs.gnumake42` in [`pkgs/development/tools/build-managers/gnumake/4.2/default.nix`](https://github.com/NixOS/nixpkgs/tree/nixos-22.05/pkgs/development/tools/build-managers/gnumake/4.2/default.nix) fulfils all criteria, even though its directory is nested in `pkgs.gnumake`'s directory, they don't reference each others files. -- `pkgs.zsh` matches all criteria, its default Nix file is moved to `pkgs/unit/zsh/zsh/pkg-fun.nix` -- `pkgs.emptyFile` doesn't fulfil [criteria 1](#user-content-criteria-1) (it's defined in `pkgs/build-support/trivial-builders.nix`), so it can't be moved into `pkgs/unit` -- `pkgs.readline` doesn't fulfil [criteria 2](#user-content-criteria-2) (it's defined as an alias to `readline6`, which is itself defined as an alias to `readline63`) -- `pkgs.readline63` doesn't fulfil [criteria 3](#user-content-criteria-3) (it [transitively references](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/libraries/readline/6.3.nix#L23) `link-against-ncurses.patch`, which is [also referenced](https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/libraries/readline/7.0.nix#L30) by the definition for `pkgs.readline70`) -- `pkgs.fetchFromGitHub` doesn't fulfil the [criteria 4](#user-content-criteria-4) (it evaluates to a function), so it can't be moved into `pkgs/unit` - -Here's how such a `pkgs/unit` directory structure would look like, note how all attribute names have the same level of nesting: +To add a new package `pkgs.foobar` to nixpkgs, one only needs to create the file `pkgs/unit/foob/foobar/pkg-fun.nix`. +No need to find an appropriate category nor to modify `pkgs/top-level/all-packages.nix` anymore. + +With many packages, the `pkgs/unit` directory may look like this: + ``` pkgs └── unit @@ -118,7 +88,6 @@ pkgs [interactions]: #interactions - `nix edit` is unaffected, since it uses a packages `meta.position` to get the file to edit. - Though with this RFC `nix edit` could be updated to not have to rely on that anymore for the packages in the new hierarchy in nixpkgs. # Drawbacks [drawbacks]: #drawbacks From e35cc4c526c8a3aad9ed131624733bfc80aafcb3 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 19 Dec 2022 23:05:04 +0100 Subject: [PATCH 8/9] Restructure and reword alternatives section --- README.md | 64 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 4384fce..859e4c5 100644 --- a/README.md +++ b/README.md @@ -39,10 +39,10 @@ This RFC establishes the convention of `pkgs/unit/${substring 0 4 name}/${name}` The `pkg-fun.nix` files in all unit directories are automatically discovered, called using `pkgs.callPackage` and added to the `pkgs` set. These requirements will be checked using CI: -1. The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${substring 0 4 name}/${name}`. -2. Files outside a unit directory must not reference files inside that unit directory, and the other way around. -3. Each unit directory must have a `pkg-fun.nix` file such that `pkgs.callPackage ./pkg-fun.nix {}` evaluates to a derivation. -4. Packages defined by unit directories must not be defined or overridden anywhere else, such as in `pkgs/top-level/all-packages.nix`. +1. The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${substring 0 4 name}/${name}`. +2. Files outside a unit directory must not reference files inside that unit directory, and the other way around. +3. Each unit directory must have a `pkg-fun.nix` file such that `pkgs.callPackage ./pkg-fun.nix {}` evaluates to a derivation. +4. Packages defined by unit directories must not be defined or overridden anywhere else, such as in `pkgs/top-level/all-packages.nix`. This convention is optional, but it will be applied to all existing packages where possible. Nixpkgs reviewers may encourage contributors to use this convention without enforcing it. @@ -105,35 +105,51 @@ pkgs # Alternatives [alternatives]: #alternatives -- Use a flat directory `pkgs/unit/*/pkg-fun.nix` instead, arguments: +## Alternate `pkgs/unit` structure + +- Use a flat directory, e.g. `pkgs.hello` would be in `pkgs/unit/hell/hello`. - Good because it's simpler, both for the user and for the code - Good because it speeds up Nix evaluation since there's only a single directory to call `builtins.readDir` on instead of many - With an optimized `readDir` this isn't much of a problem - Bad because the GitHub web interface only renders the first 1'000 entries (and we have about 10'000 that benefit from this transition, even given the restrictions) - Bad because it makes `git` slower ([TODO: By how much?](https://github.com/nixpkgs-architecture/simple-package-paths/issues/18)) - Bad because directory listing slows down with many files - -- Don't use `pkg-fun.nix` but another file name: - - `package.nix`/`pkg.nix`: Bad, because it makes the migration to a non-function form of overridable packages harder in the future. - - `default.nix`: Bad because: - - Doesn't have its main benefits in this case: - - Removing the need to specify the file name in expressions, but this does not apply because this file will be imported automatically by the code that replaces definitions from `all-packages.nix`. - - Removing the need to specify the file name on the command line, but this does not apply because a package function must be imported into an expression before it can be used, making `nix build -f pkgs/unit/hell/hello` equally broken regardless of file name. - - Not using `default.nix` frees up `default.nix` for a short expression that is actually buildable, e.g. `(import ../..).hello`. - - Choosing `default.nix` would bias the purpose of the `unit` directory to serve only as package definitions, whereas we could make the tree more human friendly by grouping files together by "topic" rather than by technical delineations. - For instance, having a package definition, changelog, package-specific config generator and perhaps even NixOS module in one directory makes work on the package in a broad sense easier. - This is not a goal of this RFC, but a motivation to make this a future possibility. +- Use only the 1-, 2- or 3-prefix instead of the 4-prefix name. This was not done because it still leads to directories in `pkgs/unit` containing more than 1'000 entries, leading to the same problems. +- Use multi-level structure, like a 2-level 2-prefix structure where `hello` is in `pkgs/unit/he/ll/hello`, + if packages are less than 4 characters long, we will it out with `-`, e.g. `z` is in `pkgs/unit/z-/--/z`. + This is not great because it's more complicated and it would improve git performance only marginally. +- Use a dynamic structure where directories are rebalanced when they have too many entries. + E.g. `pkgs.foobar` could be in `pkgs/unit/f/foobar` initially. + But when there's more than 1'000 packages starting with `f`, all packages starting with `f` are distributed under 2-letter prefixes, moving `foobar` to `pkgs/unit/fo/foobar`. + This is not great because it's very complex to determine which directory to put a package in, making it bad for contributors. + +## Alternate `pkg-fun.nix` filename + +- `default.nix`: Bad because: + - Doesn't have its main benefits in this case: + - Removing the need to specify the file name in expressions, but this does not apply because this file will be imported automatically by the code that replaces definitions from `all-packages.nix`. + - Removing the need to specify the file name on the command line, but this does not apply because a package function must be imported into an expression before it can be used, making `nix build -f pkgs/unit/hell/hello` equally broken regardless of file name. + - Not using `default.nix` frees up `default.nix` for a short expression that is actually buildable, e.g. `(import ../..).hello`. + - Choosing `default.nix` would bias the purpose of the `unit` directory to serve only as package definitions, whereas we could make the tree more human friendly by grouping files together by "topic" rather than by technical delineations. + For instance, having a package definition, changelog, package-specific config generator and perhaps even NixOS module in one directory makes work on the package in a broad sense easier. + This is not a goal of this RFC, but a motivation to make this a future possibility. +- `package.nix`/`pkg.nix`: Bad, because it makes the migration to a non-function form of overridable packages harder in the future. + +## Alternate `pkgs/unit` location - Use `unit` (at the nixpkgs root) instead of `pkgs/unit`. This is future proof in case we want to make the directory structure more general purpose, but this is out of scope -- Additionally have a backwards-compatibility layer for moved paths, such as a symlink pointing from the old to the new location, or for Nix files even a `builtins.trace "deprecated" (import ../new/path)`. - We are not doing this because it would give precedent to file paths being a stable API interface, which definitely shouldn't be the case (bar some exceptions). - It would also lead to worse merge conflicts as the transition is happening, since Git would have to resolve a merge conflict between a symlink and a changed file. -- Loosen [criteria 3](#user-content-criteria-3), allowing certain packages to be moved to the new structure even if it requires updating references to paths in Nix files. - This isn't done because it [turns out](https://github.com/nixpkgs-architecture/simple-package-paths/issues/14) that this criteria indicates the file structure being used as an API interface. - By manually refactoring the Nix code to not rely on this anymore, you can increase code quality/reusability/clarity and then do the transition described in the RFC. -- Use a different sharding scheme than `<4-prefix name>`. - Discussions regarding this can be seen [here](https://github.com/nixpkgs-architecture/simple-package-paths/issues/1), [NAT meeting #18](https://github.com/nixpkgs-architecture/meetings/blob/6282b0c6bbc47b6f1becd155586c79728eddefc9/2022-11-21.md) and [here](https://github.com/nixpkgs-architecture/simple-package-paths/pull/20#discussion_r1029004083) +- Other name proposals were deemed worse: `pkg`, `component`, `part`, `mod`, `comp` + +## Filepath backwards-compatibility + +Additionally have a backwards-compatibility layer for moved paths, such as a symlink pointing from the old to the new location, or for Nix files even a `builtins.trace "deprecated" (import ../new/path)`. +- We are not doing this because it would give precedent to file paths being a stable API interface, which definitely shouldn't be the case (bar some exceptions). +- It would also lead to worse merge conflicts as the transition is happening, since Git would have to resolve a merge conflict between a symlink and a changed file. + +## Not having the [reference requirement](#user-content-req-ref) + +The reference requirement could be removed, which would allow unit directories to reference files outside themselves, and the other way around. This is not great because it encourages the use of file paths as an API, rather than explicitly exposing functionality from Nix expressions. # Unresolved questions [unresolved]: #unresolved-questions From f9fdece806c87215116c501b2ada49c6e6d7a682 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 2 Jan 2023 17:21:18 +0100 Subject: [PATCH 9/9] Apply suggestions from code review Discussed during NAT meeting #23 Co-authored-by: John Ericson --- README.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 859e4c5..2c27b11 100644 --- a/README.md +++ b/README.md @@ -41,8 +41,8 @@ The `pkg-fun.nix` files in all unit directories are automatically discovered, ca These requirements will be checked using CI: 1. The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${substring 0 4 name}/${name}`. 2. Files outside a unit directory must not reference files inside that unit directory, and the other way around. -3. Each unit directory must have a `pkg-fun.nix` file such that `pkgs.callPackage ./pkg-fun.nix {}` evaluates to a derivation. -4. Packages defined by unit directories must not be defined or overridden anywhere else, such as in `pkgs/top-level/all-packages.nix`. +4. The definition of a package in the unit directory is the one `pkgs.` points to. +5. To avoid problems with merges, if a package attribute is defined by a unit directory, it must not be defined in `pkgs/top-level/all-packages.nix` or `pkgs/top-level/aliases.nix`. This convention is optional, but it will be applied to all existing packages where possible. Nixpkgs reviewers may encourage contributors to use this convention without enforcing it. @@ -151,6 +151,16 @@ Additionally have a backwards-compatibility layer for moved paths, such as a sym The reference requirement could be removed, which would allow unit directories to reference files outside themselves, and the other way around. This is not great because it encourages the use of file paths as an API, rather than explicitly exposing functionality from Nix expressions. +## Relax design to try to attack issues like "package variants" up front + +An issue with restrictions like the above one is that they don't work well for when we package a number of variants of package, e.g. different versions of the same package that share some infra. We do presume we would have to have *some* notion of "private details shared between multiple units" or "multiple entry points to unit" to handle these cases. + +We've chosen to explicitly ignore these tough cases, and emphasize uniform structure of units over being able to migrate over as many packages as possible from the get go. The rationale for this decision is basically: + +1. It is (a bit) easier to relax requirements later than tighten them later. +2. We plan on incrementally migrating Nixpkgs to this new system anyways, for caution's sake, so starting with fewer units is not only fine but *good*. +3. Explicitly marking use-cases out of scope allows us to have a more focused and thorough investigation of the use-cases that remain, to build a solid foundation. + # Unresolved questions [unresolved]: #unresolved-questions