pkgs.formats: Add libconfig format generator

[h7x4]

Description of changes

This is a new attempt at #208747, with the wider goal of solving the obstruction of #167388.

I've addressed some of the comments in the earlier PR, specifically the ones about not generating it in nix, and about the number formats. This implementation exports the attrset as JSON, and converts it using a relatively small rust program. It supports include directives, and special types like hex, octal and scientific notation floats. I've (ab)used the fact that libconfig doesn't allow their setting names to start with _ (see <name>), to ensure the expressiveness stays the same.

One point that might be a bit iffy, is the way the generator automatically determines whether something should be a list or an array. It reduces the expressive power a little bit, as it's now impossible to create lists of equal-typed values. I do not think it should be a problem, but it should probably be discussed.

There exists a test here:

nix-build -A pkgs.tests.pkgs-lib.libconfig.comprehensive

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[dali99]

@ofborg eval

[ckiee]

Looks good, but the commit prefix should probably be pkgs-lib or formats.libconfig, there's no precedent I can see for pkgs.formats.* in the git log.

[h7x4]

Seems like people are either using formats.*: or pkgs-lib:. I think I'll rewrite them to formats.libconfig

[ckiee]

It should still be tested to see if it works well against logiops and sslh.

[roberth]

Some suggestions that should make it work with a cross-compiling Nixpkgs.

[felixalbrigtsen]

nix build "github:h7x4/nixpkgs/add-libconfig-format-generator#legacyPackages.aarch64-darwin.tests.pkgs-lib.libconfig.comprehensive"

Builds successfully for me on aarch64-darwin as well!

[rnhmjoj]

I tested this and rebased my sslh PR on this: everything seems to work fine.

[rnhmjoj]

@roberth can you finish your review? I'd like to get this merged because it's blocking other PRs.

[roberth]

Final comments 🚀

[roberth]

Does the library support pretty printing?
Easily readable files are not technically required, but if you do need to open one of these, it would be a lot nicer with at least a few more line breaks. Troubleshooting is annoying enough as is ;)

(but only do it if the library can do it for you)

[h7x4]

libconfigs config_write_file function does seem to pretty print by default. I think this would be easy to do in the rust program as well, if we want to avoid piping stuff through several binaries? Not keen on doing FFI.

Would it be okay to add as an argument to the format options, like pkgs.formats.libconfig { pretty = true; }?

[ckiee]

A pretty option would ~2x the testing and make the diff even bigger. I'm in favor of hardcoding it for pretty output.

(We are not using a library to serialize to libconfig, only validating against libconfig.)

[rnhmjoj]

Can we get this done before the NixOS 23.11 freeze, please?

[ckiee]

I think that's a good cause to skip implementing the pretty output in this PR, if we want to not put any more pressure on @h7x4.

It shouldn't be hard to implement later, in the Rust serializer. The (self-imposed!) deadline is Oct 30.

Upon pulling & rebasing onto the latest nixos-unstable I was met with the include_file hash changing, which I believe is probably a result of #244835... I have updated these and pushed the rebased version, the test passes.

[libconfig] validation ok
installing
[…]
running tests
--- /nix/store/s9fpgmdz5zyabk4wh69vpky01qcdf2yg-expected.txt    1970-01-01 00:00:01.000000000 +0000
+++ /nix/store/kj4x8rdxk27bxcshy6c7hn7r0d6nb4br-libconfig-test.cfg      1970-01-01 00:00:01.000000000 +0000
@@ -1,6 +1,6 @@
 array1d=[1, 5, 2];array2d=([1, 2], [2, 1]);list1d=(1, "mixed!", 5, 2);list2d=(1, (1, 1.2, "foo"), ("bar", 1.2, 1));nasty_string="\"@
 \\     ^*bf
 0\";'''$";nested={attrset={has={a={integer={value=100;};};};};};simple_top_level_attr="1.0";some_floaty=29.95;weirderTypes={
-@include "/nix/store/jcp2wambykcd4nrhd720zg5j1aygn6jh-libconfig-test-include"
+@include "/nix/store/dz01aq1prsxdh0sxj801q2jk97rng2zm-libconfig-test-include"
 array_of_ints=[0732, 0xa3, 1234];bigint=9223372036854775807;float=0.0012;hex=0x1fc3;list_of_weird_types=(3.141592654, 9223372036854775807, 0x1fc3, 027, 1.2e-32, 1.0);octal=027;pi=3.141592654;};
[…]
error: building '/nix/store/c9rhjk5vgwa0326hl9hv0p3c3a7cz86d-pkgs.formats.libconfig-test-comprehensive.drv' failed

Also, looking at how quickly #244835 got merged is dismaying when compared to this one, just one review with no nits and it didn't even pass through staging for any stray NixOS tests as far as I can see.

EDIT: I don't think #244835 is actually related anymore, it didn't replace trivial-builders. Oops.

[h7x4]

@roberth I think I'll pass over the prettification for now, if you don't mind. Is there anything more needed for merge?

[h7x4]

@roberth friendly ping

[roberth]

Thank you @h7x4!

[rnhmjoj]

Thank you!

[vcunat]

git bisect blames this merge for a regressed test:
https://hydra.nixos.org/build/239737533/nixlog/1/tail
Local command e.g.

nix build -f pkgs/top-level/release.nix tests.pkgs-lib.x86_64-linux

EDIT: this test-set blocks nixpkgs-unstable channel (but not others).

[ckiee]

This should have been apparent (as an obvious layering violation) (edit: there was an attempt to fix it actually) , but I think for me it was fatigue from looking at this code for many months. I'm pretty sure others can relate :P

Anyway, fixed in #264581.