rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

[RFC 0110] Add "inherit-as-list" syntax construct to the Nix language

Open r-burns opened this issue 4 years ago • 136 comments

Rendered

r-burns avatar Oct 18 '21 01:10 r-burns

Definitely in favor of this. with is an anti-pattern that is rarely useful.

L-as avatar Oct 18 '21 10:10 L-as

I can imagine many cases where you care about the order, not to mention that the order of build inputs also affects the final derivation.

I think it's best to have the order be defined and duplicates allowed.

L-as avatar Oct 19 '21 22:10 L-as

I can imagine many cases where you care about the order

Can you list some? Maybe I'm just drawing a blank here but I feel like this is unusual (so should stick out from the typical case), and this is what most of my preference for the current form hinges on. E.g.

buildInputs = [
  inherit (pkgs) a; # needs to be first, because reasons!
  inherit (pkgs) b c d;
]

the order of build inputs also affects the final derivation

I actually like the attr sorting because of this, it means that you can reorder the attrnames in either type of inherit expression without changing the resulting value, so rebuilds don't trigger.

Lastly, the [ inherit a b c; ] syntax is useful if it preserves order + duplicates, but redundant if it just means [ a b c ]. So I like it for the purpose of consistency and orthogonality of the language. It "completes the square" so that

{ inherit (attrs) a b c; } # inherit from specified scope into attrset
[ inherit (attrs) a b c; ] # inherit from specified scope into list
{ inherit a b c; } # inherit from current scope into attrset
[ inherit a b c; ] # inherit from current scope into list

are all meaningful and useful in their own right.

r-burns avatar Oct 19 '21 23:10 r-burns

Thanks all for your feedback! I've updated the RFC to use the simplified order-preserving desugaring.

r-burns avatar Oct 20 '21 02:10 r-burns

Let's say the order is random. Each time you evaluate a derivation that uses the syntax, you would get a different derivation. Or perhaps it's not random, but changes every Nix release. You would still have the same problem. If it's defined and doesn't change depending on the version of Nix you're using, it has a defined order, even if not documented explicitly.

L-as avatar Oct 20 '21 09:10 L-as

The thing is, as soon as you derive e.g. a search-path from a list of let's say derivations, the order indeed matters. If e.g. $PATH contains several directories that have an executable with the same name, the executable from the first matching path is usually used.

Other than that I'm not very happy with reusing inherit here. If I read inherit I read "right, an attribute-set". While this doesn't seem ambiguous to parse, it certainly feels ambiguous to read, especially considering that we also have ; as separation-token apart from a space for several declarations inside a list.

Finally, while I agree that with is indeed problematic — for the reasons you correctly pointed out in your RFC — I think that this mainly is an issue with e.g. with lib; at the top of a file or something similar. It's perfectly clear what's meant when specifying with pkgs; [ foo bar baz ] and as long as pkgs is deducible, scope checking should be possible to implement.

All in all, I'm rather skeptical about this language change.

Ma27 avatar Oct 20 '21 10:10 Ma27

I can imagine many cases where you care about the order Can you list some? Maybe I'm just drawing a blank here but I feel like this is unusual (so should stick out from the typical case), and this is what most of my preference for the current form hinges on. E.g.

One place where order matters in an annoying way is building something statically and something dynamically in the same derivation (you need to get the order of glibc and glibc.static right…)

There are probably more cases like that, so I am skeptical about even a fixed order independent of the order in the invocation, let alone the annoyance of having randomly permuted search paths change the hash even if the difference is immaterial.

7c6f434c avatar Oct 20 '21 10:10 7c6f434c

Let's say the order is random. Each time you evaluate a derivation that uses the syntax, you would get a different derivation. Or perhaps it's not random, but changes every Nix release. You would still have the same problem. If it's defined and doesn't change depending on the version of Nix you're using, it has a defined order, even if not documented explicitly.

By "unspecified", I meant "not explicitly specified by the user", not "unspecified by the language". The attrValues desugaring would sort on attrnames so the as-list ordering would be stable and documented.

