Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC 0140] Simple Package Paths #140

Merged
merged 25 commits into from
Jul 12, 2023
Merged

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Jan 30, 2023

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.
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.

Rendered

Implementation:

Nixpkgs Architecture Team

This RFC has been worked on openly in a repository by the new Nixpkgs Architecture Team in the last couple months. It is the first of hopefully many quality of life improvements to nixpkgs.

The team will continue meeting regularly (calendar), see here for more details, anybody is free to join and listen in or ask questions. Also feel free to join the Matrix room to discuss this RFC.

Shepherd team

Here are the shepherds with their nominations, all of which are now accepted.

Major RFC updates

@SuperSandro2000
Copy link
Member

This gets squash merged, right?

@Ericson2314
Copy link
Member

@SuperSandro2000 I believe it does get squashed.

@SuperSandro2000
Copy link
Member

I didn't read it fully but IMO we should link the tooling already developed to prove that the transition is easy and doable.

@infinisil
Copy link
Member Author

This gets squash merged, right?

Originally we wanted to maintain the commit history from the original repository, but it's probably better to just squash it beforehand, which I now did.

IMO we should link the tooling already developed to prove that the transition is easy and doable.

It does link to NixOS/nixpkgs#211832, which itself links to the (yet unfinished) tooling (also linked in the RFC description). I think that's good enough, especially since the tooling might move around. That PR does evaluate cleanly in CI though, which is already fairly good as a proof. I also have the time to finish that tooling to be fully in line with the RFC.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixpkgs-architecture-team-meeting-27-rfc-time/25095/1

rfcs/0140-simple-package-paths.md Outdated Show resolved Hide resolved
rfcs/0140-simple-package-paths.md Outdated Show resolved Hide resolved
rfcs/0140-simple-package-paths.md Outdated Show resolved Hide resolved
rfcs/0140-simple-package-paths.md Show resolved Hide resolved
rfcs/0140-simple-package-paths.md Outdated Show resolved Hide resolved
rfcs/0140-simple-package-paths.md Outdated Show resolved Hide resolved
rfcs/0140-simple-package-paths.md Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-init-generate-nix-packages-from-urls-with-hash-prefetching-dependency-inference-license-detection-and-more/25035/6

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-43/25185/1


- 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`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to nominate auto, to me this is more future-proof than unit so we can include multiple packages in one directory

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we can include multiple packages in one directory

How do you mean that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, we currently have element-desktop and element-web that share the same pin.json (or generic.nix in some other cases), in the future we might want to allow that to be defined in pkgs/unit, but I'm not quite sure the name unit fits the behavior of having multiple packages under the same directory, and auto is more future-proof in that case IMO

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yeah so this has been discussed before. I believe in the team the consensus was that multiple versions and variants should eventually end up in the same pkgs.element attribute, once we have the design for that figured out. In that case, all those variants would naturally be together in the pkgs/unit/el/element directory.

That's also kind of why unit was chosen, we talked about explaining it better before, something like:

unit: A collection of standardized files related to the same software component

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense to me, thanks for the explanation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming is really important, especially for something as ubiquitous. Please add the rationale for choosing "unit" over the other proposals, just as you did for all the other decisions. Otherwise it will produce guesswork, confusion, and needless resistance in readers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't dislike "auto" but I'd like to note that the name implies the package is automatically called. This means that packages with custom arguments would not fit in there anymore, unlike with the current proposal. Maybe they'd be put into "pkgs/manual" instead then? (which then opens up to confusion with manual as in documentation, etc.)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-looking-back-at-2022/25234/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-01-06-nixpkgs-architecture-team-meeting-28/25262/1

@AndersonTorres
Copy link
Member

How would (e.g.) Emacs and its ecosystem would behave on this new system?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-11-28-nixpkgs-architecture-team-meeting-46/36171/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/community-calendar/18589/97

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/ci-will-soon-enforce-pkgs-by-name-for-new-packages/38098/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-52/38343/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixpkgs-architecture-team-conclusion-and-prospective/41020/1

@0x4A6F 0x4A6F mentioned this pull request Mar 21, 2024
13 tasks
KAction pushed a commit to KAction/rfcs that referenced this pull request Apr 13, 2024
* RFC 140

Initialized from nixpkgs-architecture/simple-package-paths@01948e0

* Minor improvements

* Minor nits

* Update co-authors and add pre-RFC reviewers

* pkg-fun.nix -> package.nix

* 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.

* Re-add custom argument exception and various minor improvements

* shard distribution stats, cleanup, more uniformity

* Readd accidentally removed definition

* Mention package variants

* Minor moving and formatting

* Changes from feedback in the meeting

* Link to demonstration of cherry-picking without problems

* Link to demonstrates of problematic/non-problematic Git operations

* Names must be unique when lowercased

* Properly close invisible anchor

* Update summary to mention Nixpkgs more explicitly

* Include more arguments and counter-arguments for pkgs/unit alternatives

* Add shepherd team and nicks

* Convert frontmatter to a table

* Fix table rendering

* Minor fixups

Co-Authored-By: Robert Hensing <robert@roberthensing.nl>

* Explain unit and add more alternatives

* unit -> by-name, remove "standard"

And some very minor changes

* Apply suggestions from code review

Remove the barely used term "base directory"

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

---------

Co-authored-by: Robert Hensing <robert@roberthensing.nl>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/call-for-discussion-why-and-how-much-should-we-care-about-the-override-interfaces-part-i-duckstation-and-pcsx2/45117/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/announcing-determinate-nix/54709/58

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-we-shrunk-our-javascript-monorepo-git-size-by-94/55041/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.