Implicit attribute defaults/overrides inside package definition #273534

Atemu · 2023-12-11T13:01:04Z

Issue description

Currently, in order to override a package's arguments in order to i.e. pin a version of a library, you must do so at the callPackage location, usually all-packages.nix. This has a few issues:

all-packages is huge
- Easy to run into conflicts
- Chaotic/Confusing
- Hard to work with (can crash some LSPs)
Source of truth is split
- It is no longer obvious from looking at a package definition file what its arguments are (they might be overridden)
- Maintainers might forget/never now that overrides are in place (who here frequently looks inside all-packages for their packages?)
- It just feels wrong to have this info in a separate place
Requires explicit imports
- Works against [RFC 0140] Simple Package Paths rfcs#140
- The current workaround is to explicitly callPackage the by-name path in all-packages (officially documented)

I've been a fan of explicit requirements; i.e. depending on ffmpeg_5 rather than ffmpeg with an ffmpeg = ffmpeg_5; override in all-packages. This also has issues, the primary one being that overrides aren't stable anymore and are even less intuitive as you'd have to override i.e. ffmpeg_5 = ffmpeg_6;.

IMHO, the benefits outweighed the downsides as overrides aren't really a stable thing to begin with but I'd prefer to fix this issue "proper". In order to do that we must either put the package definition into all-packages (yeah, no) or put the overrides into package definitions.

This issue exists as a collection point for ideas on how to realise this.

Related discussion: #204303

The text was updated successfully, but these errors were encountered:

eclairevoyant · 2023-12-11T19:50:47Z

The first issue has another point - github won't even index the file for searching. Though it should be massively improved with the move to by-name

infinisil · 2023-12-12T17:43:16Z

Yeah this problem has been discussed with the Nixpkgs Architecture Team throughout RFC 140. I don't have any concrete proposals nor the resources to come up with one for now, but if anybody else does, it would be great to synchronise with the NAT for that.

roberth · 2023-12-12T19:48:33Z

dream2nix deps pattern may be interesting to at least pull the default overrides into the package file.

ghost · 2024-01-19T19:54:59Z

This also has issues, the primary one being that overrides aren't stable anymore and are even less intuitive as you'd have to override i.e. ffmpeg_5 = ffmpeg_6;.

⚠️ It would be really great if we could keep version numbers out of the attrnames. You've pointed out one of the problems this causes; there are lots of others. This frequently causes my local overlays to be silently ignored after a rebase -- wlroots is the worst offender.

Analogy to multi-platform packages

This has close parallels with how we handle platforms. We don't have separate toplevel ffmpeg_x86-linux and ffmpeg_aarch64-linux attributes -- instead, ffmpeg is a "spliced"¹ collection of packages, one for each platform.

We should do the same thing with versions. Each top level attribute should be not a single package (which hasn't been true since __spliced was added) but rather a collection of all available versions of that package.

Version overrides then become straightforward: you simply delete from the __versions set any versions you know won't work with your package. This is better-behaved in the case of layered overrides: unlike replacements, deletions commute with each other, so you get the same result regardless of the order in which you apply the overrides.

Proof of Concept

I have a prototype here but it's nowhere near PR-ready. I developed it against gcc, since that is by far the gnarliest case of multiversioned packaging in nixpkgs. Some repl to give an idea of what it feels like:

nix-repl> gcc
«derivation /nix/store/20z7kwqc1fp8v4fmmm1p9jmvig3r6cys-gcc-wrapper-13.2.0.drv»

nix-repl> gcc.__versions.best
«derivation /nix/store/20z7kwqc1fp8v4fmmm1p9jmvig3r6cys-gcc-wrapper-13.2.0.drv»

nix-repl> gcc.version
"13.2.0"

nix-repl> builtins.attrNames gcc.__versions
[ "10.5.0" "11.4.0" "12.3.0" "13.2.0" "4.8.5" "4.9.4" "6.5.0" "7.5.0" "8.5.0" "9.5.0" "__unfix__" "best" "extends" "require" "unstable" ]

nix-repl> gcc.__versions.require "<10"
«derivation /nix/store/xc0hx0ijajgxjvdlrv9k92xlwfvrbb2b-gcc-wrapper-9.5.0.drv»

nix-repl> (gcc.__versions.require "<10").version
"9.5.0"

nix-repl> (gcc.__versions.require "<10").__versions.best.version
"9.5.0"

Notice how there is a reference cycle pkg.__versions.best.__versions.best... so you can always get a reference to the version-set from any of its members and vice-versa.

Highlight commits in the PoC

lib.versions.makeVersioned, where the magic happens. This is just an early draft; there are several issues that need to be fixed but all of them seem straightforward.
- Note in particular the __versions.unstable attribute, which generalizes pkgs.default-gcc-version into a mechanism that can be applied to any package instead of just gcc.
Conversion of pkgs/development/compilers/gcc to __versions style, which was easier than I expected.
what all-packages.nix looks like after converting gcc. The remaining gcc${version} attributes are for legacy compatibility; they should then be inlined at their point of use, allowing to move the primary pkgs.gcc attribute out of all-packages.nix and into pkgs/by-name.

The main thing that's missing is that .overrideAttrs on a multiversion package needs to apply the override to every version, not just the best version.

Status

I have a bunch of other nix-related projects that are close to the finish line so unfortunately I am not going to be able to finish this in the next month or two. If anybody wants to steal this idea and run with it more power to them; otherwise I'll come back to it in the spring.

I am one of the loudest complainers when it comes to issues caused by __spliced, but those problems come from two aspects which don't apply to __versions: (a) the "unspliced packages" optimization turned into a huge footgun and (b) when splicing was added we tried to keep the old buildInputs/nativeBuildInputs interface for stdenv.mkDerivation instead of having a single deps attribute with package authors projecting out the desired element of __spliced. ↩

roberth · 2024-01-20T11:26:40Z

We really ought to have a place for abstract package information that isn't specific to a build of a package, or we'll keep incurring the performance and complexity cost of unused package evaluations / unnecessary overrides.

For instance, evaluating gcc.__versions.something requires that we evaluate gcc, as well as something, which has a real performance cost when applied at a scale that isn't tiny. Also overrideAttrs isn't particularly cheap. As for complexity, there's just less division of responsibilities, and the performance problem implies a strictness problem - e.g. gcc may fail to evaluate (well, more likely for other versioned packages perhaps). Also it's not possible to pass an exact version without the possibility that some code inspects its __versions to get something else instead. This could turn out a bit chaotic.

All that said, I'm glad to see some experimentation happen in this area.

#217243 was meant to mitigate some of this, but mkDerivation tech debt is bankrupt. I've put a plan in the bottom comments, but versioning should not be shoehorned into mkDerivation, regardless of whether we execute on such a plan or not.

nitpick: @amjoseph-nixpkgs It is my understanding that the convention around __ is that _ is reserved for libraries/frameworks/infra defined in expressions, where as __ is reserved for things defined and/or handled by Nix itself. There's definitely a gray area, but your __version is an instance of the prior.

infinisil · 2024-01-31T20:22:42Z

For instance, evaluating gcc.__versions.something requires that we evaluate gcc, as well as something, which has a real performance cost when applied at a scale that isn't tiny.

With lazy attribute sets this could be improved. I'd like to pick up that PR again, cases where it seems useful frequently appear.

roberth · 2024-01-31T22:11:16Z

I don't think we should invent language features to patch architectural flaws; the lack of a package concept that's not instantiated. I know it may have other uses, but this one basically doesn't count.

nixos-discourse · 2024-06-27T11:19:40Z