If I read inherit I read "right, an attribute-set". While this doesn't seem ambiguous to parse, it certainly feels ambiguous to read, especially considering that we also have ; as separation-token apart from a space for several declarations inside a list.

Agreed, I think the keyword somewhat makes sense when using the attrValues-based desugaring, but less so if the values are simply plucked from the list in order. The keyword reuse is really just to avoid having to add a new one to the language, and inherit is at least more intuitive than let, rec etc. If anyone can think of a way to use existing keywords/symbols/operators to denote these semantics in a more intuitive but still backward-compatible way, I'd be very open to it.

It's perfectly clear what's meant when specifying with pkgs; [ foo bar baz ] and as long as pkgs is deducible, scope checking should be possible to implement.

IMO this simple scenario is a perfect illustration. We all know exactly what is meant by this expression - and yet the semantics are not so simple:

  • if any of foo bar baz are defined in the current scope, they will be shadowed instead of picked from pkgs
  • if we are already inside a with context, static scope checking breaks and lookups are ambiguous

This is a lot of "spooky action at a distance" for such a simple intent. I think a replacement without these footguns is sorely needed.

r-burns avatar Oct 21 '21 03:10 r-burns

Bravo for this RFC. with is something that has annoyed me for a long time. With that said, I do share @Ma27's opinion about reusing inherit. I'm not quite as skeptical about having something like this though.

I just wonder if adding a new keyword to Nix would be all that bad really? Wouldn't it have more or less the same implication; i.e. it doesn't break existing code but new code using the new syntax wouldn't work with older versions of Nix?

If we allow a new keyword, we can maybe come up with something a bit more clear. With a new keyword from, for example, we could make something akin to a list comprehension: [ key1 key2 from attrs ]

If we wanted to add regular values after a from expression, we chould simply use a ; as a delimiter. So: [ key1 from attrs; val1 val2 ].

While this doesn't seem ambiguous to parse, it certainly feels ambiguous to read, especially considering that we also have ; as separation-token apart from a space for several declarations inside a list.

Could you please clarify this, perhaps I am misunderstanding but it seems like you are saying ; already has a special meaning inside a list, but I wasn't aware of this. If this is what you are saying could you give a simple example?

nrdxp avatar Oct 24 '21 19:10 nrdxp

