Skip to content
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.

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

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

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

r-burns
Copy link

@r-burns r-burns commented Oct 18, 2021

r-burns added a commit to r-burns/nix that referenced this pull request Oct 18, 2021
Initial MVP implementation for [RFC 110](NixOS/rfcs#110)
r-burns added a commit to r-burns/nix that referenced this pull request Oct 18, 2021
Initial MVP implementation for [RFC 110](NixOS/rfcs#110)
@L-as
Copy link
Member

L-as commented Oct 18, 2021

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

rfcs/0110-inherit-as-list.md Outdated Show resolved Hide resolved
rfcs/0110-inherit-as-list.md Outdated Show resolved Hide resolved
@L-as
Copy link
Member

L-as commented Oct 19, 2021 via email

@r-burns
Copy link
Author

r-burns commented Oct 19, 2021

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.

Before:
```
[ inherit (attrs) a b c; ] := builtins.attrValues { inherit (attrs) a b c; }
```

After:
```
[ inherit (attrs) a b c; ] := [ attrs.a attrs.b attrs.c ];
```

The previous desugaring has some potentially nice properties such as
non-significant ordering and no-duplicate enforcement, but was
ultimately deemed unintuitive and too surprising in practical use.
@r-burns
Copy link
Author

r-burns commented Oct 20, 2021

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

@L-as
Copy link
Member

L-as commented Oct 20, 2021

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.

@Ma27
Copy link
Member

Ma27 commented Oct 20, 2021

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.

@7c6f434c
Copy link
Member

7c6f434c commented Oct 20, 2021 via email

@r-burns
Copy link
Author

r-burns commented Oct 21, 2021

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.

@nrdxp
Copy link

nrdxp commented Oct 24, 2021

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?

@Ma27
Copy link
Member

Ma27 commented Oct 24, 2021

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!

@nrdxp
Copy link

nrdxp commented Oct 24, 2021

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.

@Ma27
Copy link
Member

Ma27 commented Oct 24, 2021

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".

@ocharles
Copy link
Contributor

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)
]

@L-as
Copy link
Member

L-as commented Oct 27, 2021

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

@7c6f434c
Copy link
Member

It's a tiny bit more verbose than

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

@Profpatsch
Copy link
Member

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 💯 👍 on this change, it’s simple enough, will make the language more symmetrical, and we can finally start deprecating with everywhere.

@ocharles
Copy link
Contributor

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?

@7c6f434c
Copy link
Member

7c6f434c commented Oct 27, 2021

@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)

Linked content has moved to recipes/best-practices
@fricklerhandwerk
Copy link
Contributor

@r-burns We discussed the RFC today in the Nix maintainers meeting.

The proposal seems to optimise for a particular use of with. We're more interested in exploring a construct that would alleviate the problems associated with with in general, such as the scoped proposal or a construct that has more intuitive scoping rules.

In any case, we'd err towards not changing the language if in doubt unless there are extraordinarily compelling reasons, both to keep complexity and the learning curve in check.

Complete discussion
  • @edolstra: main problem with the syntax is that one can only inherit from one attrset, which is different from inherit
  • @fricklerhandwerk: the proposed semantics is quite limited, and the value added seems little for the effort, and also risk to stick to yet another an unusual syntax
    • all of what is proposed can already be done by using existing constructs carefully enough
    • @edolstra: yes, no other language seems to have that, except maybe Perl 6
      • @Ericson2314: not necessarily the endorsement you would like to have
  • @Ericson2314: liked the exclusive with, but seems not to be taken too seriously as a proposal
    • @thufschmitt: agreed, clearing the scope seems more flexible and still improves on with
  • @thufschmitt: no opinion on the importance of the motivating use case, not that active of a Nix language author
    • @Ericson2314: the given examples of uses for with are actually fine
  • @edolstra: more general question, also considering RFC 148, is how we determine is something is "worth it"
    • maybe we can come up with some criteria?
      • one criterion would be if something makes the code a lot shorter, for something that is a common pattern, i.e. used a lot
      • Be a sufficiently common pattern
      • Make code more likely to be correct, or reduce opportunity for suprising behavior
      • How intuitive is the new syntax
      • These have to be weighed against the increased complexity / learning curve of the language
    • @Ericson2314: hard to imagine example syntactic sugar that may be worth it
    • @fricklerhandwerk: if anything, we probably don't want to make the language larger, or only very, very carefully
      • and if we add constructs, should consider the expressive power gained
    • @edolstra: with is contentious, but this proposal does not really improve on the situation
      • @fricklerhandwerk: if the proposal put forward adding exclusive with, would we be interested in exploring that further?
        • @thufschmitt: wouldn't be opposed
        • @roberth: on the fence, against adding more scoping rules, but it would allow us to de facto abandon with which I'm in favor of
          • precedence: let { body }
          • @thufschmitt: grepping for with shows a lot more use cases than just expanding a list
        • @Ericson2314: not excited, we'd just be making the language bigger. we should not consider langauge changes until we figure out language versioning and a path to deprecating or removing things
        • @fricklerhandwerk: agree with @roberth, would be worthwhile looking into what an exclusive with would entail, but should make sure it's a strict improvement for all uses of with in the terms we discussed above
        • @edolstra: like the idea
  • @fricklerhandwerk: "inverted" with that has more intuitive precedence is an alternative
  • @roberth: not adding anything is also an alternative

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-06-26-nix-team-meeting-minutes-66/29650/1

@bobvanderlinden
Copy link
Member

