Skip to content

pkgs/README.md: discourage overrideAttrs within Nixpkgs #421201

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

Merged
philiptaron merged 1 commit into from
Sep 7, 2025
Merged

pkgs/README.md: discourage overrideAttrs within Nixpkgs #421201

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
17 changes: 17 additions & 0 deletions pkgs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,23 @@ See the Nixpkgs manual for more details on [standard meta-attributes](https://ni

Import From Derivation can be worked around in some cases by committing generated intermediate files to version control and reading those instead.

## `overrideAttrs` and `overridePythonAttrs`

Please do not introduce new uses of `overrideAttrs` or `overridePythonAttrs` in Nixpkgs.
Copy link
Member

@dotlambda dotlambda Jul 1, 2025

Choose a reason for hiding this comment

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

I don't see why we wouldn't allow them for the specific use case of defining versioned attributes. Should we rather duplicate an entire expression even if only version and hash need to be changed?

Copy link
Member

@emilazy emilazy Jul 1, 2025

Choose a reason for hiding this comment

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

It would be better to introduce an .override parameter to select between the versions, or to have some other internal logic for it like a generic builder function. Actually I can’t remember seeing many instances of overrideAttrs being used this way at all; maybe it’s more common in the Python package set.

Copy link
Member Author

@alyssais alyssais Jul 2, 2025

Choose a reason for hiding this comment

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

This is also already addressed in the text, and it does not say to duplicate an entire expression:

If you need a different version of a dependency, first try modifying your package to work with the version in Nixpkgs — it's often not very hard! — and if that's not possible, try to factor out a function that can build multiple versions of the package, including the main version.

I'm open to feedback on that advice though! I agree with Emily that it's not common for centralized version overrides of packages (as opposed to sneaky hidden overrides in leaf packages) to use overrideAttrs in the first place. Again, if overridePythonAttrs is different, I'd be happy to exclude it and just address overrideAttrs for the time being.

Copy link
Member

@dotlambda dotlambda Jul 3, 2025

Choose a reason for hiding this comment

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

Here's an example:

nixpkgs/pkgs/top-level/python-packages.nix

Lines 3189 to 3197 in 30db601

cython_3_1 = cython.overridePythonAttrs rec {
version = "3.1.1";
src = pkgs.fetchFromGitHub {
owner = "cython";
repo = "cython";
tag = version;
hash = "sha256-KdRYPH3Do3KntgqLGIUSeD6DjmXNdFjI2ZSszzMjF6k=";
};
};

I prefer to keep using overridePythonAttrs here because we'll soon get rid of cython_3_1 again and I don't want to go back an forth between a normal expression and one that uses a "function that can build multiple versions of the package".
I also don't see why this situation would be unique to Python packages, see e.g.

nixpkgs/pkgs/by-name/ni/nim-unwrapped-2_0/package.nix

Lines 8 to 10 in 30db601

nim-unwrapped-2_2.overrideAttrs (
finalAttrs: previousAttrs: {
version = "2.0.16";

Copy link
Member Author

@alyssais alyssais Jul 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't want to go back an forth between a normal expression and one that uses a "function that can build multiple versions of the package".

I don't know about cython, but for things like compilers and interpreters more generally, my experience is that we are either consistently strict about only packaging a single version (like Rust), or we are open to packaging multiple versions, and usually do. In the latter case, I'd keep the generic function around even if there happens to only be a single version packaged for some amount of time, since it's likely that in the near future there will again be multiple versions. Nim looks like it would be one of those situations.

Copy link
Member

@dotlambda dotlambda Jul 3, 2025

Choose a reason for hiding this comment

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

Oh, I thought you meant overrideAttrs.

Copy link
Member

@emilazy emilazy Jul 4, 2025

Choose a reason for hiding this comment

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

@alyssais Maybe we should remove the verbiage about avoiding copy‐paste here? It’s generally better to do, but it’s not related to overrideAttrs specifically, and I feel there are some cases where overrideAttrs avoidance is most easily accomplished by copy‐paste (cf. the vendored package example in my tl;dr comment).

Copy link
Member Author

@alyssais alyssais Jul 4, 2025

Choose a reason for hiding this comment

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

Which verbiage specifically?

Copy link
Member

@emilazy emilazy Jul 4, 2025

Choose a reason for hiding this comment

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

Hmm, I misread it the first time I think. But I think we can make this paragraph more generic by just mentioning packaging the other version and not being prescriptive about how:

If you need a different version of a dependency, first try modifying your package to work with the version in Nixpkgs — it's often not very hard! — and if that's not possible, try to factor out a function that can build multiple versions of the package, including the main version.

Copy link
Member Author

@alyssais alyssais Jul 6, 2025

Choose a reason for hiding this comment

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

Done. You're both right that there's no one-size-fits-all approach here, so I've made it more vague.

These functions are useful for out-of-tree code because they allow easy overriding a package without changing its source in Nixpkgs, but when contributing to Nixpkgs you *can* change the source of other packages. So instead of using the escape hatch that is overriding, you should try to provide proper support for the functionality you need, in ways that are visible and can be understood and accounted for by the maintainers of the patched package.
Using `overrideAttrs` and `overridePythonAttrs` in Nixpkgs causes maintainability problems:

* It's easy for multiple packages to end up duplicating basically the same override without noticing.
* It's not clear when working on an overridden package that it's being overridden elsewhere in Nixpkgs, so `overrideAttrs` and `overridePythonAttrs` are fragile and can break accidentally when the overridden package is changed.
* Package maintainers will not be requested for review of overrides, even though they are likely to have important knowledge about the package.
* It is easy for overridden packages to be forgotten and remain around long after they are no longer necessary.
* Dependency closures end up being bigger than necessary due to unnecessarily including multiple versions of the same package.

Instead, keep all instances of the same package next to each other, and try to minimize how many different instances of a package are in Nixpkgs.
If you need a patch applied to a dependency, discuss with the maintainer of that dependency whether it would be acceptable to apply to the main version of the package.
If you need a different version of a dependency, first try modifying your package to work with the version in Nixpkgs — it's often not very hard! — and if that's not possible, try to factor out a function that can build multiple versions of the package, including the main version.
If you need to enable or disable optional functionality of a dependency, add an explicit flag to the package and use `override` instead.

## Sources

Always fetch source files using [Nixpkgs fetchers](https://nixos.org/manual/nixpkgs/unstable/#chap-pkgs-fetchers).
Expand Down
Loading