The issue with a new keyword is IIRC that variables named like it will usually cause syntax errors (though I don't know the lexer well enough to say whether this is an inevitable consequence or just a side-effect). To demonstrate what I'm talking about:

λ ma27 [~] → nix repl
Welcome to Nix version 2.4pre20211006_53e4794. Type :? for help.

nix-repl> [ inherit ]
error: syntax error, unexpected INHERIT

       at «string»:1:3:

            1| [ inherit ]
             |   ^
            2|

nix-repl> let inherit = 1; in [ inherit ]
error: syntax error, unexpected '='

       at «string»:1:13:

            1| let inherit = 1; in [ inherit ]
             |             ^
            2|

But if someone who knows the lexer well-enough to falsify this assumption (e.g. @edolstra ?), I'd be totally in favor of a new keyword if such a change is supposed to be made.

Could you please clarify this, perhaps I am misunderstanding but it seems like you are saying ; already has a special meaning inside a list, but I wasn't aware of this. If this is what you are saying could you give a simple example perhaps?

Nope it doesn't have a special meaning. What I was trying to say is that I'm used to interpret a (well, alternatively a \n) as separator between two list items. Having another separator (i.e. ;) seems weird (and also makes Nix less accessible for newcomers IMHO). That said, I don't have a better suggestion in mind, sorry!

Ma27 avatar Oct 24 '21 19:10 Ma27

I guess worst case for a new keyword then would be that it requires manual intervention in the case of a colliding variable name, which doesn't sound horrible if we choose a keyword that is unlikely to be often used as a variable.

I see your point about having two delimiters inside a list. Maybe we could just remove the need for a ; by simply requiring that the list using this new syntax is standalone. I think most languages supporting list comprehensions work this way iirc. So, if somebody wanted to add regular values to the list afterward they'd just have to concat another list: [ key1 from attrs ] ++ [ val1 val2 ]

Even if we stick with inherit we could do the same.

nrdxp avatar Oct 24 '21 19:10 nrdxp

I guess worst case for a new keyword then would be that it requires manual intervention in the case of a colliding variable name, which doesn't sound horrible if we choose a keyword that is unlikely to be often used as a variable.

We'd essentially throw away the ability to instantiate any old Nix expression. Correct me if I'm wrong, but IIRC that's why e.g. the URL syntax (meta.homepage = https://example.org) is deprecated, not removed.

[ key1 from attrs ] ++ [ val1 val2 ]

If we do this, we should abolish [/] for such lists with inherits entirely though (not for "regular" lists though). Right now your example is quite ambiguous because it could mean "create a list with the variables key1/from/attrs" or "create a list with the variables attrs.key1".

Ma27 avatar Oct 24 '21 20:10 Ma27

Sorry if I've missed this as I only skimmed the comments, but is a normal function really that bad? I had a look in "alternatives", but I don't think that's exhaustive. The alternative that comes to my mind is:

[ attrs.x attrs.y attrs.z ]

becomes

select [ "x" "y" "z" ] attrs

It's a tiny bit more verbose than

[ inherit (attrs) x y z ]

For multiple inherits clauses, I would probably just build a list-of-lists and then concat:

lib.concat [
  (select [ "x" "y" ] foo)
  (select [ "u" "v" ] bar)
]

ocharles avatar Oct 27 '21 09:10 ocharles

@ocharles TBH that's probably the best solution. The issue is then getting people to do this, i.e. social.

L-as avatar Oct 27 '21 09:10 L-as

It's a tiny bit more verbose than

Well, it also highlights wrong, I guess (these strings are not data but literal variable names)

7c6f434c avatar Oct 27 '21 11:10 7c6f434c

The order of elements in the new inherit should certainly be well-defined since we are talking about lists, the data structure that gives order to a set of elements, and I only see two options:

  • use some arbitrary ordering (i.e. lexical)
  • use the ordering in the syntax graph

If the order was undefined/unstable, drv’s wouldn’t be deterministic anymore.

I want to argue against the first (arbitrary ordering), since not much is gained and people will start to depend on (that is: abuse) an implementation detail like this.


On a more general note, I’m :100: :+1: on this change, it’s simple enough, will make the language more symmetrical, and we can finally start deprecating with everywhere.

Profpatsch avatar Oct 27 '21 12:10 Profpatsch

Why is a language change is required to get people to stop using with, when you can do that right now by just using functions?

ocharles avatar Oct 27 '21 13:10 ocharles

@ocharles because with is slightly more ergonomic than this function style, and the proposed change is arguably same ergonomics as with in most cases but without semantical gotchas.

(And I am pretty sure inherit will have slightly better performance than with and the function will have slightly worse performance)

7c6f434c avatar Oct 27 '21 13:10 7c6f434c

Plus the nixpkgs stdlib isn’t always available, and since nix is a config language it should have the most important ergonomics for configs in its syntax imho.

Profpatsch avatar Oct 31 '21 20:10 Profpatsch

Not sure I like this. There currently is no seperator in lists, and this would add that.

I'd propose a list select instead, something like:

attrs.[a b c]

This is also currently a syntax error, and is a lot clearer and more consistent than what is proposed here.

Also, while we're changing syntax; I'd love a way to select "other" arguments to a function, something like { a, b, c, ...@args }: ...

Synthetica9 avatar Nov 03 '21 13:11 Synthetica9

I'm nominating myself to be a shepherd for this RFC

I am (currently) against this change, with the main reason being that I don't think this is the right solution for the given problem. The main motivation given is that we can get rid of the usage of with in combination with list creation. I'm arguing that the problem isn't with, but lists. Lists in especially NixOS modules are fundamentally flawed in that they depend on the ordering of modules, which is very unintuitive and can cause weird problems. I've hinted at this a few times before, that I'd like to propose an RFC to deprecate listOf for NixOS modules, replacing it with better alternatives without its drawbacks.

Let's look at the most common example of where RFC would be useful, lists of packages, e.g. environment.systemPackages. Currently this looks like

{
  environment.systemPackages = with pkgs; [
    foo
    bar
  ];
}

I'd like to propose to make systemPackages (and any similar values) to be an attribute set instead, like this:

{
  environment.systemPackages = {
    inherit (pkgs) foo bar;
  };
}

This not only gets rid of with, but it can furthermore fix https://github.com/NixOS/nixpkgs/issues/32405, allowing packages to be removed from systemPackages with

{
  environment.systemPackages.bar = mkForce null;
}

Doing this has the same benefit of getting rid of the main use of with, but it does so without any new syntax and also makes evaluation more deterministic (as module order, aka list order doesn't matter anymore), and it allows adding more features (giving names to values with an attribute set allows you to refer to them afterwards, to e.g. remove them). Oh and also this allows the module system to find conflicting packages.

infinisil avatar Nov 03 '21 17:11 infinisil

Lists in especially NixOS modules are fundamentally flawed in that they depend on the ordering of modules, which is very unintuitive and can cause weird problems.

Which is why the example in discussion is buildInputs

7c6f434c avatar Nov 03 '21 17:11 7c6f434c

Lists in especially NixOS modules are fundamentally flawed in that they depend on the ordering of modules, which is very unintuitive and can cause weird problems.

Which is why the example in discussion is buildInputs

I don't see how that's different? sway.overrideAttrs (old: { buildInputs.wlroots = ... }) looks pretty elegant to me tbh

Synthetica9 avatar Nov 03 '21 18:11 Synthetica9

Lists in especially NixOS modules are fundamentally flawed in that they depend on the ordering of modules, which is very unintuitive and can cause weird problems.

Which is why the example in discussion is buildInputs

I don't see how that's different? sway.overrideAttrs (old: { buildInputs.wlroots = ... }) looks pretty elegant to me tbh

The order of buildInputs is significant so unordered sets cannot be used.

jtojnar avatar Nov 03 '21 18:11 jtojnar

@Synthetica9 we normally do not apply overrides to packages in an unspecified order, and there are packages where correct order of buildInputs matters. Moreover, due to the same reason, you usually can just add overrides without removing the old entries…

Also, that would add evaluation overhead — sure, for modules we do not care as module system is even heavier, but for packages it's a bit different.

7c6f434c avatar Nov 03 '21 18:11 7c6f434c

Hmm I wasn't aware how significant the order was for buildInputs...

Synthetica9 avatar Nov 03 '21 18:11 Synthetica9

@Synthetica9:

Not sure I like this. There currently is no seperator in lists, and this would add that.

I'd propose a list select instead, something like:

attrs.[a b c]

Nice! I think I was tunnel visioning on reusing a keyword; a new symbolic syntax is another valid approach. I agree that this is much cleaner and more consistent:

  • The attrs. can be said to "distribute" over the square braces
  • It is immediately clear that the result is a list, and order is significant

Let me know what you think, all. I'd be happy to amend the RFC to use this syntax instead.

r-burns avatar Nov 03 '21 18:11 r-burns

@Infinisil, very cool idea, thank you!

I think your proposal is a great improvement when constructing combining attrs into a unified environment such that order truly does not matter. When order does matter, such as when constructing a search path, I think those who argued against my original attrValues { inherit } desugaring would oppose using an attrset, with the same rationale.

Anyway, I don't see your proposal as a competitor to this RFC - I can imagine a universe where both your proposal and this RFC are accepted. I think they would both be beneficial, in non-overlapping scenarios.

r-burns avatar Nov 03 '21 18:11 r-burns

@Synthetica9 To be fair, in most of the simple cases it should not matter because each input package is an independent concern etc., so it is fine not to be aware of the relatively rare but quite significant corner cases.

7c6f434c avatar Nov 03 '21 18:11 7c6f434c

While my proposal isn't in conflict with this RFC, it does solve the same problem and would therefore obviate the need for this RFC (which is fairly intrusive due to changes to Nix itself)

infinisil avatar Nov 03 '21 18:11 infinisil