From 3ca531ac11dcfbdedd963b7e2f1ed31516c75df9 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 30 Jan 2023 19:02:15 +0100 Subject: [PATCH 01/25] RFC 140 Initialized from https://github.com/nixpkgs-architecture/simple-package-paths/commit/01948e0551d7e0178ff5d8db83653129da49b84b --- rfcs/0140-simple-package-paths.md | 242 ++++++++++++++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 rfcs/0140-simple-package-paths.md diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md new file mode 100644 index 000000000..3fca36e67 --- /dev/null +++ b/rfcs/0140-simple-package-paths.md @@ -0,0 +1,242 @@ +--- +feature: simple-package-paths +start-date: 2022-09-02 +author: Silvan Mosberger +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: https://github.com/NixOS/nixpkgs/pull/211832 +--- + +# Summary +[summary]: #summary + +Auto-generate trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` from a directory structure 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`. + +# Motivation +[motivation]: #motivation + +- (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 + - 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 + - `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 +[design]: #detailed-design + +This RFC establishes the standard of using `pkgs/unit/${shard}/${name}` "unit" directories for the definitions of the Nix packages `pkgs.${name}` in nixpkgs, where `shard = toLower (substring 0 2 name)`. +All unit directories are automatically discovered and incorporated into the `pkgs` set using `pkgs.${name} = pkgs.callPackage pkgs/unit/${shard}/${name}/pkg-fun.nix { }`. + +The following requirements will be checked by CI. +This standard must be followed for newly added packages that can satisfy these requirements. +A treewide migration to this standard will be performed for existing packages that can satisfy these requirements. + +## Structure + +The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${shard}/${name}`. +Each unit directory must contain at least a `pkg-fun.nix` file, but may contain arbitrary other files and directories. + +This ensures that maintainers don't have to verify this structure manually, which is prone to mistakes. + +## Only derivations + +If `pkgs/unit/${shard}/${name}` exists, `pkgs.${name}` must be a derivation that can be built directly with `nix-build`. + +This ensures that people can expect the unit directories to correspond to buildable packages and not functions like `pkgs.fetchFromGitHub` or `pkgs.buildRustCrate`. + +## Stable boundary + +Unit directories may only interact with the rest of nixpkgs via the stable `pkgs.${name}` attributes, not with file references: +- Files inside a unit directory must not reference files outside that unit directory. + Therefore all dependencies on other packages must come from `pkg-fun.nix` arguments injected by `callPackage`. + This ensures that files in nixpkgs can be moved around without breaking this package. +- Files outside a unit directory must not reference files inside that unit directory. + Therefore other packages can only depend on this package via `pkgs.${name}`. + This ensures that files within unit directories (except `pkg-fun.nix`) can be freely moved and changed without breaking any other packages. + +The only notable exception to this rule is the `pkgs/top-level/all-packages.nix` file which may reference the `pkg-fun.nix` file according to the next requirement. + +## Custom arguments + +If `pkgs/top-level/all-packages.nix` contains a definition for the attribute `${name}` and the unit directory `pkgs/unit/${shard}/${name}` exists, then the attribute value must be defined as `pkgs.callPackage pkgs/unit/${shard}/${name}/pkg-fun.nix args`, where `args` may be a freely chosen expression. + +This ensures that even if a package initially doesn't require a custom `args`, if it later does, it doesn't have to be moved out of the `pkgs/unit` directory to pass custom arguments. + +## Examples +[examples]: #examples + +To add a new package `pkgs.foobar` to nixpkgs, one only needs to create the file `pkgs/unit/fo/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 + ├── _0 + │ ├── _0verkill + │ └── _0x + ┊ + ├── ch + │ ├── ChowPhaser + │ ├── CHOWTapeModel + │ ├── chroma + │ ┊ + ┊ + ├── t + │ └── t + ┊ +``` + +# Interactions +[interactions]: #interactions + +- `nix edit` and search.nixos.org are unaffected, since they rely on `meta.position` to get the file to edit, which still works +- `git blame` locally and on GitHub is unaffected, since it follows file renames properly. +- A commonly recommended way of building package directories in nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. + Since the path changes `pkg-fun.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/pkg-fun.nix {}'`, which is harder for users. + However, calling a path like this is an anti-pattern anyways, because it doesn't use the correct nixpkgs version and it doesn't use the correct argument overrides. + The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. + This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. + By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. + +# Drawbacks +[drawbacks]: #drawbacks + +- The existing categorization of packages gets lost. Counter-arguments: + - It was never that useful to begin with + - The categorization was always incomplete, because packages defined in the language package sets often don't get their own categorized file path. + - 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/) +- This breaks `builtins.unsafeGetAttrPos "hello" pkgs`. Counter-arguments: + - We have to draw a line as to what constitutes the public interface of Nixpkgs. We have decided that making attribute position information part of that is not productive. For context, this information is already accepted to be unreliable at the language level, noting the `unsafe` part of the name. + - Support for this could be added to Nix (make `builtins.readDir` propagate file as a position) + +# Alternatives +[alternatives]: #alternatives + +## Alternate `pkgs/unit` structure + +- Use a flat directory, e.g. `pkgs.hello` would be in `pkgs/unit/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 +- Use `substring 0 3 name` or `substring 0 4 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`, although at that point it might better be auto-generated or implicit in the CLI +- `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 +- 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. + +## Restrict design to try delay issues like "package variants" {#no-variants} + +We perceived some uncertainty around [package variants](#def-package-variant) that led us to scope these out at first, but we did not identify a real problem that would arise from allowing non-auto-called attributes to reference `pkgs/unit` files. However, imposing unnecessary restrictions would be counterproductive because: + + - The contributor experience would suffer, because it won't be obvious to everyone whether their package is allowed to go into `pkgs/unit`. This means that we'd fail to solve the goal "Which directory should my package definition go in?", leading to unnecessary requests for changes in pull requests. + + - Changes in dependencies can require dependents to add an override, causing packages to be moved back and forth between unit directories and the general `pkgs` tree, worsening the problem as people have to decide categories *again*. + + - When lifting the restriction, the reviewers have to adapt, again leading to unnecessary requests for changes in pull requests. + + - We'd be protracting the migration by unnecessary gatekeeping or discovering some problem late. + +That said, we did identify risks: + + - We might get something wrong, and while we plan to incrementally migrate Nixpkgs to this new system anyway, starting with fewer units is good. + - Mitigation: only automate the renames of simple (`callPackage path { }`) calls, to keep the initial change small + + - We might not focus enough on the foundation, while we could more easily relax requirements later. + - After more discussion, we feel confident that the manual `callPackage` calls are unlikely to cause issues that we wouldn't otherwise have. + +# Recommend a `callPackage` pattern with default arguments + +> - 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 + +`callPackage` does not favor the default argument when both a default argument and a value in `pkgs` exist. Changing the semantics of `callPackage` is out of scope. + +# Allow `callPackage` arguments to be specified in `/args.nix` + +The idea was to expand the auto-calling logic according to: + +Unit directories are automatically discovered and transformed to a definition of the form +``` +# If args.nix doesn't exist +pkgs.${name} = pkgs.callPackage ${unitDir}/pkg-fun.nix {} +# If args.nix does exists +pkgs.${name} = pkgs.callPackage ${unitDir}/pkg-fun.nix (import ${unitDir}/args.nix pkgs); +``` + +Pro: + - It makes another class of packages uniform, by picking a solution with restricted expressive power. + +Con: + - It does not solve the contributor experience problem of having to many rules. + - `args.nix` is another pattern that contributors need to learn how to use, as we have seen that it is not immediately obvious to everyone how it works. + - A CI check can mitigate the possible lack of uniformity, and we see a simple implementation strategy for it. + - This keeps the contents of the unit directories simple and a bit more uniform than with optional `args.nix` files. + +# Unresolved questions +[unresolved]: #unresolved-questions + +# 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): + +- Making the filetree more human-friendly by grouping files together by "topic" rather than 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 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 +- Improve the semantics of `callPackage` and/or apply a better solution, such as a module-like solution +- What to do with different versions, e.g. `wlroots = wlroots_0_14`? This goes into version resolution, a different problem to fix +- What to do about e.g. `libsForQt5.callPackage`? This goes into overrides, a different problem to fix +- What about aliases like `jami-daemon = jami.jami-daemon`? +- What about `recurseIntoAttrs`? Not single packages, package sets, another problem + +# Definitions + + - *variant attribute*: an attribute that defines a package by invoking it with non-default arguments, for example: + ``` + graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; + ``` From 269ac4f669302c9b09075a40632b664158908a0b Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 30 Jan 2023 18:59:16 +0100 Subject: [PATCH 02/25] Minor improvements --- rfcs/0140-simple-package-paths.md | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 3fca36e67..aae229490 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -105,7 +105,7 @@ pkgs - `git blame` locally and on GitHub is unaffected, since it follows file renames properly. - A commonly recommended way of building package directories in nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. Since the path changes `pkg-fun.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/pkg-fun.nix {}'`, which is harder for users. - However, calling a path like this is an anti-pattern anyways, because it doesn't use the correct nixpkgs version and it doesn't use the correct argument overrides. + However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct nixpkgs version and it doesn't use the correct argument overrides. The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. @@ -130,15 +130,12 @@ pkgs - Use a flat directory, e.g. `pkgs.hello` would be in `pkgs/unit/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 + - Bad because it makes `git` and file listings slower - Use `substring 0 3 name` or `substring 0 4 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. + This is not great because it's more complicated, longer to type and it would improve 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`. @@ -146,11 +143,13 @@ pkgs ## Alternate `pkg-fun.nix` filename -- `default.nix`: Bad because: - - Doesn't have its main benefits in this case: +- `default.nix`: + - Bad because it doesn't have its main benefits here: - 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`, although at that point it might better be auto-generated or implicit in the CLI + - Not using `default.nix` frees up `default.nix` for a short expression that is actually buildable, e.g. `(import ../.. {}).hello`, although we don't yet have a use case for this that isn't covered by `nix-build ../.. -A $attrname`. + - Bad because using `default.nix` would tempt users to invoke `nix-build .` whereas making package functions auto-callable is a known anti-pattern as it duplicates the defaults. + - Good because `default.nix` is already a convention most people are used to - `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 @@ -189,14 +188,14 @@ That said, we did identify risks: - We might not focus enough on the foundation, while we could more easily relax requirements later. - After more discussion, we feel confident that the manual `callPackage` calls are unlikely to cause issues that we wouldn't otherwise have. -# Recommend a `callPackage` pattern with default arguments +## Recommend a `callPackage` pattern with default arguments > - 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 `callPackage` does not favor the default argument when both a default argument and a value in `pkgs` exist. Changing the semantics of `callPackage` is out of scope. -# Allow `callPackage` arguments to be specified in `/args.nix` +## Allow `callPackage` arguments to be specified in `/args.nix` The idea was to expand the auto-calling logic according to: @@ -233,10 +232,3 @@ All of these questions are in scope to be addressed in future discussions in the - What to do about e.g. `libsForQt5.callPackage`? This goes into overrides, a different problem to fix - What about aliases like `jami-daemon = jami.jami-daemon`? - What about `recurseIntoAttrs`? Not single packages, package sets, another problem - -# Definitions - - - *variant attribute*: an attribute that defines a package by invoking it with non-default arguments, for example: - ``` - graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; - ``` From 21c3493a2b64e5987515d45e5590ee5c6429348c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 2 Feb 2023 19:55:30 +0100 Subject: [PATCH 03/25] Minor nits --- rfcs/0140-simple-package-paths.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index aae229490..e75d99643 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -12,7 +12,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/211832 [summary]: #summary Auto-generate trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` from a directory structure 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`. +This makes it much easier to contribute new 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 @@ -27,14 +27,14 @@ 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 + - It also slows down or even locks up editors due to the file size - `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 [design]: #detailed-design -This RFC establishes the standard of using `pkgs/unit/${shard}/${name}` "unit" directories for the definitions of the Nix packages `pkgs.${name}` in nixpkgs, where `shard = toLower (substring 0 2 name)`. +This RFC establishes the standard of using `pkgs/unit/${shard}/${name}` "unit" directories for the definitions of the Nix packages `pkgs.${name}` in Nixpkgs, where `shard = toLower (substring 0 2 name)`. All unit directories are automatically discovered and incorporated into the `pkgs` set using `pkgs.${name} = pkgs.callPackage pkgs/unit/${shard}/${name}/pkg-fun.nix { }`. The following requirements will be checked by CI. @@ -56,10 +56,10 @@ This ensures that people can expect the unit directories to correspond to builda ## Stable boundary -Unit directories may only interact with the rest of nixpkgs via the stable `pkgs.${name}` attributes, not with file references: +Unit directories may only interact with the rest of Nixpkgs via the stable `pkgs.${name}` attributes, not with file references: - Files inside a unit directory must not reference files outside that unit directory. Therefore all dependencies on other packages must come from `pkg-fun.nix` arguments injected by `callPackage`. - This ensures that files in nixpkgs can be moved around without breaking this package. + This ensures that files in Nixpkgs can be moved around without breaking this package. - Files outside a unit directory must not reference files inside that unit directory. Therefore other packages can only depend on this package via `pkgs.${name}`. This ensures that files within unit directories (except `pkg-fun.nix`) can be freely moved and changed without breaking any other packages. @@ -75,7 +75,7 @@ This ensures that even if a package initially doesn't require a custom `args`, i ## Examples [examples]: #examples -To add a new package `pkgs.foobar` to nixpkgs, one only needs to create the file `pkgs/unit/fo/foobar/pkg-fun.nix`. +To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/unit/fo/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: @@ -103,9 +103,9 @@ pkgs - `nix edit` and search.nixos.org are unaffected, since they rely on `meta.position` to get the file to edit, which still works - `git blame` locally and on GitHub is unaffected, since it follows file renames properly. -- A commonly recommended way of building package directories in nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. +- A commonly recommended way of building package directories in Nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. Since the path changes `pkg-fun.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/pkg-fun.nix {}'`, which is harder for users. - However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct nixpkgs version and it doesn't use the correct argument overrides. + However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. @@ -130,15 +130,15 @@ pkgs - Use a flat directory, e.g. `pkgs.hello` would be in `pkgs/unit/hello`. - Good because it's simpler, both for the user and for the code - - 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 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` and file listings slower -- Use `substring 0 3 name` or `substring 0 4 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 `substring 0 3 name` or `substring 0 4 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, longer to type and it would improve 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`. + 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 @@ -154,7 +154,7 @@ pkgs ## Alternate `pkgs/unit` location -- Use `unit` (at the nixpkgs root) instead of `pkgs/unit`. +- 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 - Other name proposals were deemed worse: `pkg`, `component`, `part`, `mod`, `comp` @@ -211,7 +211,7 @@ Pro: - It makes another class of packages uniform, by picking a solution with restricted expressive power. Con: - - It does not solve the contributor experience problem of having to many rules. + - It does not solve the contributor experience problem of having too many rules. - `args.nix` is another pattern that contributors need to learn how to use, as we have seen that it is not immediately obvious to everyone how it works. - A CI check can mitigate the possible lack of uniformity, and we see a simple implementation strategy for it. - This keeps the contents of the unit directories simple and a bit more uniform than with optional `args.nix` files. From e203231108e5f3d7e83ae762d4a636119ec6e949 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 27 Mar 2023 17:52:00 +0200 Subject: [PATCH 04/25] Update co-authors and add pre-RFC reviewers --- rfcs/0140-simple-package-paths.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index e75d99643..95d3d9bc6 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -2,7 +2,8 @@ feature: simple-package-paths start-date: 2022-09-02 author: Silvan Mosberger -co-authors: Nixpkgs Architecture Team +co-authors: Robert Hensing +pre-RFC reviewers: Thomas Bereknyei, John Ericson, Alex Ameen shepherd-team: (names, to be nominated and accepted by RFC steering committee) shepherd-leader: (name to be appointed by RFC steering committee) related-issues: https://github.com/NixOS/nixpkgs/pull/211832 From 967beb129b463a6096f1d3c74bc3d37343439904 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 27 Mar 2023 21:06:24 +0200 Subject: [PATCH 05/25] pkg-fun.nix -> package.nix --- rfcs/0140-simple-package-paths.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 95d3d9bc6..9af660edd 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -36,7 +36,7 @@ This makes it much easier to contribute new packages, since there's no more gues [design]: #detailed-design This RFC establishes the standard of using `pkgs/unit/${shard}/${name}` "unit" directories for the definitions of the Nix packages `pkgs.${name}` in Nixpkgs, where `shard = toLower (substring 0 2 name)`. -All unit directories are automatically discovered and incorporated into the `pkgs` set using `pkgs.${name} = pkgs.callPackage pkgs/unit/${shard}/${name}/pkg-fun.nix { }`. +All unit directories are automatically discovered and incorporated into the `pkgs` set using `pkgs.${name} = pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix { }`. The following requirements will be checked by CI. This standard must be followed for newly added packages that can satisfy these requirements. @@ -45,7 +45,7 @@ A treewide migration to this standard will be performed for existing packages th ## Structure The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${shard}/${name}`. -Each unit directory must contain at least a `pkg-fun.nix` file, but may contain arbitrary other files and directories. +Each unit directory must contain at least a `package.nix` file, but may contain arbitrary other files and directories. This ensures that maintainers don't have to verify this structure manually, which is prone to mistakes. @@ -59,24 +59,24 @@ This ensures that people can expect the unit directories to correspond to builda Unit directories may only interact with the rest of Nixpkgs via the stable `pkgs.${name}` attributes, not with file references: - Files inside a unit directory must not reference files outside that unit directory. - Therefore all dependencies on other packages must come from `pkg-fun.nix` arguments injected by `callPackage`. + Therefore all dependencies on other packages must come from `package.nix` arguments injected by `callPackage`. This ensures that files in Nixpkgs can be moved around without breaking this package. - Files outside a unit directory must not reference files inside that unit directory. Therefore other packages can only depend on this package via `pkgs.${name}`. - This ensures that files within unit directories (except `pkg-fun.nix`) can be freely moved and changed without breaking any other packages. + This ensures that files within unit directories (except `package.nix`) can be freely moved and changed without breaking any other packages. -The only notable exception to this rule is the `pkgs/top-level/all-packages.nix` file which may reference the `pkg-fun.nix` file according to the next requirement. +The only notable exception to this rule is the `pkgs/top-level/all-packages.nix` file which may reference the `package.nix` file according to the next requirement. ## Custom arguments -If `pkgs/top-level/all-packages.nix` contains a definition for the attribute `${name}` and the unit directory `pkgs/unit/${shard}/${name}` exists, then the attribute value must be defined as `pkgs.callPackage pkgs/unit/${shard}/${name}/pkg-fun.nix args`, where `args` may be a freely chosen expression. +If `pkgs/top-level/all-packages.nix` contains a definition for the attribute `${name}` and the unit directory `pkgs/unit/${shard}/${name}` exists, then the attribute value must be defined as `pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix args`, where `args` may be a freely chosen expression. This ensures that even if a package initially doesn't require a custom `args`, if it later does, it doesn't have to be moved out of the `pkgs/unit` directory to pass custom arguments. ## Examples [examples]: #examples -To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/unit/fo/foobar/pkg-fun.nix`. +To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/unit/fo/foobar/package.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: @@ -105,7 +105,7 @@ pkgs - `nix edit` and search.nixos.org are unaffected, since they rely on `meta.position` to get the file to edit, which still works - `git blame` locally and on GitHub is unaffected, since it follows file renames properly. - A commonly recommended way of building package directories in Nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. - Since the path changes `pkg-fun.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/pkg-fun.nix {}'`, which is harder for users. + Since the path changes `package.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. @@ -142,7 +142,7 @@ pkgs 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 +## Alternate `package.nix` filename - `default.nix`: - Bad because it doesn't have its main benefits here: @@ -203,9 +203,9 @@ The idea was to expand the auto-calling logic according to: Unit directories are automatically discovered and transformed to a definition of the form ``` # If args.nix doesn't exist -pkgs.${name} = pkgs.callPackage ${unitDir}/pkg-fun.nix {} +pkgs.${name} = pkgs.callPackage ${unitDir}/package.nix {} # If args.nix does exists -pkgs.${name} = pkgs.callPackage ${unitDir}/pkg-fun.nix (import ${unitDir}/args.nix pkgs); +pkgs.${name} = pkgs.callPackage ${unitDir}/package.nix (import ${unitDir}/args.nix pkgs); ``` Pro: From b077e2f7c1bb160fcb55fe215e33240eb26c7549 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 18 Apr 2023 00:04:06 +0200 Subject: [PATCH 06/25] Mid-sized refactor for improved clarity and incorporating feedback In addition to some more minor changes and incorporating feedback, the major changes are: - Restructure the RFC into two separate parts, one to introduce the convention and one to migrate packages to it when possible - Remove the restriction that files inside a unit directory can only be referenced by the corresponding `pkgs.${name}`. It feels very unnatural to have this restriction and it's hard to reason about it. Files inside a unit directory still can't reference anything _outside_ the unit directory, which is very similar to Nix's concept of allowed-uris, which may be used to implement this check in the future. - Remove the special case of allowing custom arguments. By not having this one exception, users viewing a unit directory can be sure that there's no hidden semantics anywhere (overriding arguments) and that the functions arguments correspond directly to attributes in `pkgs`, no exceptions that would require looking at `all-packages.nix`. And it would be weird just to allow this one exception of `callPackage` with custom arguments, when there's a lot of other similarly small exceptions we could make, like allowing `python3Packages.callPackage`. - Remove the requirement that new packages must use this standard. Especially with the above exception removed, this standard is now more strict and less packages satisfy it by default. A scenario could be that a user adds a new package, initially not needing custom arguments, so CI requires it to be in `pkgs/unit`, but then a custom argument is needed, so it must be moved out of there and added to `all-packages.nix`. But then the custom argument can be removed, so it _must_ be in `pkgs/unit` again. This sucks. So let's keep `all-packages.nix` unrestricted, so a package won't have to be moved back and forth like this. --- rfcs/0140-simple-package-paths.md | 115 +++++++++++++++++++----------- 1 file changed, 75 insertions(+), 40 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 9af660edd..5fb40683d 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -18,7 +18,7 @@ This makes it much easier to contribute new packages, since there's no more gues # Motivation [motivation]: #motivation -- (Especially new) package contributors are having a hard time figuring out which files to add or edit. These are very common questions: +- It is not obvious to package contributors where to add files or which ones to 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? @@ -35,43 +35,51 @@ This makes it much easier to contribute new packages, since there's no more gues # Detailed design [design]: #detailed-design -This RFC establishes the standard of using `pkgs/unit/${shard}/${name}` "unit" directories for the definitions of the Nix packages `pkgs.${name}` in Nixpkgs, where `shard = toLower (substring 0 2 name)`. -All unit directories are automatically discovered and incorporated into the `pkgs` set using `pkgs.${name} = pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix { }`. +This RFC consists of two parts, each of which is implemented with a PR to Nixpkgs. -The following requirements will be checked by CI. -This standard must be followed for newly added packages that can satisfy these requirements. -A treewide migration to this standard will be performed for existing packages that can satisfy these requirements. +## PR 1: The unit directory standard -## Structure +This part establishes the new _unit directory standard_ in Nixpkgs. +This standard is internal to Nixpkgs and not exposed as public interface. +This standard must be documented in the Nixpkgs manual. -The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${shard}/${name}`. -Each unit directory must contain at least a `package.nix` file, but may contain arbitrary other files and directories. +### File structure -This ensures that maintainers don't have to verify this structure manually, which is prone to mistakes. +Create the initially-empty directory `pkgs/unit`, called _unit base directory_, in Nixpkgs. -## Only derivations +Check the following using CI: +- The unit base directory must only contain subdirectories of the form `pkgs/unit/${shard}/${name}`, called _unit directories_. +- `name` is a string only consisting of the ASCII characters a-z, A-Z, 0-9, `-` or `_`. +- `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. +- Each unit directory must contain a `package.nix` file and may contain arbitrary other files. -If `pkgs/unit/${shard}/${name}` exists, `pkgs.${name}` must be a derivation that can be built directly with `nix-build`. +### Semantics -This ensures that people can expect the unit directories to correspond to buildable packages and not functions like `pkgs.fetchFromGitHub` or `pkgs.buildRustCrate`. +Introduce code to automatically define `pkgs.${name}` for each unit directory as a value equivalent to +```nix +pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix { }` +``` -## Stable boundary +Check the following using CI for each unit directory: +- The only definition for `pkgs.${name}` is the automatically generated one from the unit directory +- `pkgs.${name}` must evaluate to a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). +- The `package.nix` file must not transitively refer to files outside its unit directory. -Unit directories may only interact with the rest of Nixpkgs via the stable `pkgs.${name}` attributes, not with file references: -- Files inside a unit directory must not reference files outside that unit directory. - Therefore all dependencies on other packages must come from `package.nix` arguments injected by `callPackage`. - This ensures that files in Nixpkgs can be moved around without breaking this package. -- Files outside a unit directory must not reference files inside that unit directory. - Therefore other packages can only depend on this package via `pkgs.${name}`. - This ensures that files within unit directories (except `package.nix`) can be freely moved and changed without breaking any other packages. +## PR 2: Migration -The only notable exception to this rule is the `pkgs/top-level/all-packages.nix` file which may reference the `package.nix` file according to the next requirement. +Automatically migrate to the unit directory standard for all definitions `pkgs.${name}` that can be migrated by +- Only moving files +- Not changing the evaluation result -## Custom arguments +This will cause merge conflicts with all existing PRs that modify such moved files, however they can trivially be rebased using `git rebase && git push -f`. +However, to have the least amount of conflicts, this migration should be performed soon after a release when ZHF is over and the PR rate slows down. +This also gives a lot of time to fix any potential problems before the next release. -If `pkgs/top-level/all-packages.nix` contains a definition for the attribute `${name}` and the unit directory `pkgs/unit/${shard}/${name}` exists, then the attribute value must be defined as `pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix args`, where `args` may be a freely chosen expression. +Manual updates may also be done to ensure further non-evaluation validity, such as +- [CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) +- Update scripts like [this](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/applications/version-management/cz-cli/generate-dependencies.sh) -This ensures that even if a package initially doesn't require a custom `args`, if it later does, it doesn't have to be moved out of the `pkgs/unit` directory to pass custom arguments. +Due to the strict limitations of standard, this PR will not start enforcing it for new packages. ## Examples [examples]: #examples @@ -79,7 +87,7 @@ This ensures that even if a package initially doesn't require a custom `args`, i To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/unit/fo/foobar/package.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: +With some packages, the `pkgs/unit` directory may look like this: ``` pkgs @@ -102,14 +110,35 @@ pkgs # Interactions [interactions]: #interactions -- `nix edit` and search.nixos.org are unaffected, since they rely on `meta.position` to get the file to edit, which still works +## Migration size +Due to the limitations of the standard, only a limited set of top-level attributes can be migrated: +- No attributes that aren't derivations like `pkgs.fetchFromGitHub` or `pkgs.python3Packages` +- No attributes that share common files with other attributes like [`pkgs.readline`](https://github.com/NixOS/nixpkgs/tree/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/development/libraries/readline) +- No attributes that references files from other packages like [`pkgs.gettext`](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/development/libraries/gettext/default.nix#L60) + +A good estimation of this based on [a trial PR](https://github.com/NixOS/nixpkgs/pull/211832) on commit [287b071e9a71](https://github.com/nixos/nixpkgs/commit/287b071e9a7130cacf7664e5c69ec3a889b800f8): +- 18136 (100.0%) total top-level attributes +- 16319 (90.0%) are derivations (`lib.isDerivation`) +- 10763 (59.3%) are derivations and don't violate any other conditions + +## Package locations + +`nix edit` and search.nixos.org will automatically point to the new location without problems, since they rely on `meta.position` to get the file to edit, which still works. + +## Git and NixOS release problems + +- The migration will cause merge conflicts for a lot of PRs, but they are trivially resolvable using `git rebase && git push -f` due to Git's file rename tracking. +- Commits that change moved files in `pkgs/unit` can be cherry-picked to the previous file location without problems for the same reason. - `git blame` locally and on GitHub is unaffected, since it follows file renames properly. -- A commonly recommended way of building package directories in Nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. - Since the path changes `package.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. - However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. - The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. - This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. - By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. + +## `callPackage` with `nix-build -E` + +A commonly recommended way of building package directories in Nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. +Since the path changes `package.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. +However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. +The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. +This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. +By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. # Drawbacks [drawbacks]: #drawbacks @@ -127,6 +156,17 @@ pkgs # Alternatives [alternatives]: #alternatives +TODO: This needs updating + +## A different unit base directory + +Context: `pkgs/unit` is the unit base directory + +Alternatives: +- 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 +- Other name proposals were deemed worse: `pkg`, `component`, `part`, `mod`, `comp` + ## Alternate `pkgs/unit` structure - Use a flat directory, e.g. `pkgs.hello` would be in `pkgs/unit/hello`. @@ -151,13 +191,7 @@ pkgs - Not using `default.nix` frees up `default.nix` for a short expression that is actually buildable, e.g. `(import ../.. {}).hello`, although we don't yet have a use case for this that isn't covered by `nix-build ../.. -A $attrname`. - Bad because using `default.nix` would tempt users to invoke `nix-build .` whereas making package functions auto-callable is a known anti-pattern as it duplicates the defaults. - Good because `default.nix` is already a convention most people are used to -- `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 -- Other name proposals were deemed worse: `pkg`, `component`, `part`, `mod`, `comp` +- `pkg-fun.nix`/`pkg-func.nix`: The idea with this proposal was to make it easier to potentially transition to a non-function form of packages in the future, but there's no problem with introducing versioning later if needed in case we want to reuse `pkg.nix`/`package.nix`. We also don't even know if we actually want to have a non-function form of packages. Also the abbreviations are a bit jarring. ## Filepath backwards-compatibility @@ -225,6 +259,7 @@ Con: 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): +- Add a meta tagging system to packages as a replacement for the package categories. Maybe `meta.tags` with `search.nixos.org` integration. - Making the filetree more human-friendly by grouping files together by "topic" rather than 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 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 From 1d7e66a1f8b0f8d26da8f8e77fc8d06591e710ad Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 May 2023 21:04:00 +0200 Subject: [PATCH 07/25] Re-add custom argument exception and various minor improvements --- rfcs/0140-simple-package-paths.md | 47 ++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 5fb40683d..3135e20ec 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -57,30 +57,41 @@ Check the following using CI: Introduce code to automatically define `pkgs.${name}` for each unit directory as a value equivalent to ```nix -pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix { }` +pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix { } ``` +Optionally there may also be an overriding definition of `pkgs.${name}` in `pkgs/top-level/all-packages.nix` equivalent to +```nix +pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix args +``` + +with an arbitrary `args`. + Check the following using CI for each unit directory: -- The only definition for `pkgs.${name}` is the automatically generated one from the unit directory -- `pkgs.${name}` must evaluate to a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). +- `pkgs.${name}` is defined as above, either automatically or with some `args` in `pkgs/top-level/all-packages.nix`. +- `pkgs.${name}` is a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). - The `package.nix` file must not transitively refer to files outside its unit directory. -## PR 2: Migration +## PR 2: Automated migration + +When possible, automatically migrate to the unit directory standard for all _satisfiying definitions_ `pkgs.${name}`, meaning derivations defined as above using `callPackage`. -Automatically migrate to the unit directory standard for all definitions `pkgs.${name}` that can be migrated by -- Only moving files -- Not changing the evaluation result +Automatic migration is only possible if: +- Files only need to be moved, not changed, with the exception of `pkgs/top-level/all-packages.nix` +- The Nixpkgs package evaluation result does not change -This will cause merge conflicts with all existing PRs that modify such moved files, however they can trivially be rebased using `git rebase && git push -f`. +All satisfying definitions that can't be automatically migrated due to the above restrictions will be added to a CI exclusion list. +CI is added to ensure that all satisfying definitions except the CI exclusion list must be using the unit directory standard. +This means that the unit directory standard becomes mandatory for new satisfying definitions after this PR. + +This PR will cause merge conflicts with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. However, to have the least amount of conflicts, this migration should be performed soon after a release when ZHF is over and the PR rate slows down. This also gives a lot of time to fix any potential problems before the next release. -Manual updates may also be done to ensure further non-evaluation validity, such as +Manual updates may also be done to ensure further correctness, such as - [CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) - Update scripts like [this](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/applications/version-management/cz-cli/generate-dependencies.sh) -Due to the strict limitations of standard, this PR will not start enforcing it for new packages. - ## Examples [examples]: #examples @@ -111,15 +122,17 @@ pkgs [interactions]: #interactions ## Migration size -Due to the limitations of the standard, only a limited set of top-level attributes can be migrated: +Due to the limitations of the standard, only a limited set of top-level attributes can be automatically migrated: - No attributes that aren't derivations like `pkgs.fetchFromGitHub` or `pkgs.python3Packages` +- No attributes defined using non-`pkgs.callPackage` functions like `pkgs.python3Packages.callPackage` or `pkgs.haskellPackages.callPackage`. +In the future we might consider having a separate namespace for such definitions. + +Concretely this [can be computed](https://gist.github.com/infinisil/4f2bd165c2603fc28ab536f39ac2fd27) to be 81.2% (14036) attributes out of the 17280 total non-alias top-level Nixpkgs attributes in revision [6948ef4deff7](https://github.com/nixos/nixpkgs/commit/6948ef4deff7a72ebe5242244bd3730e8542b925). + +And the initial automatic migration will be a bit more limited due to the additional constraints: - No attributes that share common files with other attributes like [`pkgs.readline`](https://github.com/NixOS/nixpkgs/tree/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/development/libraries/readline) - No attributes that references files from other packages like [`pkgs.gettext`](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/development/libraries/gettext/default.nix#L60) - -A good estimation of this based on [a trial PR](https://github.com/NixOS/nixpkgs/pull/211832) on commit [287b071e9a71](https://github.com/nixos/nixpkgs/commit/287b071e9a7130cacf7664e5c69ec3a889b800f8): -- 18136 (100.0%) total top-level attributes -- 16319 (90.0%) are derivations (`lib.isDerivation`) -- 10763 (59.3%) are derivations and don't violate any other conditions +These attributes will need to be moved to the standard manually with some arguably-needed refactoring to improve reusability of common files. ## Package locations From 0054e7a4d5de453c81a16c0e0f26498f61bc28c9 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 May 2023 23:07:35 +0200 Subject: [PATCH 08/25] shard distribution stats, cleanup, more uniformity --- rfcs/0140-simple-package-paths.md | 134 ++++++++++++++++++++---------- 1 file changed, 89 insertions(+), 45 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 3135e20ec..66860c703 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -70,7 +70,7 @@ with an arbitrary `args`. Check the following using CI for each unit directory: - `pkgs.${name}` is defined as above, either automatically or with some `args` in `pkgs/top-level/all-packages.nix`. - `pkgs.${name}` is a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). -- The `package.nix` file must not transitively refer to files outside its unit directory. +- The `package.nix` file must not transitively refer to files outside its unit directory. ## PR 2: Automated migration @@ -121,6 +121,19 @@ pkgs # Interactions [interactions]: #interactions +## Shard distribution + +The shard structure nesting unit directories under their lowercased two-letter prefix [leads to a distribution](https://gist.github.com/infinisil/95c7013db62e9f23ab2bc92165a37221) into shards as follows: +- There's 17305 total non-alias top-level attribute names in Nixpkgs revision [6948ef4deff7](https://github.com/nixos/nixpkgs/commit/6948ef4deff7a72ebe5242244bd3730e8542b925) +- These are split into 726 shards +- The top three shards are: + - "li": 1092 values, coming from the common `lib` prefix + - "op": 260 values + - "co": 252 values +- There's only a single directory with over 1 000 entries, which is notably GitHub's display limit, so this means only 92 attributes would be harder to see on GitHub + +These stats are also similar for other package sets for if this standard were to be adopted for them in the future. + ## Migration size Due to the limitations of the standard, only a limited set of top-level attributes can be automatically migrated: - No attributes that aren't derivations like `pkgs.fetchFromGitHub` or `pkgs.python3Packages` @@ -142,7 +155,7 @@ These attributes will need to be moved to the standard manually with some arguab - The migration will cause merge conflicts for a lot of PRs, but they are trivially resolvable using `git rebase && git push -f` due to Git's file rename tracking. - Commits that change moved files in `pkgs/unit` can be cherry-picked to the previous file location without problems for the same reason. -- `git blame` locally and on GitHub is unaffected, since it follows file renames properly. +- `git blame` locally and on GitHub is unaffected, since it follows file moves properly. ## `callPackage` with `nix-build -E` @@ -151,13 +164,23 @@ Since the path changes `package.nix` is now used, this becomes like `nix-build - However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. -By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. +By teaching users that `pkgs/unit//` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. + +## Removing custom arguments + +While this RFC allows passing custom arguments, doing so means that `pkgs/top-level/all-packages.nix` will have to be maintained for that package. +In specific cases where attributes of custom arguments are of the form `name = value` and `name` isn't a package attribute, they can be avoided without breaking the API. +To do so, ensure that the function in the called file has `value` as an argument and set the default of the `name` argument to `value`. + +This notably doesn't work when `name` is already a package attribute, because then the default is never used and instead overridden. # Drawbacks [drawbacks]: #drawbacks +- This standard can only be used for top-level packages using `callPackage`, so not for e.g. `python3Packages.requests` or a package defined using `haskellPackages.callPackage` +- It's not possible anymore to be a [GitHub code owner](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) of category directories. - The existing categorization of packages gets lost. Counter-arguments: - - It was never that useful to begin with + - It was never that useful to begin with. - The categorization was always incomplete, because packages defined in the language package sets often don't get their own categorized file path. - 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 @@ -169,55 +192,82 @@ By teaching users that `pkgs/unit/*/` corresponds to `nix-build -A ` # Alternatives [alternatives]: #alternatives -TODO: This needs updating - ## A different unit base directory Context: `pkgs/unit` is the unit base directory Alternatives: -- Use `unit` (at the Nixpkgs root) instead of `pkgs/unit`. +- Don't nest the directory under `pkgs` - This is future proof in case we want to make the directory structure more general purpose, but this is out of scope -- Other name proposals were deemed worse: `pkg`, `component`, `part`, `mod`, `comp` +- Another name than `unit`, proposed were `auto`, `pkg`, `mod`, `component`, `part`, `comp`, `app`, `simple`, ## Alternate `pkgs/unit` structure -- Use a flat directory, e.g. `pkgs.hello` would be in `pkgs/unit/hello`. - - Good because it's simpler, both for the user and for the code - - 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` and file listings slower -- Use `substring 0 3 name` or `substring 0 4 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, longer to type and it would improve performance only marginally. +Context: The structure is `pkgs/unit/${shard}/${name}` with `shard` being the lowercased two-letter prefix of `name`. + + +Alternatives: +- A flat directory, where `pkgs.hello` would be in `pkgs/unit/hello`. + - (+) Simpler for the user and code. + - (-) The GitHub web interface only renders the first 1 000 entries when browsing directories, which would make most packages inaccessible in this way. + - (+) This feature is not used often. + - (-) [A poll](https://discourse.nixos.org/t/poll-how-often-do-you-navigate-through-directories-on-github-to-get-to-a-nixpkgs-package-file/21641) showed that about 41% of people rely on this feature every week. + - (-) Bad because it makes `git` and file listings slower. +- Use three-letter or hour-letter prefixes. + - (-) Also leads to directories containing more than 1 000 entries, see above. +- Use multi-level structure, e.g. a two-level two-letter prefix structure where `hello` is in `pkgs/unit/he/ll/hello` + - (+) This would virtually a unlimited number of packages without performance problems + - (-) It's hard to understand, type and implement, needs a special case for packages with few characters + - E.g. `x` could go in `pkgs/unit/x-/--/x` + - (-) There's not enough packages even in Nixpkgs that a two-level structure would make sense. Most of the structure would only be filled by a couple entries. + - (-) Even Git only uses 2-letter prefixes for its objects hex hashes - 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. + - (-) The structure depends not only on the name of the package then, making it harder to find packages again and figure out where they should go + - (-) Complex to implement ## Alternate `package.nix` filename -- `default.nix`: - - Bad because it doesn't have its main benefits here: +- `default.nix` + - (+) `default.nix` is already a convention most people are used to. + - (-) We don't benefit from the usual `default.nix` benefits: - 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`. + - (+) But there's still some support for `all-packages.nix` for custom arguments, which requires people to type out the name + - (-) This is hopefully only temporary, in the future we should fully get rid of `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`, although we don't yet have a use case for this that isn't covered by `nix-build ../.. -A $attrname`. - - Bad because using `default.nix` would tempt users to invoke `nix-build .` whereas making package functions auto-callable is a known anti-pattern as it duplicates the defaults. - - Good because `default.nix` is already a convention most people are used to -- `pkg-fun.nix`/`pkg-func.nix`: The idea with this proposal was to make it easier to potentially transition to a non-function form of packages in the future, but there's no problem with introducing versioning later if needed in case we want to reuse `pkg.nix`/`package.nix`. We also don't even know if we actually want to have a non-function form of packages. Also the abbreviations are a bit jarring. + - (-) Not using `default.nix` frees up `default.nix` for an expression that is actually buildable, e.g. `(import ../.. {}).hello`, although we don't yet have a use case for this that isn't covered by `nix-build ../.. -A `. + - (-) Using `default.nix` would tempt users to invoke `nix-build .`, which wouldn't work and making package functions auto-callable is a known anti-pattern. +- `pkg-fun[c].nix` + - (+) Makes a potentially transition to a non-function form of packages in the future easier. + - (-) There's no problem with introducing versioning later with different filenames. + - (-) We don't even know if we actually want to have a non-function form of packages. + - (-) Abbreviations are a bit jarring. ## 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. +Context: The migration moves files around without providing any backwards compatibility for those moved paths. + +Alternative: +- 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)`. + - (-) It would give precedent to file paths being a stable API interface, which definitely shouldn't be the case (bar some exceptions). + - (-) Leads 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) +## Reference check -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. +Context: There's a [requirement](#user-content-req-ref) to check that unit directories can't access paths outside themselves. + +Alternatives: +- Don't have this requirement + - (-) Doesn't discourage the use of file paths as an API. + - (-) Makes further migrations to different file structures harder. +- Make the requirement also apply the other way around: Files outside the unit directory cannot access files inside it, with `package.nix` being the only exception + - (-) Enforcing this requires a global view of Nixpkgs, which is hard to implement ## Restrict design to try delay issues like "package variants" {#no-variants} +TODO: This section might be outdated, it's a bit ambiguous + We perceived some uncertainty around [package variants](#def-package-variant) that led us to scope these out at first, but we did not identify a real problem that would arise from allowing non-auto-called attributes to reference `pkgs/unit` files. However, imposing unnecessary restrictions would be counterproductive because: - The contributor experience would suffer, because it won't be obvious to everyone whether their package is allowed to go into `pkgs/unit`. This means that we'd fail to solve the goal "Which directory should my package definition go in?", leading to unnecessary requests for changes in pull requests. @@ -236,17 +286,12 @@ That said, we did identify risks: - We might not focus enough on the foundation, while we could more easily relax requirements later. - After more discussion, we feel confident that the manual `callPackage` calls are unlikely to cause issues that we wouldn't otherwise have. -## Recommend a `callPackage` pattern with default arguments - -> - 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 - -`callPackage` does not favor the default argument when both a default argument and a value in `pkgs` exist. Changing the semantics of `callPackage` is out of scope. - ## Allow `callPackage` arguments to be specified in `/args.nix` -The idea was to expand the auto-calling logic according to: +Context: Custom `callPackage` arguments have to be added to `all-packages.nix` +Alternative: +Expand the auto-calling logic according to: Unit directories are automatically discovered and transformed to a definition of the form ``` # If args.nix doesn't exist @@ -255,14 +300,11 @@ pkgs.${name} = pkgs.callPackage ${unitDir}/package.nix {} pkgs.${name} = pkgs.callPackage ${unitDir}/package.nix (import ${unitDir}/args.nix pkgs); ``` -Pro: - - It makes another class of packages uniform, by picking a solution with restricted expressive power. - -Con: - - It does not solve the contributor experience problem of having too many rules. - - `args.nix` is another pattern that contributors need to learn how to use, as we have seen that it is not immediately obvious to everyone how it works. - - A CI check can mitigate the possible lack of uniformity, and we see a simple implementation strategy for it. - - This keeps the contents of the unit directories simple and a bit more uniform than with optional `args.nix` files. +- (+) It makes another class of packages uniform, by picking a solution with restricted expressive power. +- (-) It does not solve the contributor experience problem of having too many rules. + `args.nix` is another pattern that contributors need to learn how to use, as we have seen that it is not immediately obvious to everyone how it works. + - (+) A CI check can mitigate the possible lack of uniformity, and we see a simple implementation strategy for it. +- (-) Complicates the unit directory structure with an optional file # Unresolved questions [unresolved]: #unresolved-questions @@ -272,11 +314,13 @@ Con: 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): -- Add a meta tagging system to packages as a replacement for the package categories. Maybe `meta.tags` with `search.nixos.org` integration. +- Expose an API to get access to the package functions directly, without calling them +- Add a meta tagging or categorization system to packages as a replacement for the package categories. Maybe `meta.tags` with `search.nixos.org` integration. Maybe https://repology.org/ integration. See also https://github.com/NixOS/rfcs/pull/146. - Making the filetree more human-friendly by grouping files together by "topic" rather than 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 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 - Improve the semantics of `callPackage` and/or apply a better solution, such as a module-like solution +- Potentially establish an updateScript standard to avoid problems like, relates to Flakes too - What to do with different versions, e.g. `wlroots = wlroots_0_14`? This goes into version resolution, a different problem to fix - What to do about e.g. `libsForQt5.callPackage`? This goes into overrides, a different problem to fix - What about aliases like `jami-daemon = jami.jami-daemon`? From ffad8eadfd00e54755c255719eff6daa9720f5e7 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 May 2023 23:10:16 +0200 Subject: [PATCH 09/25] Readd accidentally removed definition --- rfcs/0140-simple-package-paths.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 66860c703..4488a0c0c 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -268,6 +268,11 @@ Alternatives: TODO: This section might be outdated, it's a bit ambiguous +- *variant attribute*: an attribute that defines a package by invoking it with non-default arguments, for example: + ``` + graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; + ``` + We perceived some uncertainty around [package variants](#def-package-variant) that led us to scope these out at first, but we did not identify a real problem that would arise from allowing non-auto-called attributes to reference `pkgs/unit` files. However, imposing unnecessary restrictions would be counterproductive because: - The contributor experience would suffer, because it won't be obvious to everyone whether their package is allowed to go into `pkgs/unit`. This means that we'd fail to solve the goal "Which directory should my package definition go in?", leading to unnecessary requests for changes in pull requests. From 66f0225af85eed85709db365264e4c1844842f60 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 May 2023 23:45:56 +0200 Subject: [PATCH 10/25] Mention package variants --- rfcs/0140-simple-package-paths.md | 85 ++++++++++++++++++------------- 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 4488a0c0c..0d06bbf13 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -121,6 +121,37 @@ pkgs # Interactions [interactions]: #interactions +## Package variants + +Sometimes there's a need to create a variant of a package with different `callPackage` arguments. This can be achieved using `.override` as follows: +```nix +{ + graphviz_nox = graphviz.override { withXorg = false; }; +} +``` + +However this can cause problems with an overlay that tries to make the variant the default as follows: +```nix +self: super: { + # Oops, infinite recursion! + graphviz = self.graphviz_nox; +} +``` + +Because of this, there's the pattern of duplicating the `callPackage` call with the custom arguments as such: +```nix +{ + graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; +} +``` + +The semantics of how unit directories are checked by CI do allow the definition of package variants from unit directories: +```nix +{ + graphviz_nox = callPackage ../unit/gr/graphviz/package.nix { withXorg = false; }; +} +``` + ## Shard distribution The shard structure nesting unit directories under their lowercased two-letter prefix [leads to a distribution](https://gist.github.com/infinisil/95c7013db62e9f23ab2bc92165a37221) into shards as follows: @@ -166,7 +197,7 @@ The correct way of doing it was to add the package to `pkgs/top-level/all-packag This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. By teaching users that `pkgs/unit//` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. -## Removing custom arguments +## Limited manual removal of custom arguments While this RFC allows passing custom arguments, doing so means that `pkgs/top-level/all-packages.nix` will have to be maintained for that package. In specific cases where attributes of custom arguments are of the form `name = value` and `name` isn't a package attribute, they can be avoided without breaking the API. @@ -205,7 +236,6 @@ Alternatives: Context: The structure is `pkgs/unit/${shard}/${name}` with `shard` being the lowercased two-letter prefix of `name`. - Alternatives: - A flat directory, where `pkgs.hello` would be in `pkgs/unit/hello`. - (+) Simpler for the user and code. @@ -229,6 +259,9 @@ Alternatives: ## Alternate `package.nix` filename +Context: The only file that has to exist in unit directories is `package.nix`, it must contain a function suitable for `callPackage`. + +Alternatives: - `default.nix` - (+) `default.nix` is already a convention most people are used to. - (-) We don't benefit from the usual `default.nix` benefits: @@ -253,43 +286,27 @@ Alternative: - (-) It would give precedent to file paths being a stable API interface, which definitely shouldn't be the case (bar some exceptions). - (-) Leads 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. -## Reference check - -Context: There's a [requirement](#user-content-req-ref) to check that unit directories can't access paths outside themselves. - -Alternatives: -- Don't have this requirement - - (-) Doesn't discourage the use of file paths as an API. - - (-) Makes further migrations to different file structures harder. -- Make the requirement also apply the other way around: Files outside the unit directory cannot access files inside it, with `package.nix` being the only exception - - (-) Enforcing this requires a global view of Nixpkgs, which is hard to implement +## Don't allow custom arguments -## Restrict design to try delay issues like "package variants" {#no-variants} +Context: It's possible to override the default `{ }` argument to `callPackage` by manually specifying the full definition in `all-packages.nix` -TODO: This section might be outdated, it's a bit ambiguous +The alternative is to not allow that, requiring that `pkgs.${name}` corresponds directly to `callPackage pkgs/unit/${shard}/${name}/package.nix { }`. +- (-) It's harder to explain to beginners whether their package can use the unit directory standard or not +- (+) The direct correspondance ensures that the unit directory contains all information about the package, which is very intuitive + - (-) We're not at the point where we can have that though, custom arguments don't have a good replacement yet +- (-) If a package previously didn't need custom arguments, it would be moved to a unit directory. But then there's a need for a custom argument, which then requires moving it out from the unit directory and into the freeform structure of `pkgs/` again. +- (+) It's easier to relax restrictions than to impose new ones -- *variant attribute*: an attribute that defines a package by invoking it with non-default arguments, for example: - ``` - graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; - ``` - -We perceived some uncertainty around [package variants](#def-package-variant) that led us to scope these out at first, but we did not identify a real problem that would arise from allowing non-auto-called attributes to reference `pkgs/unit` files. However, imposing unnecessary restrictions would be counterproductive because: - - - The contributor experience would suffer, because it won't be obvious to everyone whether their package is allowed to go into `pkgs/unit`. This means that we'd fail to solve the goal "Which directory should my package definition go in?", leading to unnecessary requests for changes in pull requests. - - - Changes in dependencies can require dependents to add an override, causing packages to be moved back and forth between unit directories and the general `pkgs` tree, worsening the problem as people have to decide categories *again*. - - - When lifting the restriction, the reviewers have to adapt, again leading to unnecessary requests for changes in pull requests. - - - We'd be protracting the migration by unnecessary gatekeeping or discovering some problem late. +## Reference check -That said, we did identify risks: +Context: There's a [requirement](#user-content-req-ref) to check that unit directories can't access paths outside themselves. - - We might get something wrong, and while we plan to incrementally migrate Nixpkgs to this new system anyway, starting with fewer units is good. - - Mitigation: only automate the renames of simple (`callPackage path { }`) calls, to keep the initial change small - - - We might not focus enough on the foundation, while we could more easily relax requirements later. - - After more discussion, we feel confident that the manual `callPackage` calls are unlikely to cause issues that we wouldn't otherwise have. +Alternatives: Don't have this requirement +- (-) Doesn't discourage the use of file paths as an API. +- (-) Makes further migrations to different file structures harder. +- Make the requirement also apply the other way around: Files outside the unit directory cannot access files inside it, with `package.nix` being the only exception, and only for the one attribute in `all-packages.nix` +- (-) Enforcing this requires a global view of Nixpkgs, which is nasty to implement +- (-) [Package variants](#package-variants) would not be possible to define ## Allow `callPackage` arguments to be specified in `/args.nix` From 89ea8e7bc8141f509ab307e031eb320eb8150018 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 11 May 2023 23:58:57 +0200 Subject: [PATCH 11/25] Minor moving and formatting --- rfcs/0140-simple-package-paths.md | 99 +++++++++++++++---------------- 1 file changed, 48 insertions(+), 51 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 0d06bbf13..dafb43aee 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -13,7 +13,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/211832 [summary]: #summary Auto-generate trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` from a directory structure that matches the attribute name. -This makes it much easier to contribute new 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`. +This makes it much easier to contribute new packages, since there's no more guessing needed as to where the package should go, both in the ad-hoc directory categories and in `all-packages.nix`. # Motivation [motivation]: #motivation @@ -23,9 +23,9 @@ This makes it much easier to contribute new packages, since there's no more gues - 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? + - 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 + - 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 locks up editors due to the file size @@ -36,6 +36,7 @@ This makes it much easier to contribute new packages, since there's no more gues [design]: #detailed-design This RFC consists of two parts, each of which is implemented with a PR to Nixpkgs. +These PR's should be done after a release to maximize the testing period and minimize merge conflicts. ## PR 1: The unit directory standard @@ -49,7 +50,7 @@ Create the initially-empty directory `pkgs/unit`, called _unit base directory_, Check the following using CI: - The unit base directory must only contain subdirectories of the form `pkgs/unit/${shard}/${name}`, called _unit directories_. -- `name` is a string only consisting of the ASCII characters a-z, A-Z, 0-9, `-` or `_`. +- `name` is a string only consisting of the ASCII characters `a-z`, `A-Z`, `0-9`, `-` or `_`. - `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. - Each unit directory must contain a `package.nix` file and may contain arbitrary other files. @@ -70,33 +71,29 @@ with an arbitrary `args`. Check the following using CI for each unit directory: - `pkgs.${name}` is defined as above, either automatically or with some `args` in `pkgs/top-level/all-packages.nix`. - `pkgs.${name}` is a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). -- The `package.nix` file must not transitively refer to files outside its unit directory. +- The `package.nix` file evaluated from `pkgs.${name}` must not access files outside its unit directory. ## PR 2: Automated migration -When possible, automatically migrate to the unit directory standard for all _satisfiying definitions_ `pkgs.${name}`, meaning derivations defined as above using `callPackage`. +Automatically migrate to the unit directory standard for all _satisfiying definitions_ `pkgs.${name}`, meaning derivations defined as above using `callPackage`. -Automatic migration is only possible if: -- Files only need to be moved, not changed, with the exception of `pkgs/top-level/all-packages.nix` +However automatic migration is only possible if: +- Files don't need to be changed, only moved, with the exception of `pkgs/top-level/all-packages.nix` - The Nixpkgs package evaluation result does not change All satisfying definitions that can't be automatically migrated due to the above restrictions will be added to a CI exclusion list. CI is added to ensure that all satisfying definitions except the CI exclusion list must be using the unit directory standard. This means that the unit directory standard becomes mandatory for new satisfying definitions after this PR. -This PR will cause merge conflicts with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. -However, to have the least amount of conflicts, this migration should be performed soon after a release when ZHF is over and the PR rate slows down. -This also gives a lot of time to fix any potential problems before the next release. - -Manual updates may also be done to ensure further correctness, such as -- [CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) +Non-automatic updates may also be done to ensure further correctness, such as +- [GitHub's CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) - Update scripts like [this](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/applications/version-management/cz-cli/generate-dependencies.sh) ## Examples [examples]: #examples To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/unit/fo/foobar/package.nix`. -No need to find an appropriate category nor to modify `pkgs/top-level/all-packages.nix` anymore. +No need to find an appropriate category nor to modify `all-packages.nix` anymore. With some packages, the `pkgs/unit` directory may look like this: @@ -121,37 +118,6 @@ pkgs # Interactions [interactions]: #interactions -## Package variants - -Sometimes there's a need to create a variant of a package with different `callPackage` arguments. This can be achieved using `.override` as follows: -```nix -{ - graphviz_nox = graphviz.override { withXorg = false; }; -} -``` - -However this can cause problems with an overlay that tries to make the variant the default as follows: -```nix -self: super: { - # Oops, infinite recursion! - graphviz = self.graphviz_nox; -} -``` - -Because of this, there's the pattern of duplicating the `callPackage` call with the custom arguments as such: -```nix -{ - graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; -} -``` - -The semantics of how unit directories are checked by CI do allow the definition of package variants from unit directories: -```nix -{ - graphviz_nox = callPackage ../unit/gr/graphviz/package.nix { withXorg = false; }; -} -``` - ## Shard distribution The shard structure nesting unit directories under their lowercased two-letter prefix [leads to a distribution](https://gist.github.com/infinisil/95c7013db62e9f23ab2bc92165a37221) into shards as follows: @@ -184,8 +150,8 @@ These attributes will need to be moved to the standard manually with some arguab ## Git and NixOS release problems -- The migration will cause merge conflicts for a lot of PRs, but they are trivially resolvable using `git rebase && git push -f` due to Git's file rename tracking. -- Commits that change moved files in `pkgs/unit` can be cherry-picked to the previous file location without problems for the same reason. +- The migration PR will cause merge conflicts with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. +- Commits that change moved files in `pkgs/unit` can be cherry-picked to the previous file location without problems. - `git blame` locally and on GitHub is unaffected, since it follows file moves properly. ## `callPackage` with `nix-build -E` @@ -193,18 +159,49 @@ These attributes will need to be moved to the standard manually with some arguab A commonly recommended way of building package directories in Nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. Since the path changes `package.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. -The correct way of doing it was to add the package to `pkgs/top-level/all-packages.nix`, then calling `nix-build -A hello`. +The correct way of doing it was to add the package to `all-packages.nix`, then calling `nix-build -A hello`. This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. By teaching users that `pkgs/unit//` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. -## Limited manual removal of custom arguments +## Manual removal of custom arguments -While this RFC allows passing custom arguments, doing so means that `pkgs/top-level/all-packages.nix` will have to be maintained for that package. +While this RFC allows passing custom arguments, doing so means that `all-packages.nix` will have to be maintained for that package. In specific cases where attributes of custom arguments are of the form `name = value` and `name` isn't a package attribute, they can be avoided without breaking the API. To do so, ensure that the function in the called file has `value` as an argument and set the default of the `name` argument to `value`. This notably doesn't work when `name` is already a package attribute, because then the default is never used and instead overridden. +## Package variants + +Sometimes there's a need to create a variant of a package with different `callPackage` arguments. This can be achieved using `.override` as follows: +```nix +{ + graphviz_nox = graphviz.override { withXorg = false; }; +} +``` + +However this can cause problems with an overlay that tries to make the variant the default as follows: +```nix +self: super: { + # Oops, infinite recursion! + graphviz = self.graphviz_nox; +} +``` + +Because of this, there's the pattern of duplicating the `callPackage` call with the custom arguments as such: +```nix +{ + graphviz_nox = callPackage ../tools/graphics/graphviz { withXorg = false; }; +} +``` + +The semantics of how unit directories are checked by CI do allow the definition of package variants from unit directories: +```nix +{ + graphviz_nox = callPackage ../unit/gr/graphviz/package.nix { withXorg = false; }; +} +``` + # Drawbacks [drawbacks]: #drawbacks From 4426f20fa2de0145c38a281711cc534ee44fa2ee Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 12 May 2023 19:45:52 +0200 Subject: [PATCH 12/25] Changes from feedback in the meeting --- rfcs/0140-simple-package-paths.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index dafb43aee..eeb6325db 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -43,6 +43,7 @@ These PR's should be done after a release to maximize the testing period and min This part establishes the new _unit directory standard_ in Nixpkgs. This standard is internal to Nixpkgs and not exposed as public interface. This standard must be documented in the Nixpkgs manual. +This PR will be backported to the stable release in order to ensure that backports of new packages work. ### File structure @@ -84,11 +85,15 @@ However automatic migration is only possible if: All satisfying definitions that can't be automatically migrated due to the above restrictions will be added to a CI exclusion list. CI is added to ensure that all satisfying definitions except the CI exclusion list must be using the unit directory standard. This means that the unit directory standard becomes mandatory for new satisfying definitions after this PR. +The CI exclusion list should be removed eventually once the non-automatically-migratable satisfying definitions have been manually migrated. +Only in very limited circumstances is it allowed to add new entries to the CI exclusion list. Non-automatic updates may also be done to ensure further correctness, such as - [GitHub's CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) - Update scripts like [this](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/applications/version-management/cz-cli/generate-dependencies.sh) +Since this migration should be widely announced with a pinned issue on the Nixpkgs issue tracker and a Discourse post. + ## Examples [examples]: #examples From 4f0c06fe6c85b85667c53086160b6eb2052177a7 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 15 May 2023 20:33:32 +0200 Subject: [PATCH 13/25] Link to demonstration of cherry-picking without problems --- rfcs/0140-simple-package-paths.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index eeb6325db..53bf84f90 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -156,7 +156,7 @@ These attributes will need to be moved to the standard manually with some arguab ## Git and NixOS release problems - The migration PR will cause merge conflicts with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. -- Commits that change moved files in `pkgs/unit` can be cherry-picked to the previous file location without problems. +- Commits that change moved files in `pkgs/unit` [can be cherry-picked](https://gist.github.com/infinisil/00b5ccc62b76bc1fe91b32db758adb41) to the previous file location without problems. - `git blame` locally and on GitHub is unaffected, since it follows file moves properly. ## `callPackage` with `nix-build -E` From 98f828779295e3984395835f17f06a1b7ed15c8d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 15 May 2023 21:06:25 +0200 Subject: [PATCH 14/25] Link to demonstrates of problematic/non-problematic Git operations --- rfcs/0140-simple-package-paths.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 53bf84f90..3e9decf1b 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -92,7 +92,9 @@ Non-automatic updates may also be done to ensure further correctness, such as - [GitHub's CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) - Update scripts like [this](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/applications/version-management/cz-cli/generate-dependencies.sh) -Since this migration should be widely announced with a pinned issue on the Nixpkgs issue tracker and a Discourse post. +This PR [will cause merge conflicts](https://github.com/nixpkgs-architecture/nixpkgs/pull/2) with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. +Because of this, merging of this PR should be widely announced with a pinned issue on the Nixpkgs issue tracker and a Discourse post. +Additionally this PR can benefit from being merged after a release due to the decreased PR count, leading to less conflicts. ## Examples [examples]: #examples @@ -153,10 +155,9 @@ These attributes will need to be moved to the standard manually with some arguab `nix edit` and search.nixos.org will automatically point to the new location without problems, since they rely on `meta.position` to get the file to edit, which still works. -## Git and NixOS release problems +## Git and NixOS release -- The migration PR will cause merge conflicts with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. -- Commits that change moved files in `pkgs/unit` [can be cherry-picked](https://gist.github.com/infinisil/00b5ccc62b76bc1fe91b32db758adb41) to the previous file location without problems. +- Backporting changes to moved files [won't be problematic](https://github.com/nixpkgs-architecture/nixpkgs/pull/4) - `git blame` locally and on GitHub is unaffected, since it follows file moves properly. ## `callPackage` with `nix-build -E` From e6bd2f583b3cf937ceafbca829bbcd1ddef078d0 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 16 May 2023 17:35:41 +0200 Subject: [PATCH 15/25] Names must be unique when lowercased --- rfcs/0140-simple-package-paths.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 3e9decf1b..a3471604d 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -51,6 +51,7 @@ Create the initially-empty directory `pkgs/unit`, called _unit base directory_, Check the following using CI: - The unit base directory must only contain subdirectories of the form `pkgs/unit/${shard}/${name}`, called _unit directories_. +- The `name`'s of unit directories must be unique when lowercased - `name` is a string only consisting of the ASCII characters `a-z`, `A-Z`, `0-9`, `-` or `_`. - `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. - Each unit directory must contain a `package.nix` file and may contain arbitrary other files. From 632fb4d41e5e35920432d43f336486505fc496c3 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 17 May 2023 03:01:59 +0200 Subject: [PATCH 16/25] Properly close invisible anchor --- rfcs/0140-simple-package-paths.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index a3471604d..d640e4f9d 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -73,7 +73,7 @@ with an arbitrary `args`. Check the following using CI for each unit directory: - `pkgs.${name}` is defined as above, either automatically or with some `args` in `pkgs/top-level/all-packages.nix`. - `pkgs.${name}` is a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). -- The `package.nix` file evaluated from `pkgs.${name}` must not access files outside its unit directory. +- The `package.nix` file evaluated from `pkgs.${name}` must not access files outside its unit directory. ## PR 2: Automated migration From 71e40dce22fede839fdc32f69a5846745b9f3012 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 18 May 2023 00:20:49 +0200 Subject: [PATCH 17/25] Update summary to mention Nixpkgs more explicitly --- rfcs/0140-simple-package-paths.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index d640e4f9d..77907c4ad 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -12,7 +12,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/211832 # Summary [summary]: #summary -Auto-generate trivial top-level attribute definitions in `pkgs/top-level/all-packages.nix` from a directory structure that matches the attribute name. +Auto-generate trivial top-level attribute definitions in Nixpkgs' `pkgs/top-level/all-packages.nix` from a Nixpkgs-internally standardised directory structure that matches the attribute name. This makes it much easier to contribute new packages, since there's no more guessing needed as to where the package should go, both in the ad-hoc directory categories and in `all-packages.nix`. # Motivation From 46c23ae73b7d1c959a2c1216d995c8a9971671fd Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 18 May 2023 00:37:03 +0200 Subject: [PATCH 18/25] Include more arguments and counter-arguments for pkgs/unit alternatives --- rfcs/0140-simple-package-paths.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 77907c4ad..7bfcbb409 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -232,11 +232,22 @@ The semantics of how unit directories are checked by CI do allow the definition Context: `pkgs/unit` is the unit base directory Alternatives: -- Don't nest the directory under `pkgs` - - This is future proof in case we want to make the directory structure more general purpose, but this is out of scope -- Another name than `unit`, proposed were `auto`, `pkg`, `mod`, `component`, `part`, `comp`, `app`, `simple`, - -## Alternate `pkgs/unit` structure +- Use `unit` in the root directory instead + - (+) This is future proof in case we want to make the directory structure more general purpose + - (-) We don't yet know if we want that, so this is out of scope for now +- Use `pkgs` instead, so that the `${shard}`'s are siblings to the other current directories in `pkgs` such as `top-level`, with the intention that the other directories would be hopefully removed at some point, then only leaving the shards in `pkgs` + - (+) If we remove the other directories at some point, only the `${shard}`'s will be left in `pkgs` + - (-) This leads to ambiguities between the directories from the standard and the other directories, requiring special handling in the code and CI, leading to complexities. + - (-) This would be harder to document and explain to people, since one always has to disregard all non-unit directories, with no obvious justification + - (-) Currently we cannot apply this standard to all definitions in `pkgs`, in particular nested packages like `pythonPackages.*`, non-`callPackage`'d definitions like `copyDesktopItems` and non-derivations like `fetchFromGitHub`. + Depending on how we want to handle those, it might make more sense to keep `pkgs/unit` or to use `pkgs` directly once all legacy paths are migrated away to another top-level directory, we don't yet know. `pkgs/unit` will be easier to migrate to `pkgs` than the other way around though. +- Another name than `unit`, proposed were `auto`, `pkg`, `mod`, `component`, `part`, `comp`, `app`, `simple` + - (-) `unit` is considered the most appropriate because a "unit" typically refers to a discrete, indivisible entity that is distict from other entities of the same type. + - (-) In addition we envision that in the future we would extend the unit directory standard to not just include a package definition for each unit, but also other parts such as NixOS modules, library components, tests, etc. In this case `unit` would fit even better and could be described as + > A collection of standardized files related to the same software component + + +## Alternate shard structure Context: The structure is `pkgs/unit/${shard}/${name}` with `shard` being the lowercased two-letter prefix of `name`. From f7c3056b58260d545cae03406a4f708bcd1d2b27 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 18 May 2023 00:42:13 +0200 Subject: [PATCH 19/25] Add shepherd team and nicks --- rfcs/0140-simple-package-paths.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 7bfcbb409..37b5a2346 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -1,11 +1,11 @@ --- feature: simple-package-paths start-date: 2022-09-02 -author: Silvan Mosberger -co-authors: Robert Hensing -pre-RFC reviewers: Thomas Bereknyei, John Ericson, Alex Ameen -shepherd-team: (names, to be nominated and accepted by RFC steering committee) -shepherd-leader: (name to be appointed by RFC steering committee) +author: "Silvan Mosberger (@infinisil)" +co-authors: "Robert Hensing (@roberth)" +pre-RFC reviewers: "Thomas Bereknyei (@tomberek), John Ericson (@Ericson2314), Alex Ameen (@aakropotkin)" +shepherd-team: "@phaer @06kellyjac @aakropotkin @piegamesde" +shepherd-leader: - related-issues: https://github.com/NixOS/nixpkgs/pull/211832 --- From f53a862c0fbf43a14e995fa4637c846a6fc5b4ef Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 18 May 2023 00:45:05 +0200 Subject: [PATCH 20/25] Convert frontmatter to a table --- rfcs/0140-simple-package-paths.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 37b5a2346..441a0df37 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -1,13 +1,11 @@ ---- -feature: simple-package-paths -start-date: 2022-09-02 -author: "Silvan Mosberger (@infinisil)" -co-authors: "Robert Hensing (@roberth)" -pre-RFC reviewers: "Thomas Bereknyei (@tomberek), John Ericson (@Ericson2314), Alex Ameen (@aakropotkin)" -shepherd-team: "@phaer @06kellyjac @aakropotkin @piegamesde" -shepherd-leader: - -related-issues: https://github.com/NixOS/nixpkgs/pull/211832 ---- +| feature | simple-package-paths | +| start-date | 2022-09-02 | +| author | Silvan Mosberger (@infinisil) | +| co-authors | Robert Hensing (@roberth) | +| pre-RFC reviewers | Thomas Bereknyei (@tomberek), John Ericson (@Ericson2314), Alex Ameen (@aakropotkin) | +| shepherd-team | @phaer @06kellyjac @aakropotkin @piegamesde | +| shepherd-leader | - | +| related-issues | https://github.com/NixOS/nixpkgs/pull/211832 | # Summary [summary]: #summary From e5b2019045ed9d8c079d2045093409968d696513 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 18 May 2023 00:46:19 +0200 Subject: [PATCH 21/25] Fix table rendering --- rfcs/0140-simple-package-paths.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 441a0df37..b207ad2c7 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -1,4 +1,5 @@ | feature | simple-package-paths | +| --- | --- | | start-date | 2022-09-02 | | author | Silvan Mosberger (@infinisil) | | co-authors | Robert Hensing (@roberth) | From b72d482e249fdd41d02a05281cf65a6457b58733 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 29 May 2023 16:13:54 +0200 Subject: [PATCH 22/25] Minor fixups Co-Authored-By: Robert Hensing --- rfcs/0140-simple-package-paths.md | 35 ++++++++++++++++--------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index b207ad2c7..5d57b5d68 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -49,7 +49,7 @@ This PR will be backported to the stable release in order to ensure that backpor Create the initially-empty directory `pkgs/unit`, called _unit base directory_, in Nixpkgs. Check the following using CI: -- The unit base directory must only contain subdirectories of the form `pkgs/unit/${shard}/${name}`, called _unit directories_. +- The unit base directory must only contain subdirectories of the form `${shard}/${name}`, called _unit directories_. - The `name`'s of unit directories must be unique when lowercased - `name` is a string only consisting of the ASCII characters `a-z`, `A-Z`, `0-9`, `-` or `_`. - `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. @@ -160,14 +160,14 @@ These attributes will need to be moved to the standard manually with some arguab - Backporting changes to moved files [won't be problematic](https://github.com/nixpkgs-architecture/nixpkgs/pull/4) - `git blame` locally and on GitHub is unaffected, since it follows file moves properly. -## `callPackage` with `nix-build -E` +## `callPackage` with `nix-build --expr` -A commonly recommended way of building package directories in Nixpkgs is to use `nix-build -E 'with import {}; callPackage pkgs/applications/misc/hello {}'`. -Since the path changes `package.nix` is now used, this becomes like `nix-build -E 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. +A commonly recommended way of building package directories in Nixpkgs is to use `nix-build --expr 'with import {}; callPackage pkgs/applications/misc/hello {}'`. +Since the path changes `package.nix` is now used, this becomes like `nix-build --expr 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. The correct way of doing it was to add the package to `all-packages.nix`, then calling `nix-build -A hello`. -This `nix-build -E` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. -By teaching users that `pkgs/unit//` corresponds to `nix-build -A `, the need for such `nix-build -E` workarounds should disappear. +This `nix-build --expr` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. +By teaching users that `pkgs/unit//` corresponds to `nix-build -A `, the need for such `nix-build --expr` workarounds should disappear. ## Manual removal of custom arguments @@ -175,7 +175,7 @@ While this RFC allows passing custom arguments, doing so means that `all-package In specific cases where attributes of custom arguments are of the form `name = value` and `name` isn't a package attribute, they can be avoided without breaking the API. To do so, ensure that the function in the called file has `value` as an argument and set the default of the `name` argument to `value`. -This notably doesn't work when `name` is already a package attribute, because then the default is never used and instead overridden. +This notably doesn't work when `name` is already a package attribute or when such a package is added later, because then the default is never used and instead overridden. ## Package variants @@ -257,10 +257,10 @@ Alternatives: - (+) This feature is not used often. - (-) [A poll](https://discourse.nixos.org/t/poll-how-often-do-you-navigate-through-directories-on-github-to-get-to-a-nixpkgs-package-file/21641) showed that about 41% of people rely on this feature every week. - (-) Bad because it makes `git` and file listings slower. -- Use three-letter or hour-letter prefixes. +- Use three-letter or four-letter prefixes. - (-) Also leads to directories containing more than 1 000 entries, see above. - Use multi-level structure, e.g. a two-level two-letter prefix structure where `hello` is in `pkgs/unit/he/ll/hello` - - (+) This would virtually a unlimited number of packages without performance problems + - (+) This would allow virtually a unlimited number of packages without performance problems - (-) It's hard to understand, type and implement, needs a special case for packages with few characters - E.g. `x` could go in `pkgs/unit/x-/--/x` - (-) There's not enough packages even in Nixpkgs that a two-level structure would make sense. Most of the structure would only be filled by a couple entries. @@ -286,7 +286,7 @@ Alternatives: - (-) Not using `default.nix` frees up `default.nix` for an expression that is actually buildable, e.g. `(import ../.. {}).hello`, although we don't yet have a use case for this that isn't covered by `nix-build ../.. -A `. - (-) Using `default.nix` would tempt users to invoke `nix-build .`, which wouldn't work and making package functions auto-callable is a known anti-pattern. - `pkg-fun[c].nix` - - (+) Makes a potentially transition to a non-function form of packages in the future easier. + - (+) Makes a potential transition to a non-function form of packages in the future easier. - (-) There's no problem with introducing versioning later with different filenames. - (-) We don't even know if we actually want to have a non-function form of packages. - (-) Abbreviations are a bit jarring. @@ -308,19 +308,20 @@ The alternative is to not allow that, requiring that `pkgs.${name}` corresponds - (-) It's harder to explain to beginners whether their package can use the unit directory standard or not - (+) The direct correspondance ensures that the unit directory contains all information about the package, which is very intuitive - (-) We're not at the point where we can have that though, custom arguments don't have a good replacement yet -- (-) If a package previously didn't need custom arguments, it would be moved to a unit directory. But then there's a need for a custom argument, which then requires moving it out from the unit directory and into the freeform structure of `pkgs/` again. +- (-) If a package previously didn't need custom arguments, it would be moved to a unit directory. But when the need for a custom argument arises, it then requires moving it out from the unit directory and into the freeform structure of `pkgs/` again. - (+) It's easier to relax restrictions than to impose new ones ## Reference check Context: There's a [requirement](#user-content-req-ref) to check that unit directories can't access paths outside themselves. -Alternatives: Don't have this requirement -- (-) Doesn't discourage the use of file paths as an API. -- (-) Makes further migrations to different file structures harder. +Alternatives: +- Don't have this requirement + - (-) Doesn't discourage the use of file paths as an API. + - (-) Makes further migrations to different file structures harder. - Make the requirement also apply the other way around: Files outside the unit directory cannot access files inside it, with `package.nix` being the only exception, and only for the one attribute in `all-packages.nix` -- (-) Enforcing this requires a global view of Nixpkgs, which is nasty to implement -- (-) [Package variants](#package-variants) would not be possible to define + - (-) Enforcing this requires a global view of Nixpkgs, which is nasty to implement + - (-) [Package variants](#package-variants) would not be possible to define ## Allow `callPackage` arguments to be specified in `/args.nix` @@ -358,6 +359,6 @@ All of these questions are in scope to be addressed in future discussions in the - Improve the semantics of `callPackage` and/or apply a better solution, such as a module-like solution - Potentially establish an updateScript standard to avoid problems like, relates to Flakes too - What to do with different versions, e.g. `wlroots = wlroots_0_14`? This goes into version resolution, a different problem to fix -- What to do about e.g. `libsForQt5.callPackage`? This goes into overrides, a different problem to fix +- What to do about e.g. `python3Packages.callPackage`? This goes into overrides, a different problem to fix - What about aliases like `jami-daemon = jami.jami-daemon`? - What about `recurseIntoAttrs`? Not single packages, package sets, another problem From 7a03d43c3f18b09dd0fdd9f5db138b6805968521 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 9 Jun 2023 21:02:12 +0200 Subject: [PATCH 23/25] Explain unit and add more alternatives --- rfcs/0140-simple-package-paths.md | 45 ++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 5d57b5d68..4e2b79c92 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -44,6 +44,16 @@ This standard is internal to Nixpkgs and not exposed as public interface. This standard must be documented in the Nixpkgs manual. This PR will be backported to the stable release in order to ensure that backports of new packages work. +### The name "unit" + +The term "unit" is chosen partly because it's not associated with any pre-existing assumptions about what it means, which should cause people unfamiliar with this standard to read the documentation. + +However additionally it makes sense to view unit directories as units, because they are discrete entities distinct from other entities of the same type. + +Furthermore, we envision that in the future we could extend the unit directory standard to not just include a package definition for each unit, but also other parts such as NixOS modules, library components, tests, etc. In this case `unit` would fit even better and could be described as + +> A collection of standardized files related to the same software component + ### File structure Create the initially-empty directory `pkgs/unit`, called _unit base directory_, in Nixpkgs. @@ -237,14 +247,31 @@ Alternatives: - Use `pkgs` instead, so that the `${shard}`'s are siblings to the other current directories in `pkgs` such as `top-level`, with the intention that the other directories would be hopefully removed at some point, then only leaving the shards in `pkgs` - (+) If we remove the other directories at some point, only the `${shard}`'s will be left in `pkgs` - (-) This leads to ambiguities between the directories from the standard and the other directories, requiring special handling in the code and CI, leading to complexities. + - (-) This makes it hard to pick out the few non-shard directories in directory listings since they will be interleaved with the ~700 shards. - (-) This would be harder to document and explain to people, since one always has to disregard all non-unit directories, with no obvious justification - (-) Currently we cannot apply this standard to all definitions in `pkgs`, in particular nested packages like `pythonPackages.*`, non-`callPackage`'d definitions like `copyDesktopItems` and non-derivations like `fetchFromGitHub`. Depending on how we want to handle those, it might make more sense to keep `pkgs/unit` or to use `pkgs` directly once all legacy paths are migrated away to another top-level directory, we don't yet know. `pkgs/unit` will be easier to migrate to `pkgs` than the other way around though. -- Another name than `unit`, proposed were `auto`, `pkg`, `mod`, `component`, `part`, `comp`, `app`, `simple` - - (-) `unit` is considered the most appropriate because a "unit" typically refers to a discrete, indivisible entity that is distict from other entities of the same type. - - (-) In addition we envision that in the future we would extend the unit directory standard to not just include a package definition for each unit, but also other parts such as NixOS modules, library components, tests, etc. In this case `unit` would fit even better and could be described as - > A collection of standardized files related to the same software component - + - (-) Causes poor auto-completion for the existing directories +- A variation of the above that improves on this is altering the shards to be prefixed with `_` so that they're always ordered together and not interleaved with non-shards. Non-shards would still be at the bottom of file listings though, but at least together. It shares the same other problems however. +- Various proposals: `pkgs/auto`, `pkgs/pkg`, `pkgs/mod`, `pkgs/component`, `pkgs/part`, `pkgs/comp`, `pkgs/app`, `pkgs/simple`, `pkgs/default`, `pkgs/shards`, `pkgs/top`, `pkgs/main` + - (-) Generally all of these names have some pre-existing assumptions about them, causing potential confusion when used for this concept + - `pkgs/default`: Could be interpreted to be some Nix-builtin magic that defaults to that folder. Could also be interpreted as "this is where the default packages go", which then raises the question "which packages are part of the default ones?" + - `pkgs/shards`: The sharding is a self-evident implementation detail, it shouldn't be repeated + - `pkgs/simple`: Implies that there's a complicated way to declare packages, which there currently is, but it's something we should get away from. + If we migrate everything, simple wouldn't mean anything anymore. + - `pkgs/top`: Easily confusable with `pkgs/top-level`, though `top` would make sense otherwise if we eventually moved all top-level packages to there. + - We could consider moving `pkgs/top-level` to another location then, e.g. `pkgs/package-sets`. + - `pkgs/main`: "If these are the main packages, where do the others go? What even is a main package?". Also could be confused with an entry-point +- `packages/${shard}` + - (+) Provides a clean starting point without having to be close to the legacy structure + - (-) This would be very confusing to newcomers because there's now both a `pkgs` and a `packages` directory in the Nixpkgs root, both spelled the same but very different contents. +- `pkgs/_` + - (+) Very short, fast to type (though that can depend on the keyboard layout) + - (+) Avoids naming discussions, because there is no name + - (-) Naming things is hard, but we shouldn't avoid the problem by giving it no name, which is arguably the worst name + - (-) Looks hacky and internal + - (+) Looks temporary, intention to move to `pkgs` itself once everything is sharded + - (-) It shouldn't be temporary. While we do hope to migrate all packages to some sharded form at some point, this may never happen, or the direction is completely changed, and this may take years to form. ## Alternate shard structure @@ -263,8 +290,14 @@ Alternatives: - (+) This would allow virtually a unlimited number of packages without performance problems - (-) It's hard to understand, type and implement, needs a special case for packages with few characters - E.g. `x` could go in `pkgs/unit/x-/--/x` - - (-) There's not enough packages even in Nixpkgs that a two-level structure would make sense. Most of the structure would only be filled by a couple entries. + - (-) There's not enough packages even in Nixpkgs that a two-level 4-letter structure would make sense. Most of the structure would only be filled by a couple entries. - (-) Even Git only uses 2-letter prefixes for its objects hex hashes +- Use two-letter prefixes split into two directories, like `pkgs/unit/h/e/hello` + - (+) Allows easy traversal by clicking on GitHub file listings, shard directories being limited to under 40 children + - (-) Requires special-casing single-letter attribute names + - (+) There's currently only 6 such cases, which could be handled on a one-off basis + - (-) Makes auto-completion worse, having to tab-complete once more + - (-) Makes it harder to create shards: if a shard doesn't exist yet, it has to be created with either one or two `mkdir`'s, or a `mkdir -p` - 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`. From 13c4f392d8bae0c3d5b3a69364b0613853f700dd Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 16 Jun 2023 14:44:17 +0200 Subject: [PATCH 24/25] unit -> by-name, remove "standard" And some very minor changes --- rfcs/0140-simple-package-paths.md | 136 +++++++++++++++--------------- 1 file changed, 67 insertions(+), 69 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 4e2b79c92..52dee5ff5 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -6,12 +6,12 @@ | pre-RFC reviewers | Thomas Bereknyei (@tomberek), John Ericson (@Ericson2314), Alex Ameen (@aakropotkin) | | shepherd-team | @phaer @06kellyjac @aakropotkin @piegamesde | | shepherd-leader | - | -| related-issues | https://github.com/NixOS/nixpkgs/pull/211832 | +| related-issues | https://github.com/NixOS/nixpkgs/pull/237439, https://github.com/NixOS/nixpkgs/pull/211832 | # Summary [summary]: #summary -Auto-generate trivial top-level attribute definitions in Nixpkgs' `pkgs/top-level/all-packages.nix` from a Nixpkgs-internally standardised directory structure that matches the attribute name. +Auto-generate trivial top-level attribute definitions in Nixpkgs' `pkgs/top-level/all-packages.nix` from a directory structure that matches the attribute name. This makes it much easier to contribute new packages, since there's no more guessing needed as to where the package should go, both in the ad-hoc directory categories and in `all-packages.nix`. # Motivation @@ -37,70 +37,61 @@ This makes it much easier to contribute new packages, since there's no more gues This RFC consists of two parts, each of which is implemented with a PR to Nixpkgs. These PR's should be done after a release to maximize the testing period and minimize merge conflicts. -## PR 1: The unit directory standard +## PR 1: The directory structure -This part establishes the new _unit directory standard_ in Nixpkgs. -This standard is internal to Nixpkgs and not exposed as public interface. -This standard must be documented in the Nixpkgs manual. +This part establishes the new directory structure in Nixpkgs. +This directory structure is internal to Nixpkgs and not exposed as public interface. +This directory structure must be documented in the Nixpkgs manual. This PR will be backported to the stable release in order to ensure that backports of new packages work. -### The name "unit" - -The term "unit" is chosen partly because it's not associated with any pre-existing assumptions about what it means, which should cause people unfamiliar with this standard to read the documentation. - -However additionally it makes sense to view unit directories as units, because they are discrete entities distinct from other entities of the same type. - -Furthermore, we envision that in the future we could extend the unit directory standard to not just include a package definition for each unit, but also other parts such as NixOS modules, library components, tests, etc. In this case `unit` would fit even better and could be described as - -> A collection of standardized files related to the same software component - ### File structure -Create the initially-empty directory `pkgs/unit`, called _unit base directory_, in Nixpkgs. +Create the initially-empty _base directory_ `pkgs/by-name` in Nixpkgs. Check the following using CI: -- The unit base directory must only contain subdirectories of the form `${shard}/${name}`, called _unit directories_. -- The `name`'s of unit directories must be unique when lowercased +- The base directory must only contain subdirectories of the form `${shard}/${name}`, called _package directories_. +- The `name`'s of package directories must be unique when lowercased - `name` is a string only consisting of the ASCII characters `a-z`, `A-Z`, `0-9`, `-` or `_`. - `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. -- Each unit directory must contain a `package.nix` file and may contain arbitrary other files. +- Each package directory must contain a `package.nix` file and may contain arbitrary other files. ### Semantics -Introduce code to automatically define `pkgs.${name}` for each unit directory as a value equivalent to +Introduce code to automatically define `pkgs.${name}` for each package directory as a value equivalent to ```nix -pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix { } +pkgs.callPackage pkgs/by-name/${shard}/${name}/package.nix { } ``` Optionally there may also be an overriding definition of `pkgs.${name}` in `pkgs/top-level/all-packages.nix` equivalent to ```nix -pkgs.callPackage pkgs/unit/${shard}/${name}/package.nix args +pkgs.callPackage pkgs/by-name/${shard}/${name}/package.nix args ``` with an arbitrary `args`. -Check the following using CI for each unit directory: +Check the following using CI for each package directory: - `pkgs.${name}` is defined as above, either automatically or with some `args` in `pkgs/top-level/all-packages.nix`. - `pkgs.${name}` is a [derivation](https://nixos.org/manual/nix/stable/glossary.html#gloss-derivation). -- The `package.nix` file evaluated from `pkgs.${name}` must not access files outside its unit directory. +- The `package.nix` file evaluated from `pkgs.${name}` must not access files outside its package directory. ## PR 2: Automated migration -Automatically migrate to the unit directory standard for all _satisfiying definitions_ `pkgs.${name}`, meaning derivations defined as above using `callPackage`. +Automatically migrate to new directory structure for all _satisfiying definitions_ `pkgs.${name}`, meaning derivations defined as above using `callPackage`. However automatic migration is only possible if: - Files don't need to be changed, only moved, with the exception of `pkgs/top-level/all-packages.nix` - The Nixpkgs package evaluation result does not change All satisfying definitions that can't be automatically migrated due to the above restrictions will be added to a CI exclusion list. -CI is added to ensure that all satisfying definitions except the CI exclusion list must be using the unit directory standard. -This means that the unit directory standard becomes mandatory for new satisfying definitions after this PR. +CI is added to ensure that all satisfying definitions except the CI exclusion list must be using the new directory structure. +This means that the new directory structure becomes mandatory for new satisfying definitions after this PR. The CI exclusion list should be removed eventually once the non-automatically-migratable satisfying definitions have been manually migrated. Only in very limited circumstances is it allowed to add new entries to the CI exclusion list. Non-automatic updates may also be done to ensure further correctness, such as - [GitHub's CODEOWNERS](https://github.com/NixOS/nixpkgs/blob/master/.github/CODEOWNERS) - Update scripts like [this](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/applications/version-management/cz-cli/generate-dependencies.sh) +- The Nixpkgs manual like [here](https://github.com/NixOS/nixpkgs/blob/4c8ca604aef8204145c185c89cc52ee54dd7fc1a/doc/contributing/quick-start.chapter.md#L27) This PR [will cause merge conflicts](https://github.com/nixpkgs-architecture/nixpkgs/pull/2) with all existing PRs that modify moved files, however they can trivially be rebased using `git rebase && git push -f`. Because of this, merging of this PR should be widely announced with a pinned issue on the Nixpkgs issue tracker and a Discourse post. @@ -109,14 +100,14 @@ Additionally this PR can benefit from being merged after a release due to the de ## Examples [examples]: #examples -To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/unit/fo/foobar/package.nix`. +To add a new package `pkgs.foobar` to Nixpkgs, one only needs to create the file `pkgs/by-name/fo/foobar/package.nix`. No need to find an appropriate category nor to modify `all-packages.nix` anymore. -With some packages, the `pkgs/unit` directory may look like this: +With some packages, the `pkgs/by-name` directory may look like this: ``` pkgs -└── unit +└── by-name ├── _0 │ ├── _0verkill │ └── _0x @@ -137,7 +128,7 @@ pkgs ## Shard distribution -The shard structure nesting unit directories under their lowercased two-letter prefix [leads to a distribution](https://gist.github.com/infinisil/95c7013db62e9f23ab2bc92165a37221) into shards as follows: +The sharded structure [leads to a distribution](https://gist.github.com/infinisil/95c7013db62e9f23ab2bc92165a37221) as follows: - There's 17305 total non-alias top-level attribute names in Nixpkgs revision [6948ef4deff7](https://github.com/nixos/nixpkgs/commit/6948ef4deff7a72ebe5242244bd3730e8542b925) - These are split into 726 shards - The top three shards are: @@ -146,10 +137,10 @@ The shard structure nesting unit directories under their lowercased two-letter p - "co": 252 values - There's only a single directory with over 1 000 entries, which is notably GitHub's display limit, so this means only 92 attributes would be harder to see on GitHub -These stats are also similar for other package sets for if this standard were to be adopted for them in the future. +These stats are also similar for other package sets for if directory structure were to be adopted for them in the future. ## Migration size -Due to the limitations of the standard, only a limited set of top-level attributes can be automatically migrated: +Due to the limitations of the new directory structure, only a limited set of top-level attributes can be automatically migrated: - No attributes that aren't derivations like `pkgs.fetchFromGitHub` or `pkgs.python3Packages` - No attributes defined using non-`pkgs.callPackage` functions like `pkgs.python3Packages.callPackage` or `pkgs.haskellPackages.callPackage`. In the future we might consider having a separate namespace for such definitions. @@ -159,7 +150,7 @@ Concretely this [can be computed](https://gist.github.com/infinisil/4f2bd165c260 And the initial automatic migration will be a bit more limited due to the additional constraints: - No attributes that share common files with other attributes like [`pkgs.readline`](https://github.com/NixOS/nixpkgs/tree/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/development/libraries/readline) - No attributes that references files from other packages like [`pkgs.gettext`](https://github.com/NixOS/nixpkgs/blob/cb2d5a2fa9f2fa6dd2a619fc3be3e2de21a6a2f4/pkgs/development/libraries/gettext/default.nix#L60) -These attributes will need to be moved to the standard manually with some arguably-needed refactoring to improve reusability of common files. +These attributes will need to be moved to the new directory structure manually with some arguably-needed refactoring to improve reusability of common files. ## Package locations @@ -172,12 +163,12 @@ These attributes will need to be moved to the standard manually with some arguab ## `callPackage` with `nix-build --expr` -A commonly recommended way of building package directories in Nixpkgs is to use `nix-build --expr 'with import {}; callPackage pkgs/applications/misc/hello {}'`. -Since the path changes `package.nix` is now used, this becomes like `nix-build --expr 'with import {}; callPackage pkgs/unit/he/hello/package.nix {}'`, which is harder for users. +A commonly recommended way of building current package directories in Nixpkgs is to use `nix-build --expr 'with import {}; callPackage pkgs/applications/misc/hello {}'`. +Since the path changes `package.nix` is now used, this becomes like `nix-build --expr 'with import {}; callPackage pkgs/by-name/he/hello/package.nix {}'`, which is harder for users. However, calling a path like this is an anti-pattern anyway, because it doesn't use the correct Nixpkgs version and it doesn't use the correct argument overrides. The correct way of doing it was to add the package to `all-packages.nix`, then calling `nix-build -A hello`. This `nix-build --expr` workaround is partially motivated by the difficulty of knowing the mapping from attributes to package paths, which is what this RFC improves upon. -By teaching users that `pkgs/unit//` corresponds to `nix-build -A `, the need for such `nix-build --expr` workarounds should disappear. +By teaching users that `pkgs/by-name//` corresponds to `nix-build -A `, the need for such `nix-build --expr` workarounds should disappear. ## Manual removal of custom arguments @@ -211,17 +202,17 @@ Because of this, there's the pattern of duplicating the `callPackage` call with } ``` -The semantics of how unit directories are checked by CI do allow the definition of package variants from unit directories: +The semantics of how package directories are checked by CI do allow the definition of package variants from package directories: ```nix { - graphviz_nox = callPackage ../unit/gr/graphviz/package.nix { withXorg = false; }; + graphviz_nox = callPackage ../by-name/gr/graphviz/package.nix { withXorg = false; }; } ``` # Drawbacks [drawbacks]: #drawbacks -- This standard can only be used for top-level packages using `callPackage`, so not for e.g. `python3Packages.requests` or a package defined using `haskellPackages.callPackage` +- This directory structure can only be used for top-level packages using `callPackage`, so not for e.g. `python3Packages.requests` or a package defined using `haskellPackages.callPackage` - It's not possible anymore to be a [GitHub code owner](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) of category directories. - The existing categorization of packages gets lost. Counter-arguments: - It was never that useful to begin with. @@ -236,24 +227,31 @@ The semantics of how unit directories are checked by CI do allow the definition # Alternatives [alternatives]: #alternatives -## A different unit base directory +## A different base directory -Context: `pkgs/unit` is the unit base directory +Context: `pkgs/by-name` is the base directory Alternatives: -- Use `unit` in the root directory instead +- Use `by-name` in the root directory instead - (+) This is future proof in case we want to make the directory structure more general purpose - (-) We don't yet know if we want that, so this is out of scope for now - Use `pkgs` instead, so that the `${shard}`'s are siblings to the other current directories in `pkgs` such as `top-level`, with the intention that the other directories would be hopefully removed at some point, then only leaving the shards in `pkgs` - (+) If we remove the other directories at some point, only the `${shard}`'s will be left in `pkgs` - - (-) This leads to ambiguities between the directories from the standard and the other directories, requiring special handling in the code and CI, leading to complexities. + - (-) This leads to ambiguities between the directories from the new directory structure and the other directories, requiring special handling in the code and CI, leading to complexities. - (-) This makes it hard to pick out the few non-shard directories in directory listings since they will be interleaved with the ~700 shards. - - (-) This would be harder to document and explain to people, since one always has to disregard all non-unit directories, with no obvious justification - - (-) Currently we cannot apply this standard to all definitions in `pkgs`, in particular nested packages like `pythonPackages.*`, non-`callPackage`'d definitions like `copyDesktopItems` and non-derivations like `fetchFromGitHub`. - Depending on how we want to handle those, it might make more sense to keep `pkgs/unit` or to use `pkgs` directly once all legacy paths are migrated away to another top-level directory, we don't yet know. `pkgs/unit` will be easier to migrate to `pkgs` than the other way around though. + - (-) This would be harder to document and explain to people, since one always has to disregard all non-sharded directories, with no obvious justification + - (-) Currently we cannot apply this directory structure to all definitions in `pkgs`, in particular nested packages like `pythonPackages.*`, non-`callPackage`'d definitions like `copyDesktopItems` and non-derivations like `fetchFromGitHub`. + Depending on how we want to handle those, it might make more sense to keep `pkgs/by-name` or to use `pkgs` directly once all legacy paths are migrated away to another top-level directory, we don't yet know. `pkgs/by-name` will be easier to migrate to `pkgs` than the other way around though. - (-) Causes poor auto-completion for the existing directories - A variation of the above that improves on this is altering the shards to be prefixed with `_` so that they're always ordered together and not interleaved with non-shards. Non-shards would still be at the bottom of file listings though, but at least together. It shares the same other problems however. -- Various proposals: `pkgs/auto`, `pkgs/pkg`, `pkgs/mod`, `pkgs/component`, `pkgs/part`, `pkgs/comp`, `pkgs/app`, `pkgs/simple`, `pkgs/default`, `pkgs/shards`, `pkgs/top`, `pkgs/main` +- `pkgs/unit`: This was the name initially used by the RFC until `by-name` was proposed and favored. + - (+) It's not associated with any pre-existing assumptions about what it means, which should cause people unfamiliar with this directory structure to read the documentation. + - (-) This is however also a disadvantage, the name doesn't inform people anything about what it does + - (-) Systemd also has the term "unit", which could be confused with this + - (+) It makes sense to view package directories as units, because they are discrete entities distinct from other entities of the same type + - (+) We envision that in the future we could extend the directory structure to not just include a package definition for each directory, but also other parts such as NixOS modules, library components, tests, etc. In this case `unit` would fit even better and could be described as + > A collection of standardized files related to the same software component +- Various other proposals: `pkgs/auto`, `pkgs/pkg`, `pkgs/mod`, `pkgs/component`, `pkgs/part`, `pkgs/comp`, `pkgs/app`, `pkgs/simple`, `pkgs/default`, `pkgs/shards`, `pkgs/top`, `pkgs/main` - (-) Generally all of these names have some pre-existing assumptions about them, causing potential confusion when used for this concept - `pkgs/default`: Could be interpreted to be some Nix-builtin magic that defaults to that folder. Could also be interpreted as "this is where the default packages go", which then raises the question "which packages are part of the default ones?" - `pkgs/shards`: The sharding is a self-evident implementation detail, it shouldn't be repeated @@ -275,10 +273,10 @@ Alternatives: ## Alternate shard structure -Context: The structure is `pkgs/unit/${shard}/${name}` with `shard` being the lowercased two-letter prefix of `name`. +Context: The structure is `pkgs/by-name/${shard}/${name}` with `shard` being the lowercased two-letter prefix of `name`. Alternatives: -- A flat directory, where `pkgs.hello` would be in `pkgs/unit/hello`. +- A flat directory, where `pkgs.hello` would be in `pkgs/by-name/hello`. - (+) Simpler for the user and code. - (-) The GitHub web interface only renders the first 1 000 entries when browsing directories, which would make most packages inaccessible in this way. - (+) This feature is not used often. @@ -286,27 +284,27 @@ Alternatives: - (-) Bad because it makes `git` and file listings slower. - Use three-letter or four-letter prefixes. - (-) Also leads to directories containing more than 1 000 entries, see above. -- Use multi-level structure, e.g. a two-level two-letter prefix structure where `hello` is in `pkgs/unit/he/ll/hello` +- Use multi-level structure, e.g. a two-level two-letter prefix structure where `hello` is in `pkgs/by-name/he/ll/hello` - (+) This would allow virtually a unlimited number of packages without performance problems - (-) It's hard to understand, type and implement, needs a special case for packages with few characters - - E.g. `x` could go in `pkgs/unit/x-/--/x` + - E.g. `x` could go in `pkgs/by-name/x-/--/x` - (-) There's not enough packages even in Nixpkgs that a two-level 4-letter structure would make sense. Most of the structure would only be filled by a couple entries. - (-) Even Git only uses 2-letter prefixes for its objects hex hashes -- Use two-letter prefixes split into two directories, like `pkgs/unit/h/e/hello` +- Use two-letter prefixes split into two directories, like `pkgs/by-name/h/e/hello` - (+) Allows easy traversal by clicking on GitHub file listings, shard directories being limited to under 40 children - (-) Requires special-casing single-letter attribute names - (+) There's currently only 6 such cases, which could be handled on a one-off basis - (-) Makes auto-completion worse, having to tab-complete once more - (-) Makes it harder to create shards: if a shard doesn't exist yet, it has to be created with either one or two `mkdir`'s, or a `mkdir -p` - 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`. + E.g. `pkgs.foobar` could be in `pkgs/by-name/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/by-name/fo/foobar`. - (-) The structure depends not only on the name of the package then, making it harder to find packages again and figure out where they should go - (-) Complex to implement ## Alternate `package.nix` filename -Context: The only file that has to exist in unit directories is `package.nix`, it must contain a function suitable for `callPackage`. +Context: The only file that has to exist in package directories is `package.nix`, it must contain a function suitable for `callPackage`. Alternatives: - `default.nix` @@ -315,7 +313,7 @@ Alternatives: - 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`. - (+) But there's still some support for `all-packages.nix` for custom arguments, which requires people to type out the name - (-) This is hopefully only temporary, in the future we should fully get rid of `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. + - 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/by-name/hell/hello` equally broken regardless of file name. - (-) Not using `default.nix` frees up `default.nix` for an expression that is actually buildable, e.g. `(import ../.. {}).hello`, although we don't yet have a use case for this that isn't covered by `nix-build ../.. -A `. - (-) Using `default.nix` would tempt users to invoke `nix-build .`, which wouldn't work and making package functions auto-callable is a known anti-pattern. - `pkg-fun[c].nix` @@ -337,44 +335,44 @@ Alternative: Context: It's possible to override the default `{ }` argument to `callPackage` by manually specifying the full definition in `all-packages.nix` -The alternative is to not allow that, requiring that `pkgs.${name}` corresponds directly to `callPackage pkgs/unit/${shard}/${name}/package.nix { }`. -- (-) It's harder to explain to beginners whether their package can use the unit directory standard or not -- (+) The direct correspondance ensures that the unit directory contains all information about the package, which is very intuitive +The alternative is to not allow that, requiring that `pkgs.${name}` corresponds directly to `callPackage pkgs/by-name/${shard}/${name}/package.nix { }`. +- (-) It's harder to explain to beginners whether their package can use the new directory structure or not +- (+) The direct correspondance ensures that the package directory contains all information about the package, which is very intuitive - (-) We're not at the point where we can have that though, custom arguments don't have a good replacement yet -- (-) If a package previously didn't need custom arguments, it would be moved to a unit directory. But when the need for a custom argument arises, it then requires moving it out from the unit directory and into the freeform structure of `pkgs/` again. +- (-) If a package previously didn't need custom arguments, it would be moved to the new directory structure. But when the need for a custom argument arises, it then requires moving it out from new directory structure and into the freeform structure of `pkgs/` again. - (+) It's easier to relax restrictions than to impose new ones ## Reference check -Context: There's a [requirement](#user-content-req-ref) to check that unit directories can't access paths outside themselves. +Context: There's a [requirement](#user-content-req-ref) to check that package directories can't access paths outside themselves. Alternatives: - Don't have this requirement - (-) Doesn't discourage the use of file paths as an API. - (-) Makes further migrations to different file structures harder. -- Make the requirement also apply the other way around: Files outside the unit directory cannot access files inside it, with `package.nix` being the only exception, and only for the one attribute in `all-packages.nix` +- Make the requirement also apply the other way around: Files outside the package directory cannot access files inside it, with `package.nix` being the only exception, and only for the one attribute in `all-packages.nix` - (-) Enforcing this requires a global view of Nixpkgs, which is nasty to implement - (-) [Package variants](#package-variants) would not be possible to define -## Allow `callPackage` arguments to be specified in `/args.nix` +## Allow `callPackage` arguments to be specified in `args.nix` Context: Custom `callPackage` arguments have to be added to `all-packages.nix` Alternative: Expand the auto-calling logic according to: -Unit directories are automatically discovered and transformed to a definition of the form +Package directories are automatically discovered and transformed to a definition of the form ``` # If args.nix doesn't exist -pkgs.${name} = pkgs.callPackage ${unitDir}/package.nix {} +pkgs.${name} = pkgs.callPackage ${packageDir}/package.nix {} # If args.nix does exists -pkgs.${name} = pkgs.callPackage ${unitDir}/package.nix (import ${unitDir}/args.nix pkgs); +pkgs.${name} = pkgs.callPackage ${packageDir}/package.nix (import ${packageDir}/args.nix pkgs); ``` - (+) It makes another class of packages uniform, by picking a solution with restricted expressive power. - (-) It does not solve the contributor experience problem of having too many rules. `args.nix` is another pattern that contributors need to learn how to use, as we have seen that it is not immediately obvious to everyone how it works. - (+) A CI check can mitigate the possible lack of uniformity, and we see a simple implementation strategy for it. -- (-) Complicates the unit directory structure with an optional file +- (-) Complicates the directory structure with an optional file # Unresolved questions [unresolved]: #unresolved-questions From f15c1cf8a73013d32092148147abaaddbf1b5e75 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 16 Jun 2023 18:03:41 +0200 Subject: [PATCH 25/25] Apply suggestions from code review Remove the barely used term "base directory" Co-authored-by: Robert Hensing --- rfcs/0140-simple-package-paths.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rfcs/0140-simple-package-paths.md b/rfcs/0140-simple-package-paths.md index 52dee5ff5..0926166a7 100644 --- a/rfcs/0140-simple-package-paths.md +++ b/rfcs/0140-simple-package-paths.md @@ -46,10 +46,10 @@ This PR will be backported to the stable release in order to ensure that backpor ### File structure -Create the initially-empty _base directory_ `pkgs/by-name` in Nixpkgs. +Create the initially-empty `pkgs/by-name` directory in Nixpkgs, and migrate the `hello` package into it. Check the following using CI: -- The base directory must only contain subdirectories of the form `${shard}/${name}`, called _package directories_. +- `pkgs/by-name` must only contain subdirectories of the form `${shard}/${name}`, called _package directories_. - The `name`'s of package directories must be unique when lowercased - `name` is a string only consisting of the ASCII characters `a-z`, `A-Z`, `0-9`, `-` or `_`. - `shard` is the lowercased first two letters of `name`, expressed in Nix: `shard = toLower (substring 0 2 name)`. @@ -227,9 +227,9 @@ The semantics of how package directories are checked by CI do allow the definiti # Alternatives [alternatives]: #alternatives -## A different base directory +## An alternative to the `pkgs/by-name` location -Context: `pkgs/by-name` is the base directory +Context: this directory contains the shards, which contain the package directories. We could move the shards to a different location. Alternatives: - Use `by-name` in the root directory instead