Documentation: guide for using `let in` vs `rec` vs `finalAttrs`

[isabelroses]

Problem

I often see maintainers and committers alike suggesting to use the let in syntax following the practices suggested by https://nix.dev/guides/best-practices.html. However, this is hardly uniform some would suggest using rec, and it's hard to get a solid grasp on what to use in each situation. This was only exemplified when finalAttrs started popping up more commonly as of recent.

Proposal

From my understanding https://nix.dev is an official source so perhaps it would be best to get everyone on board and aware of this best practice? Or maybe to get a more concrete set of best practices to use in each given situation. Or at the very least make members aware that the let in syntax is said to be the best practice, as stated in https://nix.dev/guides/best-practices.html.

Checklist

• checked latest Nixpkgs manual (source) and latest NixOS manual (source)
• checked open documentation issues for possible duplicates
• checked open documentation pull requests for possible solutions

---

Add a 👍 reaction to issues you find important.

[Atemu]

At the very least I'd like reviewers to stop nitpicking between these.

Unless there is explicit community-wide consensus (i.e. RFC), wide-spread historical precedent (i.e. mkDerivation attr order) or significant concrete problems with the code (i.e. correctness, readability), people should be free to use whichever code patterns or organisation they like.

And by significant, I really do mean significant. Code being organised against your personal preference or patterns you dislike being used are minor differences in taste, not a significant readability problems.

Nitpicking such "issues" is not in any way productive and utterly confusing/unwelcoming to newcomers. They are faced with dozens such implicit rules that aren't explicitly written down anywhere and, even worse, differ wildly depending on which person reviews the code. I don't doubt we've lost more than a dozen contributors whose first PR was faced with 12 nitpicks and 0 actually substantial review comments and then left to bitrot for months.

I've been guilty of this aswell and we all just need to stop doing it.

[pluiedev]

I was (and still am, I guess) one of those nitpickers and I think what really needs to happen here is to standardize the finalAttrs pattern so that it's not only applicable to mkDerivations. Nowadays I don't even nitpick recs appearing in other builders and change them to let-ins, but when I didn't change that in one of my own PRs I got nitpicked for that 😅

It almost feels pedantic, in a way, with the rule being more of a dogma, and the rationale behind it not being at all clear to new contributors and the applicability of it still very limited. We should really try to explain things more, come to a common standard, and make that standard applicable to everything.

Perhaps a table could serve as a brief explanation as to why one pattern may be preferred over the other?

Pattern	Overridability	Readability	Applicability
rec	❌ Does not automatically respond to overrides	❌ Recursive variables could be hard to spot and locate in a large file	✔️ All builders
let in	❌ Does not automatically respond to overrides	✔️ Variables are clearly defined in the let portion, and used in the in portion	✔️ All builders
finalAttrs	✔️ Automatically responds to overrides	✔️ Recursive variables are explicitly prefixed with finalAttrs or inherited from finalAttrs	⚠️ Only mkDerivation and a few other builders (e.g. buildRustPackage, buildGoModule, etc.)

Also, the existence of finalAttrs is buried in a tiny chapter in the huge Nixpkgs manual, which nobody would even know of were it not for the recent surge of its use. I think there should also be more coverage of this pattern elsewhere in the scattered Nix docs, so that new contributors could be made aware of it before they start contributing. The explanation of the differences between rec and finalAttrs is also terribly unclear and not backed up with concrete examples, like so:

let 
  withRec = stdenv.mkDerivation rec {
  	pname = "with-rec";
  	version = "0.1.0";
  	
  	src = fetchFromGitHub {
  	  owner = "example";
  	  repo = "example";
  	  rev = "v${version}";
  	  hash = "...";
  	};
  };
  withFinalAttrs = stdenv.mkDerivation (finalAttrs: {
  	pname = "with-rec";
  	version = "0.1.0";
  	
  	src = fetchFromGitHub {
  	  owner = "example";
  	  repo = "example";
  	  rev = "v${finalAttrs.version}";
  	  hash = "...";
  	};
  });
in
lib.trace (withRec.overrideAttrs { version = "0.2.0"; }).src.rev # v0.1.0
lib.trace (withFinalAttrs.overrideAttrs { version = "0.2.0"; }).src.rev # v0.2.0
{};
`` 

[getchoo]

i think #310373 is important to bring up in this conversation.

at first glance, finalAttrs does seem like a "silver bullet" to overrides propagating across the expression. however, continuing from the example above:

lib.trace (withRec.overrideAttrs { version = "0.2.0"; }).src.rev # v0.1.0
lib.trace (withFinalAttrs.overrideAttrs { version = "0.2.0"; }).src.rev # v0.2.0

while the rev of src is overridden through finalAttrs, hash isn't. as explained in the issue, this and other uses of finalAttrs can easily introduce a lot of footguns to users when overriding; and even in the best case, the use of finalAttrs over rec here doesn't really improve anything for the end user

At the very least I'd like reviewers to stop nitpicking between these.

this is probably what i want most out of this issue, though. a lot of times the differences between these patterns are exactly that: nitpicks. i'm sure it's frustrated many here time and time again - and if it doesn't actually benefit our user in many cases, why are we doing it? why are we giving new contributors a hard time over preferred patterns to reach the same goal?

i would personally encourage package maintainers to use their personal preference unless it's actually affecting how the expression is used or defined. for other readability/consistency concerns, we will hopefully have nixfmt soon tree-wide soon 😄

[pluiedev]

The problem with version specifically, I think, is that it's either easy to forget to update the hash if the rev depends on finalAttrs.version, or it's easy to forget to update the source altogether. Might be better to have a built-in lint/warning/assert against that in mkDerivation

[isabelroses]

It may also be worth reading over #293452 and #119942, for more insight.

[Atemu]

@getchoo made the same points I was going to make.

To that I want to add that whenever this comes up, one should think about which parts of the package they want to expose as an external API and what parts you don't.

finalAttrs responds specifically to overrideAttrs, not overrides in general. Whether you want to use the pattern depends on whether you want the reference attributes to be be generically overrideable; part of the package's API. It does not always make sense to do this however. As @getchoo pointed out, exposing the version as part of the API using finalAttrs like this has issues.

As a more robust alternative, I've recently discovered that you can simply make such parameters part of the regular override interface. I think this is even better as it makes them explicit part of the package's external API and allows other parts of the external API to respond to version changes (i.e. withFoo ? lib.versionAtLeast version "6.1"). I did this in the ffmpeg package and I think it's better for it: https://github.com/NixOS/nixpkgs/pull/295691/files#diff-547e4d5699df1c405afdc0c2837df95959ed6de64f384ca5720ee37367eebcce
This design allows you to take any ffmpeg variant (i.e. ffmpeg_7-full) and transform it into any other (i.e. ffmpeg_4-headless) using only override, without any overrideAttrs.

Regardless of opinions on these manners though, a table like @pluiedev's would be great to have in the contributor's manual. We generally need more such short summaries and general guidelines to be written down in accessible places.

I also want to mention that, in my book, it is perfectly valid to use rec, let ... in and finalAttrs in combination as they each have distinct purposes.