formats.kdl: init (usage ergonomics)#426828
Conversation
5c7409d to
8da1c40
Compare
|
nice work! these are angles i definitely had not taken into account at #412029. it maybe does feel unfortunate aspects like extent of laziness aren't as easy to test. on others having worked on kdl from nix and might be able to provide feedback, i can mostly find home-manager's |
Basically, the only "laziness" that matters here is that the top-level document list on an option should not be strict in any other values. That is, it should not depend on any conditionals or such. This is important, because no matter what we do, the module system is always strict in option definitions (because they are always checked for All other parts of the document don't really need to be "lazy"; we're passing it directly into |
|
then i guess this one should be "closes #198655" |
|
Pardon, it might be outside of scope but is |
That just seems difficult on so many levels. I'm sure it could be done, but imo it's outside the scope of this PR, multiple home-manager programs need this pulled (zellij and niri off the top of my head). |
As in, using the KDL-in-Nix DSL defined here (with "documents", "splats", "nodes", etc) and returning a JSON-shaped configuration? Something like this is potentially worth doing in the future if we want to support arbitrary KDL queries (with KQL) or validation of "KDL schema"; but it's certainly not in scope for this PR. I also don't quite get why you'd want to do this. In the context of Nix configuration in If you mean, like JSON-in-KDL(-in-Nix), then yes you can totally do that with this PR. just plug the output of |
I personally don't agree with this. |
As i've explained in detail, KDL is different in a way that matters a lot for Nix, and completely shatters what is expected by
I also wish to, at some point, support something that allows you to define how to merge KDL documents. I think of this as "extensible KDL": a more flexible, fully-fledged, fits-in-a-module-no-questions-asked, alternative to It's also common to make (and, on a more personal note, this is
|
|
i would argue (convergence in) the |
|
Hello, apologies for not having checked this earlier! I'm currently jet lagged and haven't had the capacity to review this fully. However, I feel it's really important that we get the namespacing right, for a few reasons:
This is why I've been stressing that
But the entire point of
I understand where you're coming from on this, but I believe it's absolutely in scope. It's difficult to get anything standardized under nixpkgs, mainly because we have to get so many people to agree with it. RFC42 is one of the few standards we have that keeps modules somewhat consistent. Diverging from this format would break a well-established design pattern, and be a big problem for subsequent PRs, as the namespace would no longer have any meaning. |
Like @nezia1 said, that's definitely in scope. The reason for splitting off the PR is (a) to discuss the functionality changes (restrictions in usage + "splat" ergonomics), as well as (b) introducing better documentation for the format. I included an example module with my preference of option naming, and mentioned in the PR why i think we need to diverge from |
|
It seems at least i and @nezia1 are in agreement that this probably needs a different namespace. But uh. Which namespace? I genuinely don't know. |
As I said, I'd be completely fine with something like Or perhaps we could go with |
toKDL? |
¿Por qué no los dos? (would genuinely love to see it, even if out of scope) |
|
Having functions (that aren't package expressions) at the top-level would be confusing. IMO it merits going in some other namespace (there's a |
Already happens a ton (just open up Noogle and scroll down past the subattrs). I don't like it - I wish there was a separate output for functions that rely on |
Unfortunately not. |
|
I've opened a draft PR in home-manager, with a "real" example of a module. It's basically a subset of what i want |
|
(rebasing because the home-manager PR depends on new maintainers entries that didn't exist in nixpkgs 3 weeks ago) |
I think you're right about the top level. That's an oversight on my part - it should be pretty easy to fix, I just didn't think of it. You can see how it works if you nest things under a top-level node: lib.hm.generators.toKDL { } {
value._children = [
{
output._args = ["DP-1"];
output.mode = "2560x1440";
}
{
output._args = ["HDMI-A-1"];
output.mode = "1920x1080";
}
];
}which generates: value {
output "DP-1" {
mode "2560x1440"
}
output "HDMI-A-1" {
mode "1920x1080"
}
}I think we could either add support for The design principle here is that "normal" nodes (e.g. nodes that only have args, or only have a single arg even) can be handled with normal-looking attrsets in Nix, and you can sprinkle in The way that I think about KDL is that it's more in the family of XML than it is in the family of JSON/YAML/etc. It represents a more highly-structured document, and it doesn't just have data types that cleanly map to Nix's list/attrset/scalars. The (Incidentally, this is a problem with YAML in Nix - YAML has complex value types called "tags" that can't be generated by the built-in YAML generator because it can only generate JSON. See how e.g. the |
Yeah, probably both. The "children" of a node should have exactly the same schema as the toplevel part.
I wouldn't say "more highly-structured", but it's fundamentally differently structured. The data types do map very cleanly to Nix (for instance, the scalars are one-to-one), it's just that the toplevel datatype is a list and Nix's module system sucks at dealing with complicated lists. XML and KDL have in common that they don't allow nesting attrsets: they are only part of a node's "properties", which contain leaf values, and not full nodes. They're not just prioritizing ordered lists, they force you to use them. It's not a better or higher-level structure, just a different one.
Honestly, idk wtf
I agree that it is very nice to use attrsets for briefer configuration, but i've expressed that i think "it's an attrset unless you need an escape hatch" is not the correct way to do that. it doesn't let me do things like this^1, and it's also misleading, because that's not how KDL works. At all. It's useful for several real-world apps and services, because at the end of the day it's all deserialized to something like this (usually with a predictable schema in KDL), but it's fundamentally not how KDL is structured and i would really like if there is a "Correct" implementation of the actual structure of KDL. home-manager's A more correct way to interact with actually KDL-shaped configurations is probably with something like KDL schema for validation/merging and KDL query for reading attributes. ^1 (note that my mapping here is also somewhat misleading, it doesn't easily expose ordering, although here's my current workaround; see intended usage in this user's workspace configuration)
Yes. That's expected. I've talked about this. The point of splitting off this PR was to specifically restrict that. Notice how the type this PR exposes is
As long as we expose any general KDL configuration, callers must be aware of what KDL is, how nodes work, and what they can do with them. That named constructor is optional (you don't have to call it), but the fact that a node has attrs like |
Update pkgs/pkgs-lib/formats.nix Co-authored-by: sodiboo <37938646+sodiboo@users.noreply.github.com> Update nixos/doc/manual/development/settings-options.section.md Co-authored-by: sodiboo <37938646+sodiboo@users.noreply.github.com> use submoduleWith assert version
i think technically our paths could never be invalid UTF-8 and they're always absolute, so this is not strictly necessary, but jsonkdl 1.1.0 allows us to specify this, so we might as well.
|
Hi everyone 👋 I’ve been following this PR with interest — KDL support in nixpkgs looks like a great addition! I was wondering if there’s any consensus forming (or if decisions are still needed) around the remaining blocking points — for example, namespace placement (pkgs.formats vs lib.kdl vs pkgs.kdl), naming conventions, or related topics. Could a maintainer share whether there’s a clear direction, or what the decision-making process might look like to help move this PR forward? Also, is there anything missing in the PR that you would like help with @sodiboo? I’d be happy to contribute if there’s something I can do to help it progress. |
|
|
3cba7bb to
ce64143
Compare
TODO: properly handle v2 merging
c6c1a36 to
0b73a0c
Compare
|
any update on this? can i help with something? |
|
Stale pr? I would love to have this, even it its would do IFD, but as idea, would it be better to implement this at the |
no, to be a builtins function you would need to ALWAYS provide the same input/output as long as the tree stays the same. there is a reason why builtins.toTOML doesnt exist
|
Like #412029, but different somehow? Idk, this isn't necessarily separate, it doesn't "supersede" that PR, but i implemented some code changes in my latest comment, and i'd like to get feedback on it, so it's nice to have a separate code review thing.(i guess it does supersede it now. and closes #198655)
Mainly, in this comment, i wrote a bunch about what i thought could be a more ergonomic way to define a KDL document. In this PR, i've also updated the documentation for how to use the KDL format with my changes. However, it is notably quite long! Previously, i was quite adamant that this KDL format belongs in
pkgs.formats, but because the documentation i wrote is relatively much longer and i think that the KDL format in particular kinda needs a usage example, it probably no longer belongs in the same section as all the other formats, but i do think that documentation does belong somewhere. So, i'm open for suggestions for alternate namespaces like nezia first suggested here.Scope
The commit formatting is currently a bit "unpolished": before this is ever merged (if it is merged), they should all be squashed into a
formats.kdl: init(or whatever other namespace we decide on). The initial commit that gets merged does not need all the history of our discussions.The particular way i've implemented some helper types (
uniqFlat{List,Attrs}Of) by placing them in the format is probably not idiomatic? Like, i recognize this needs to change? But i'm not sure how exactly.The main thing i want feedback on first is not the specifics of how i've written my library code, but rather, do we think the way i suggest the KDL format be used, is reasonable? At least, for the purposes of
niriwhich is the program i maintain a Nix module for, this is exactly what i need and want. Please comment on how using this is like when writing a configuration in the style of the four configs mentioned at the end of this comment. Please comment on the documentation i've written, if it's fundamentally wrong, or my explanations are weird. Please comment on all the user-facing aspects of this.I also realize (like i've said in the other PR) that long-term, there needs to be a better "extensible" KDL settings-like format. Something where you can define rules for how sections are merged, and then define the config in multiple locations and have them merged in an application-specific way. I recognize that this should exist. This PR isn't that. This PR is a foundation for that by making the KDL document format representable in Nix. That's all. But i want to make that representation ergonomic to write.
Call for module authors
Also, are there any other modules that currently exist outside of Nixpkgs with a KDL configuration? I genuinely don't know! And I don't know what those applications need. I'm approaching this mainly from the perspective of
niri(and the needs of my niri-flake), so the "style" of flattening the document and having optional nodes is taken from my existing libraries in that repo. I think what i've put together here is sufficiently simple and generic that it's easy to grasp, and useful to most module authors.namespacing
Once the KDL formats are merged and available in stable Nixpkgs, i plan to make use of them in my
niri-flake. What i want to do, then, is to have my settings module be a standalone thing to import, which you can place manually into your config. It will have a "main" output like.renderedwhich should have a type of "KDL document". I don't really want to have to thread through apkgsinstance to make use of the KDL type. This is a primary reason why i'd love to have it in a different namespace.My settings module shouldn't be inherently dependent on any packages: it's just Nix library code, and you're (eventually) supposed to plug in the
.renderedpart from my comprehensive settings mapping, into theconfig =part of the upstream modules (which exist inhome-manager? perhaps?). But i do want to make use of the KDL type from Nixpkgs; i don't want to have to reimplement the "splatting" logic in niri-flake just to avoid importingpkgs(although i am currently reimplementing it, so?? i can just keep that? it's not a Huge Problem)recommended usage patterns
As i said above, i think it's actually really important to have a usage example. Mainly, i want to encourage module authors making use of this format to not name their options
settings. That should be reserved for an "extensible kdl"-like format, with multi-definition merging.But what should we call the options? I think for niri, it would make sense to just have a single
configoption, for the entire configuration. Like, the upstream modules (nixpkgs, home-manager, hjem-rum, etc), really don't need to do much: for instance, niri will already read the systemdLocale1settings to choose the keyboard layout, so the existing keyboard options should just apply without any configuration.For more complex situations, where a single "config" is not viable, i think
renderedis a good name for the full config (or for any submodule of it, that can render a "partial" config as a KDL document). That name comes from the fact that it should only be assigned once, in a place where you "render" your Nix options into a single KDL document.The distinction here is that
configis for "user-defined" configuration inputs, andrenderedis for "output" configurations from a Nix module. It's also easy to go fromconfigtorenderedby making a renameconfig->extraConfig.If you have multiple large sections of config, which are mapped to Nix submodules, each dealing with their own bit, it makes sense to render those subsections independently.
So, for instance, a potential way niri-flake could look in the future is to have a top-level
renderedoutput like so:"splat"
The above example is also making use of my "splat" functionality, which is maybe not the best name for it? It sounds like it's a language feature, when it's not. But idk what else to call it. In particular, this is not just more ergonomic than having to
++different parts of the config, but it is also more lazy (as mentioned in my last comment on the other PR).mkMergewould also be equally as lazy, but its usage is indistinguishable from defining the config in multiple places, which is a footgun when working with raw KDL documents, because implicit concatenation across files is almost never what you want.The splatting could perhaps be made explicit? I think my code would look uglier then, but i could see use for something like a
lib.mkSplatthat can "splat" multiple list elements, at a single element definition. Currently, list merging allows elements to havelib.mkIfandlib.mkMergeand if they resolve to no definitions, the element is deleted; otherwise, the definitions are merged as that elemType instead of becoming multiple list elements. But in KDL documents, we want a more "splat"-like behaviour; which is why i implemented that (and not in terms oflib.mkMerge, because it has different semantics). In regularlistOf, obviously such a splat would have to be explicit with alib.mkSplat, and if this existed already, i would just use it in my implementation; but since it doesn't, i made it implicit, which works fine here because "KDL document in KDL document" can only really mean you want to splat.Is the splatting actually a good idea? I think it will make using the KDL format a lot nicer. I will make extensive use of the splatting in my code. Or whatever we call it.
cc @KiaraGrouwstra @nezia1 (the two others who are currently participating in discussion in the other PR)
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.