I'm not sure how to continue with this as I don't really see how to come up with more pro/cons for this idea. Does someone have a good path to take for this RFC? Should this RFC be closed?

@fricklerhandwerk
Copy link
Contributor

We may want to get input from @sternenseemann, @tazjin or @amjoseph-nixpkgs on whether this specific approach is viable. I can imagine it can be continued to elaborate on the exclusive with idea, which is clearly more work but seems to have a realistic chance of getting broad support.

You can continue piling up considerations and that may change the conclusion, nothing seems wrong with that. If you feel like taking a new direction mandates a different proposal, that should be fine as well.

In any case, for any such substantial change to the language to get buy-in, we'd need agreement by implementers (currently Nix and Tvix) and large-scale users (Nixpkgs maintainers), who will have most experience with what to look out for.

@oxalica
Copy link

oxalica commented Jul 28, 2023

Visual Basic forces .attr inside With blocks, so it's easy to know which is from With and which is a variable. And inner With fully shadows outer With.

https://learn.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/with-end-with-statement#example-1

But unfortunately this is not possible for Nix, because [ a .b ] will be interpreted as [ a.b ]. But we can, similarly, introduce a new syntax to access attributes of with, differentiating it from normal variable names.

newwith pkgs; [ variable &.attr &.attr2 ]

Here &. is a demo and can be bikeshedded later. This approach has advantages of:

  • Allow both variables and attributes to be referenced inside, unlike current with or exclusive-with.
  • Easy for human to distinguish between variables and attributes.
  • Fully static, no matter what pkgs contains and if any new attributes are added later.
  • Not specific to lists, one can also write { lib, pkgs }: newwith pkgs; &.mkShell { packages = lib.optional FOO [ &.gcc ]; } and everything is easy to read also.

@infinisil
Copy link
Member

infinisil commented Jul 28, 2023

There is a long discussion (at least 1h30) about with, this PR and the alternatives on Matrix (@oxalica also talked about the above comment there).

I originally proposed a let with syntax earlier in the comments, which I believe could fully deprecate the old with while offering similar semantics with different trade-offs.

But in the end the only thing I care about is that we can fully deprecate with. Because if we accept a proposal to only allow partial deprecation, we're gonna need another proposal to fully deprecate with later.

So I think this RFC needs to include that with will be fully deprecated if accepted, including justification for why this proposal is good enough to do that, how deprecation will happen (maybe #137 should be a prerequisite), and what the impact is.

@kevincox
Copy link
Contributor

kevincox commented Aug 1, 2023

I slightly disagree.

I do agree that we do need a plan for how to deprecate with in order to fully understand and accept this RFC. But I don't think that this RFC needs to completely deprecate it. It is entirely possible that we have multiple tools or features that work together to cover the ground that with previously covered.

That being said I don't think that it is good to add these feature piecemeal. I think we need to understand the whole replacement before we start moving on any part of it to ensure consistency while avoiding gaps and overlaps from different features.

@kevincox
Copy link
Contributor

kevincox commented Aug 9, 2023

After discussing with @r-burns we are going to mark this RFC as a draft. It is clear that before moving forward with this proposal we need more understanding and design about a post-with Nix language. At the time Ryan is not able to take on this larger task so we will put this to rest for now. Ryan and I think that this feature still has merit and may be a key part of replacing with but will not be pushing this design further at the current time.

Others are of course welcome to pick up this idea and run with it if they have the resources to fit together the full story of with deprecation.

@infinisil
Copy link
Member

If somebody has the time and energy, I think like something like #137 would be a more impactful change to work towards, which will also benefit any potential with deprecation

@7c6f434c
Copy link
Member

7c6f434c commented Aug 9, 2023

#137 is not really that relevant for reduction of with use in Nixpkgs…

@infinisil
Copy link
Member

@7c6f434c This RFC here is not specific to Nixpkgs, it proposes to introduce a new Nix language feature with the intention of getting closer to the deprecation of with. #137 would be directly beneficial to this RFC because is answers the question of how to deprecate language features at all. And I think #137 is more worthwhile to focus on because it's a more fundamental problem with a greater impact to not just with deprecation, but also all the other problematic bits in the language.

@7c6f434c
Copy link
Member

7c6f434c commented Aug 9, 2023

If with is restricted (if it is constrained to a few well-demarcated magic places, we get most of the benefits) or fully removed in Nixpkgs it will be done on a much shorter timescale than even close to reasonable for a full blown language feature deprecation in Nix.

And the question for this is not about getting Nix to deprecate it globally, but to have enough replacements for frequent Nixpkgs uses and confine the rest.

#137 is not relevant to the parts that actually matter for the motivation here — providing alternatives to with for the cases where it's just not worth it.

@kevincox kevincox marked this pull request as draft August 23, 2023 13:26
* Expressions reliant on the new syntax will be incompatible with
Nix versions prior to the introduction of this feature.

# Alternatives
Copy link
Member

Choose a reason for hiding this comment

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

Was a new keyword suggested?

@philiptaron
Copy link

I'm chiming in to say that I really wish this existed. attrset.[a b c d] is fine syntax for it.

@L-as
Copy link
Member

L-as commented Mar 9, 2024

If someone implemented this the RFC would probably have a better chance of being accepted. It shouldn't be too hard.

Or maybe someone already has implemented it?

@tomberek
Copy link
Contributor

@L-as Please note that the parser is currently being re-written by pennae, but I'm not sure how hard it would be to add this.

@7c6f434c
Copy link
Member

Also, there were some comments from Nix team about not wanting to add new syntax before any kind of policy to removing it has been finalised…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.