Sharding and other updates #20
Conversation
- Describe sharding as discussed in the last NAT meeting
- Refines which packages are eligible for this transition
- Exclude functions and packages that aren't self-contained regarding
references
- Add some TODO's
- General text improvements
- Add an example tree
infinisil
requested review from
tomberek,
roberth,
Ericson2314 and
aakropotkin
|
I updated this PR to reflect the most recent discussions we've been having and addressing all reviews. Please review again @roberth, @SuperSandro2000 but also others in the @nixpkgs-architecture/team. |
README.md
Outdated
| 1. <a id="criteria-1"/> Is defined in `pkgs/top-level/all-packages.nix` | ||
| (necessary so that the overlay containing the automatically discovered packages can be ordered directly before the `all-packages.nix` overlay without changing any behavior) | ||
| 2. <a id="criteria-2"/> Is defined to be equal to `pkgs.callPackage <path> { }` | ||
| 3. <a id="criteria-3"/> All transitively referenced paths from the default Nix file of `<path>` are under the same directory as the default Nix file and can be moved around together without breaking any references in other Nix files (except the one reference in `pkgs/top-level/all-packages.nix`). |
There was a problem hiding this comment.
This is an implementation detail, not an intentional restriction.
Please separate the end goal from what the current tool can do.
There was a problem hiding this comment.
I guess you're thinking of the tool doing automatic fixing of references in Nix files when moving paths around? At least in all the cases I've seen, that's a violation of API boundaries: Packages shouldn't rely on paths outside of their own "unit" directory. And similarly the other way around. So as far as I can see, it's reasonable that the tool only supports this, and not just for the sake of simplifying the implementation.
There was a problem hiding this comment.
I think the reader would like to know in a bit more detail what Nixpkgs will look like before diving into the technical details of the process of getting there.
I guess you're thinking of the tool doing automatic fixing of references in Nix files when moving paths around?
That would be nice, but isn't really the point.
implementation detail, not an intentional restriction
Suppose I want to move cassandra into pkgs/unit. Its files are referenced by pkgs.cassandra and pkgs.cassandra_3_11. Am I not allowed to move it, because it violates (2) and (3)?
There was a problem hiding this comment.
I rewrote the details section a bunch to not mention implementation details anymore.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
README.md
Outdated
|
|
||
| # Interactions | ||
| [interactions]: #interactions | ||
|
|
||
| - `nix edit` is unaffected, since it uses a packages `meta.position` to get the file to edit | ||
| - `nix edit` is unaffected, since it uses a packages `meta.position` to get the file to edit. | ||
| Though with this RFC `nix edit` could be updated to not have to rely on that anymore for the packages in the new hierarchy in nixpkgs. |
There was a problem hiding this comment.
| Though with this RFC `nix edit` could be updated to not have to rely on that anymore for the packages in the new hierarchy in nixpkgs. |
I don't think it can or should.
There was a problem hiding this comment.
If nixpkgs sufficiently exposes it as a stable API, I think it could. The benefit would be that nix edit could also work on packages where meta.position can't be evaluated, e.g. due to syntax errors. Additionally, nix edit might open the wrong file due to a builder not properly populating meta.position, this problem would also be avoided.
There was a problem hiding this comment.
We discussed this in the last NAT meeting, I removed the sentence now
README.md
Outdated
| This isn't done because it [turns out](https://github.com/nixpkgs-architecture/simple-package-paths/issues/14) that this criteria indicates the file structure being used as an API interface. | ||
| By manually refactoring the Nix code to not rely on this anymore, you can increase code quality/reusability/clarity and then do the transition described in the RFC. | ||
| - Use a different sharding scheme than `<4-prefix name>`. | ||
| Discussions regarding this can be seen [here](https://github.com/nixpkgs-architecture/simple-package-paths/issues/1), [NAT meeting #18](https://github.com/nixpkgs-architecture/meetings/blob/6282b0c6bbc47b6f1becd155586c79728eddefc9/2022-11-21.md) and [here](https://github.com/nixpkgs-architecture/simple-package-paths/pull/20#discussion_r1029004083) |
There was a problem hiding this comment.
Should be summarized here.
There was a problem hiding this comment.
I redid the alternatives section, including writing a summary of this one here
Co-Authored-By: Robert Hensing <robert@roberthensing.nl>
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2022-12-19-nixpkgs-architecture-team-meeting-22/24125/1 |
README.md
Outdated
| These requirements will be checked using CI: | ||
| 1. <a id="req-1"/> The `pkgs/unit` directory must only contain unit directories, and only in subdirectories of the form `${substring 0 4 name}/${name}`. | ||
| 2. <a id="req-2"/> Files outside a unit directory must not reference files inside that unit directory, and the other way around. | ||
| 3. <a id="req-3"/> Each unit directory must have a `pkg-fun.nix` file such that `pkgs.callPackage ./pkg-fun.nix {}` evaluates to a derivation. | ||
| 4. <a id="req-4"/> Packages defined by unit directories must not be defined or overridden anywhere else, such as in `pkgs/top-level/all-packages.nix`. |
There was a problem hiding this comment.
I also codified these requirements that should be checked by CI
- So that people don't pollute the
pkgs/unitdirectory with wrong shards - Makes the unit self-contained which is a good thing to have in general, and we get this check essentially for free with the tooling for the transition. If this becomes a point of contention we could also remove this requirement though.
- So we can always
callPackage - So that we don't have one person adding a file in
all-packages.nixand another inpkgs/unit, and one of them being ignored without any indication.
Discussed during NAT meeting #23 Co-authored-by: John Ericson <git@JohnEricson.me>
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2022-01-03-nixpkgs-architecture-team-meeting-23/24404/1 |
Closes #16, closes #19