-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Overload /
to mean path append; alternative to path + "/" + "foo"
footgun
#7301
Comments
/
to mean path component append; alternative to path + "/" + "foo"
footgun/
to mean path append; alternative to path + "/" + "foo"
footgun
This is a bit problematic because |
I wonder if something like |
I should also point out the recently merged #5066, which actually supports almost this:
I feel like support for |
We could at least do this in C++ actually. @edolstra wdyt? |
Discussed in meeting today
|
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-02-05-nix-team-meeting-minutes-121/39340/1 |
See also the newish |
There could be a system to this:
I'll leave it at this, because Nixpkgs library design should probably be discussed over at nixpkgs instead. Thanks for linking this @infinisil |
What are the benefits of introducing an overload for |
I'm not opposed to extending path interpolation, but I don't like that we can't tell beginners that
I think you meant
This violates the property that a path literal produces a path value.
I don't think this is all that important. It's highly unlikely that someone would quote their numbers, so this is supposed to be representative of an expression It is fair to say that by allowing |
@roberth Thanks for the clarification; I was trying to interpret your earlier meeting notes. It was not clear to me that a path append expression's result's type depends on its LHS's type. To clarify, is this an accurate typing? {-# LANGUAGE FunctionalDependencies #-}
class Div lhs rhs result | lhs -> result
(/) :: lhs -> rhs -> result
instance Num a => Div a a a
instance Div String String String
-- explicitly not an instance:
-- instance Div String Path String
instance Div Path String Path
instance Div Path Path Path The above presumes
I agree that this isn't important; I brought it up because your meeting notes did.
These could be made to work if relative path values were introduced as a subset of path values, to complement the current absolute path values, but that'd break cases like
Agreed here. I think a better Nix would have a built-in URL type, handle path literals like You can also see effective relative path values showing up again in lazy trees, where you have absolute virtual path values that are effectively relative paths paired with an absolute virtual path root. Certain operations can coerce those absolute virtual paths into absolute store paths. Pure flakes in current Nix already work with non-store path values, where non-store paths can be used & manipulated as long as they stay in the Nix level (for the most part). Accidentally coercing one of these paths into a store path is often easier to miss or ignore because normally it only results in copying a file/tree from the flake you're working in, not potentially a full Nixpkgs source tree. I think Nix needs to teach a consistent, understandable set of rules for both when paths are coerced to be store-relative and what file/tree is the store path root (i.e. the relevant child of If we can guarantee that path appends via I believe that path appends via path interpolations would also never directly coerce paths to store paths. A reason I brought up extending path interpolation, besides avoiding confusion with numeric division, is that currently expressions like Alternatively, lazy-trees changes its design to make coercing to store paths a much more intentional operation, so Nix programmers don't have to worry as much about strings containing paths. See also #7335, which has this quote:
To add on to that, I think that as long as NixOS continues to provide modules that can't have their functionality fully disabled as Nix source files, like the majority of Having written all that, I'd like path interpolation to be semantically equivalent to using This proposal means path literals starting with path interpolation may not result in path values, but I think that's reasonable for the improved consistency. Path operations will be more likely to "do the right thing", even in the face of lazy trees. |
Not sure about this one:
The rhs path is only meaningful if it's an absolute path (ie not a virtual path into e.g. a git repo), and even then, constructing absolute paths to things that won't exist is not a great habit, perhaps subjectively.
Sounds about right, and my attempts with path values and derivation outputs (for good measure) failed. 👍 My point was that they're easily confused with string interpolations though, which do copy.
I think Currently the error is only reported after adding to the store, which is not optimal, but sufficient for you not to have doubts about existing code you're reading. A lazy path strings solution (lazy-trees or better) could postpone the copying operation until after the error is detected.
I was surprised to read this, because the context wasn't presented clearly. This is about implementing operations on paths and particularly path strings within the Nix language. IIRC Eelco's point was that Nix is a DSL and we shouldn't need to implement libraries for such basic functionality in it.
This is an unsolved problem. Lazy trees will leak non-deterministic virtual path names in this case, which in my view is the primary reason why we can't just adopt that solution.
I think I agree with your conclusions, but I'm unsure about the value to users of |
Agreed; this case shouldn't be legal.
If string interpolations & string concatenations never copy, by making them eagerly fail when a non-store path or non-store string context is used, the confusion disappears.
My bad there; thank you for clarifying. I strongly agree that your regular set of path operations should be sane at the Nix level, without Nix-coded libraries.
I feel like this is a footgun that'll cause problems the moment one of those paths leaves a module system evaluation and starts being manipulated by some consuming flake that isn't using flake-parts.
We're in the non-deterministic (or effectively random) boat already with allowing the sorting of lists with lazy path values inside them.
Given that we already allow strings (with context) in lists that you can sort, that's probably the most consistent.
If I have a forced I want a function that solely converts an existing path into an actual store-relative path, because I want people to use it, especially if we emit deprecation warnings constantly when people don't use either It's a similar argument to allowing |
Is your feature request related to a problem? Please describe.
In the Nix language,
+
associates in such a way thatpath + "/" + "foo"
evaluates topath + "foo"
for any path valuepath
.#6530 pushes people towards this syntax, so perhaps it's worth considering to give them a concise alternative that doesn't lead to almost inexplicable malformed paths every now and then.
Describe the solution you'd like
Instead of
evaluate to
Describe alternatives you've considered
Add builtin fetchers to the string context, and add fixed-size holes to strings, so that "${fetchTree foo}/bar" does not immediately fetch the tree.
Holes are a lot to ask though.
Perhaps better phrased as a "rope" of strings and lazy tree thunks. A rope, or lazily concatenated string representation might not be a bad idea?
Additional context
The text was updated successfully, but these errors were encountered: