lib/generators: add RFC42-style libconfig format by ckiee · Pull Request #208747 · NixOS/nixpkgs

ckiee · 2023-01-02T12:36:18Z

Description of changes

Hey again. I've split out the libconfig serializer from #167388 and took the time to acknowledge a few more edge-cases, as well as adding a test and auto-validator.

Things done

nixos-discourse · 2023-01-02T13:27:48Z

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/1647

infinisil · 2023-01-02T14:20:29Z

I'm wondering, wouldn't it be better to write these generators in a generic programming language, converting from the builtins.toJSON output to the desired format? While this wouldn't work for lib.generators since it needs to return a Nix string, it should work for pkgs.formats, which is mostly what's used for configuration generation nowadays. The benefits would be faster evaluation, ability to reuse existing tooling, better testability.

ckiee · 2023-01-02T15:32:53Z

@infinisil: I think that is the way forward generally but libconfig doesn't really have much of an ecosystem or any CLI tools as far as I can tell so I'd like to continue with this PR as-is. I'm already 3.5 layers deep (#165936 -> #167388 -> [this PR] -> taking over as upstream maintainer soon most likely) and would really rather not recurse more.

infinisil · 2023-01-05T17:56:29Z

It's just that this adds a whole bunch of code that's slow to evaluate, hard to debug, test and maintain. And yes that's half of nixpkgs already but I don't think it's a good idea to make this worse. Others might review and merge this PR, and I won't interfere with that, but personally I'm gonna abstain from this and focus on cleaner code instead.

roberth

I agree with infinisil, and the semantics are fragile; see comment.

roberth · 2023-01-08T21:55:05Z

lib/generators.nix

Please don't assign meaning without context. Such assumptions are likely to lead to bugs.

"12A" -> "12A" "12B" -> "12B" ... "12L" -> 12

This is not how Nix works. Strings are strings. If you want a number, you write a number, or call a function that returns one.

Instead, you could represent these special with special wrappers instead mkOctal, etc, but do you really need multiple ways to write the same number?

but do you really need multiple ways to write the same number?

Spec says there are a few scalar types and @anund previously said:

~ This [couldn't] currently generate valid config for the example. :(

Other then that mk{Octal|…} seems reasonable but annoying, future note: make it return just a simple { _type = …; … } attrset.

I'll get to this Eventually.

Playing with https://github.com/PixlOne/logiops/blob/master/logid.example.cfg as an example.

devices = [ { #<snip> out direct translations buttons = [ { cid: (toLibConfig.mkOctal "0xc3"); action = { type: "Gestures"; gestures: [ { direction: "Up"; mode: "OnRelease"; action = { type: "Keypress"; keys: (toLibConfig.mkArray ["KEY_UP"]); }; } ]; }; } ]; } ]; #nesting would also need to work (toLibConfig.mkArray [(toLibConfig.mkOctal "0x3")]);

Is that the sort of wrappers you have in mind @roberth?

If NixOS/nix#7578 is implemented, the hex and octal numbers won't need any sort of escaping or wrapping. That is, unless the program parsing the config makes decisions based on the syntax used, which would be decimal regardless of what base the module user used to configured the number.

I don't think we have many examples of wrappers yet, but in the module system we generally use a representation that's an attrset with a _type attribute. For example:

mkOverride = priority: content: { _type = "override"; inherit priority content; };

I don't think we have any examples of formats using this approach. (I kind of expected at least something RFC42-like to use this, so sorry for my brevity earlier)
We could add a general mkUnescape to lib, but otherwise I don't know exactly where to put it. Perhaps we could have (formats.foo {}).type and also (formats.foo {}).lib?
The meaning of this mkUnescape is that it would take a string and concatenate it directly into the config file without any processing (except perhaps re-indenting, for some formats). This is in contrast to regular strings, which should be escaped to become strings in the target config language. This means that the string is reinterpreted according to the target configuration language syntax, and this would allow a user to specify any syntax, assuming it's going to be valid in the end. This includes such things as hex: lib.mkUnescape "0xc3" would do the job.

cc @infinisil

That's not to say that a formats.libconfig.mkOctal wouldn't be nice to have.

I also notice I've mixed up octal and hex in my example(s).

A quick survey: octal is 0o[0-7]+ in python/haskell, 0[0-7]+ in java/c/golang/libconfig. This might be a format that requires a wrapper. (or a pattern of read as octal write as decimal?)

roberth · 2023-01-08T21:56:18Z

lib/generators.nix

These should be escaped and/or validated, but that would be better to do in a proper program as infinisil suggests.

anund · 2023-01-10T01:19:10Z

The type system for libconfig includes types not expressible directly in nix (or json). Lists and Arrays can't be unambiguously selected given a nix list. (a List is close to a nix list, an Array must contain only scalars of the same type). It's unclear if there are consumers of libconfig files that would need to care about a Group vs Array vs List. Expressible numbers include 0x123, -1, +1.0, or 5L. Octal and positive/negative I don't think are expressible. Long values might be? Include directives are also awkward to encode.

For logiops in particular the usually documented config style includes references to octal number values. Ideally experimenting with packages in Nixos doesn't also come with experimenting with translating config examples.

With that context I have trouble interpreting the intention of RFC 42. Something like toLibConfig would require documenting the above mismatch everywhere it's used and generally adding .file inputs as an escape hatch. That sort of approach generates documentation issues. Is there a good way to handle this in Nixpkgs? Examples where nix config and file inputs are mixed perhaps?

ckiee · 2023-01-10T03:37:50Z

The type system for libconfig includes types not expressible directly in nix (or json). Lists and Arrays can't be unambiguously selected given a nix list. (a List is close to a nix list, an Array must contain only scalars of the same type). It's unclear if there are consumers of libconfig files that would need to care about a Group vs Array vs List.

In this PR I poked your patch a bit more to get the behavior closer to what we want. (though I don't remember the specifics very confidently ATM) and perfection is the enemy of good. (no toLibconfig currently!)

Expressible numbers include 0x123, -1, +1.0, or 5L. Octal and positive/negative I don't think are expressible. Long values might be?

deferring on @roberth's thread →

Include directives are also awkward to encode.

Yes and although I'm not sure if these should block this PR I think "@include" = keys as (also similarly discussed in the previous thread (@#167388)) are at least partially no go due to the awkward merging config value merging: Two includes? Maybe. "foo.list" and "foo.array"? Definitely not.

For logiops in particular the usually documented config style includes references to octal number values. Ideally experimenting with packages in [NixOS] doesn't also come with experimenting with translating config examples.

This is kind of a wider "found problem" with RFC 42 and I have ran into it before but it's usually been resolved quickly by opening the docs and looking at the example or defaults. (see @#167388)

With that context I have trouble interpreting the intention of RFC 42. Something like toLibConfig would require documenting the above mismatch everywhere it's used and generally adding .file inputs as an escape hatch. That sort of approach generates documentation issues. Is there a good way to handle this in Nixpkgs? Examples where nix config and file inputs are mixed perhaps?

RFC42 is already widely deployed (search has 51 merged results) and with all of the things I've tried so far this hasn't been that much of a problem? libconfig isn't too complicated a format considering #192285 akkoma: init at 3.5.0 was recently merged using this elixrConf voodoo.

I'm pretty sure neither of us are calibrated just right, but after all of the nixpkgs NixOS modules I've consumed I have yet to touch environment.etc in my own config, which is always there along with mkForce.

pkgs/pkgs-lib/formats.nix

anund · 2023-01-10T12:09:01Z

You are right, this work is in the land of compromise. I'm no expert in what's being done in the scope of nix options. Consider the above a mixture of documentation on the mismatch between nix and libconfig and an open ended request for a pattern, any pattern, to use in cases where nix types are insufficient for the target config schema. (I favour a .file option with my limited experience)

One of the least enjoyable loops in nix happens when grabbing a new package with a nix options example of {}. This typically involves needing to; learn a new config format, learn a new translation scheme, and iterate through a cycle of which step is wrong? the nix -> the generated config -> the program + config. That's a lot of the motivation for my original requests for examples and attempts to limit the need to mentally map things like 'the source docs use octal but decimal will also work'.

ckiee · 2023-10-03T17:16:13Z

Proceeded by #246115.

ckiee requested review from Profpatsch, edolstra, infinisil and nbp as code owners January 2, 2023 12:36

ckiee mentioned this pull request Jan 2, 2023

nixos/logiops: init #167388

Closed

13 tasks

ckiee added 3 commits January 2, 2023 14:42

lib/generators: add toLibconfig

f9ec1b3

pkgs-lib: add libconfig format

97ccf6b

lib/tests: add testToLibconfigComprehensive

2b45729

ckiee force-pushed the lib-add-gen-libconfig branch from a2fe50f to fb8147a Compare January 2, 2023 12:46

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 2, 2023

roberth requested changes Jan 8, 2023

View reviewed changes

anund reviewed Jan 10, 2023

View reviewed changes

pkgs/pkgs-lib/formats.nix Outdated Show resolved Hide resolved

pkgs-lib: add libconfig output validation

28a27cd

ckiee force-pushed the lib-add-gen-libconfig branch from fb8147a to 28a27cd Compare January 10, 2023 13:15

roberth mentioned this pull request Jan 10, 2023

Support hexadecimal and octal syntax for integers NixOS/nix#7578

Open

h7x4 mentioned this pull request Jul 30, 2023

pkgs.formats: Add libconfig format generator #246115

Merged

12 tasks

ckiee mentioned this pull request Aug 14, 2023

nixos/sslh: update and refactor for RFC42 #245855

Merged

12 tasks

ckiee closed this Oct 3, 2023

h7x4 mentioned this pull request Feb 10, 2024

nixos/logiops: init #287399

Open

13 tasks

h7x4 mentioned this pull request Aug 1, 2024

formats.kdl: init #295211

Closed

13 tasks

Uh oh!

Conversation

ckiee commented Jan 2, 2023

Description of changes

Things done

Uh oh!

nixos-discourse commented Jan 2, 2023

Uh oh!

infinisil commented Jan 2, 2023

Uh oh!

ckiee commented Jan 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

infinisil commented Jan 5, 2023

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

roberth Jan 8, 2023

Choose a reason for hiding this comment

Uh oh!

ckiee Jan 8, 2023

Choose a reason for hiding this comment

Uh oh!

anund Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roberth Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roberth Jan 10, 2023

Choose a reason for hiding this comment

Uh oh!

anund Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

anund Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

roberth Jan 8, 2023

Choose a reason for hiding this comment

Uh oh!

anund commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ckiee commented Jan 10, 2023

Uh oh!

Uh oh!

anund commented Jan 10, 2023

Uh oh!

ckiee commented Oct 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ckiee commented Jan 2, 2023 •

edited

Loading

anund Jan 10, 2023 •

edited

Loading

roberth Jan 10, 2023 •

edited

Loading

anund commented Jan 10, 2023 •

edited

Loading