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.

Sign up for GitHub

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

Jump to bottom

pkgs/by-name: document toplevel argument restrictions #325262

Closed
wants to merge 1 commit into from
Closed

pkgs/by-name: document toplevel argument restrictions #325262

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions pkgs/by-name/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ and override its value in [`pkgs/top-level/all-packages.nix`](../top-level/all-p
}
```

### Toplevel argument restrictions
The list of toplevel arguments should be restricted to packages defined in the
Copy link
Member

Choose a reason for hiding this comment

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

Well, technically nothing hinders someone of creating a package with the exact same name of a Boolean flag.

What about with-shell?
https://github.com/NixOS/nixpkgs/blob/nixos-24.05/pkgs/applications/misc/with-shell/default.nix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't have any "non-package args", so don't understand the question.

Copy link
Member

Choose a reason for hiding this comment

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

I am saying that with-shell is an existing package, and because of callPackage no one can use with-shell as a simple Boolean flag.

common nixpkgs package set, and simple `enable*` boolean flags.
Copy link
Member

Choose a reason for hiding this comment

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

Attributes of type string are employed to indicate options. Emacs expression accepts toolkit as "gtk", "motif", "athena"...

And Emacs is outside by-name.


As the expression is being `callPackage`'ed, both newly-defined packages, as
well as those added through the use of overlays will cause defaults to be
overwritten in a unexpected and hard-to-debug fashion.

In case you're intending to write more flexible package expressions, consider
Copy link
Member

Choose a reason for hiding this comment

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

This is not by-name's mistake. This is about callPackage. Whenever people use callPackage they are affected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed it's not by-name's mistake.

This also say it's because it's callPackage'd. I still think it's worth to write it down like this, because people tend to be too fancy with what they do in by-name, and restricting it only to packages defined in the top-level (without defaults) and to only boolean enable flags prevents this effectively.

Copy link
Contributor

@eclairevoyant eclairevoyant Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The main restriction with by-name is that you can't use a custom callPackage, there's nothing else that I know of affecting the override interface. Besides, there is an upcoming check to enforce moving to by-name when touching packages at the top-level, so this suggestion doesn't make sense.

Let's just drop the sentence and make the guidelines more explicit if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please propose exact wording in a new PR.

defining them outside `by-name`.

## Manual migration guidelines

Most packages are still defined in `all-packages.nix` and the [category hierarchy](../README.md#category-hierarchy).
Expand Down