[RFC 0126] Standardize special properties

[schuelermine]

Rendered

[nixos-discourse]

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

https://discourse.nixos.org/t/rfc-standardize-magic-properties/19344/1

[fricklerhandwerk]

Suggested change

	Properties treated specially by the Nix tools and Nix language when dealing with arbitrary are renamed to start with a double underscore. Instances where this is not necessary are `__functor` and `__toString`. `recurseForDerivations` is renamed `__recurseForDerivations` and `type` is renamed `__type`.
	Properties treated specially by the Nix tools and Nix language when dealing with are arbitrarily renamed to start with a double underscore. Instances where this is not necessary are `__functor` and `__toString`. `recurseForDerivations` is renamed `__recurseForDerivations` and `type` is renamed `__type`.

[fricklerhandwerk]

I have a hard time understanding the exact problem and its severity.

Could you please make an explicit list of the magic attributes you are aware of, explain what they are for, why they are magic, and link to relevant documentation and source code?

[schuelermine]

The problem is that there’s a bunch of specially treated attribute names that mean attrsets aren’t neutral data structures.
Maybe magic is a bad word.
As I say further down, I don’t know a full list, since these seem to be things that aren’t considered together in the documentation.

type is magic because if type = "derivation", the data is displayed completely differently in the Nix repl.
By renaming them to start with __, I hope to make them more obvious, more segregated from normal attributes, and easier to deal with.

A problem this could fix could arise when a user who does not know how a process works inputs data into it, which is unknowingly processed to create an attribute that unexpectedly alters the result’s behaviour.
None exist ATM AFAIK, but they could.

[vcunat]

Attrsets by themselves are (almost?) neutral, I believe, but some functions consuming them treat some attributes specially.

[schuelermine]

With the exception of __functor. And precisely these, in my opinion, should be dunder(__)-prefixed

[pennae]

type is magic because if type = "derivation", the data is displayed completely differently in the Nix repl.
By renaming them to start with __, I hope to make them more obvious, more segregated from normal attributes, and easier to deal with.

it's even more magic than that. changing type is a flag day event, and for that reason alone probably not worth it. also note that recurseForDerivations isn't (afawk( honored by the new nix commands at all either.

[schuelermine]

That’s a good point. Looking at the code though it seems not so hard to change. Don’t the new commands try to build non-derivation store path results anyways? But yeah, this would be a big change, probably not worth it. Also, AFAIK nix-command honors recurseForDerivations in legacyPackages only.

[Ma27]

I don't think it's such a good idea to make it actually breaking: IIRC one of the benefits Nix advertises is that you can instantiate old Nix expressions and still get the same result (but that also means that the behavior of the expression shouldn't change).

[schuelermine]

I see, but then this reduces to just guardMagic.

[schuelermine]

Also, this doesn’t stop old expressions from building, since __toString and __functor are the only ones here that aren’t user-facing, and they’re not renamed.

[Ma27]

When grepping for recurseForDerivations in nixpkgs, there's a bunch of places where it's explicitly used, so what do you mean with "user-facing"?

[schuelermine]

Their special effect is outside the expression language. AFAIK recurseForDerivations tells nix commands to search for derivations deeper down, but it’s only being set in Nixpkgs, not used by it. Renaming it without renaming it in nixpkgs or using an old nix expression will not affect directly building any particular derivation, but only auxiliary things like searching. It’s unlikely that many nix expression directly check recurseForDerivations.
The other ones, on the other hand, if changed, would actually stop some Nix expressions from evaluating. __functor concerns the in-expression behaviour of these values.

[7c6f434c]

Technically speaking, adding synonyms that start with __ for all the magic attributes and deciding that Nixpkgs will use the underscore-based things from now on is more than guardMagic.

Evaluation-affecting magic seems to be indeed underscored; nix command behaviour is nominally still experimental so some changes are possible with proper warning.

[Ma27]

Evaluation-affecting magic seems to be indeed underscored; nix command behaviour is nominally still experimental so some changes are possible with proper warning.

But doesn't that affect nix-* commands usually as well?

[schuelermine]

It does. In my opinion that’s not as much a breaking change as a change in how some derivations build.

[schuelermine]

I have updated this PR to say "special" instead of "magic", I think that’s better.

[bew]

I have updated this PR to say "special" instead of "magic", I think that’s better.

Please also change the 'Rendered' link in first post (:

[edolstra]

This RFC is now open for shepherd nominations!

[roberth]

recurseForDerivations is everywhere, not just in Nixpkgs.
Do we have any evidence of this causing a problem in the real world?
I suggest adapting the definition of builtins.guardSpecial instead.

Same goes for type.
Note that we also have _type, which is the type tag used by module system related constructors. Again, changing this is more trouble than it's worth. Just add _type to guardSpecial's deny list.

[schuelermine]

Wouldn’t that have to be a completely different RFC?

Note that I’m open to closing this for good, it’s a much bigger change than I had anticipated when I wrote it up.

[pennae]

to expand on the "any evidence of this causing a problem" bit: changing attribute names would be incredibly hard in the very dynamic language nix is. we can find all instances of e.g. drvPath in code, but which of these are used to construct/modify derivations and which aren't? meta is to some extent a special name, changing that would touch almost the entire nixpkgs tree (and introduce mismatches between derivations and modules). changing old names feels like asking for a disaster, and silently dropping assignments can also easily turn into a footgun. 😕

which is not to say that guarding against accidental use of special names is bad! such a guard should (usually) fail very loudly though to catch mistakes early, behaving much like assert ! elem name [ «specials» ] && match "__.*" name == null; name. prefixing new special names also sounds very reasonable.

[edolstra]

@fricklerhandwerk @roberth @Ma27 @pennae Are any of you interested in shepherding this RFC?

[fricklerhandwerk]

@edolstra No. I‘m not qualified, both in terms of nixpkgs and RFC expertise, and also don’t want to dilute my focus on documentation work.

[schuelermine]

A note: I am no longer necessarily in favor of this RFC, even though I authored it. . The comments have convinced me this is too big an undertaking. Maybe it should be closed? I’m sorry for cluttering up the RFC process in that case.

[schuelermine]

GitHub appears to have changed the behaviour (on Firefox, at least) of Ctrl+Enter to closing instead of commenting… Sorry about that.

[pennae]

@edolstra no, don't feel up to the task (and don't think the scope outlined in the RFC is good at all, if anything it should be cut down to "new attributes with special interpretation by nix should have a __ prefix", which is common practice already)

[roberth]

@edolstra The original idea behind this RFC doesn't work out in cost/benefit. I wouldn't mind doing a bit more guidance, but I don't think we need the full RFC process to carry this to a satisfactory conclusion.

@schuelermine Functions like you describe could be added to the Nixpkgs lib without controversy, when they don't impose your original breaking changes.
In my opinion, for this outcome, it's better not to go through RFCs, because such an RFC would only be useful for describing the status quo, and it may go out of date when the implementation of the functions is improved. (For documentation, we have the manual)

[lheckemann]

@schuelermine if you feel that there's anything useful this RFC could be rewritten to do instead, feel free; otherwise you're also welcome to withdraw it (just close it) :)