primops: add builtins.toTOML

[polykernel]

Motivation

This commit introduces the a new experimental builtin function: builtins.toTOML.
The builtin is guarded by an experimental feature to-toml to allow
more time for testing that round-trips with builtins.fromTOML works
properly.

The functions printValueAsTOML were introduced to serialize Nix
values to TOML with a slight difference in behavior from printValueAsJSON
being null values are not supported.

In order to match the global table structure TOML imposes, the argument of
builtins.toTOML was restricted to an attribute set but sublevels values
have no restriction.

A language test was added and the release notes were updated to document the addition.

Context

• Closes: add builtins.toTOML #3929
• Related: Parse TOML timestamps as strings #8120

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

• agreed on idea
• agreed on implementation strategy
• tests, as appropriate
  • functional tests - tests/**.sh
  • unit tests - src/*/tests
  • integration tests - tests/nixos/*
• documentation in the manual
• documentation in the internal API docs
• code and comments are self-explanatory
• commit message explains why the change was made
• new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

[polykernel]

Following https://discourse.nixos.org/t/2023-06-02-nix-team-meeting-minutes-59/28666, I have added the builtin behind an experimental feature flag and updated the release notes. I wasn't sure what a good name for the feature would be so I just used to-toml for now. Any suggestions for a better name would be greatly appreciated.

[fricklerhandwerk]

Discussed in the Nix team meeting 2023-06-05:

• @roberth raised doubts about reproducibility
  • @edolstra: agreed, it's hard to get it right in a way such that it doesn't have to change. non-trivial specification problem
  • @roberth: can be implemented as a Nix language library, even if it's not fun at all
  • @Kranzes: there is a Nixpkgs tool that works at build time, but that's a different animal

@polykernel is there anything that would prevent using a Nix language library to satisfy your use case, apart from implementing one being tedious? Sorry for the back and forth, but as @roberth and @edolstra point out, we don't want to commit to something that may be a huge long-term liability to maintain while keeping reproducibility. It would outweigh the benefits of a symmetric interface for (de)serialising TOML. Unless there's a very strong reason to have a builtin that we haven't considered, we'd close the PR.

[nixos-discourse]

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

https://discourse.nixos.org/t/2023-06-05-nix-team-meeting-minutes-60/28933/1

[polykernel]

@fricklerhandwerk I don't have any particular argument against using a Nix language library to fulfill the role of this builtin. I created this PR while looking at some stale issues with the language label and thought it may be helpful to implement the feature(I understand now not to do so without careful thought). As I am personally not a user of the builtin, it may be better to reference the decision in the original issue at #3929 to see if potential users have any unsatisfied use cases or arguments. That said, I am fine with closing the PR.

[roberth]

@polykernel, your help is much appreciated. We did discuss the possibility of reusing the code you wrote in a test. If that's the sort of thing you're interested in, writing a property test with rapidcheck, that might be a worthwhile destination for this code.

Regarding the situation, I feel bad that you've seemingly done the right thing, but it did not lead to the intended result. Nix has some tricky requirements. We want to make an effort to communicate those better in design docs / contribution docs, but I don't think that takes away the risk entirely. We try to triage as much as possible, which should also help prevent situations like this, but that's not guaranteed to happen in time, and may not be thorough enough in some cases.

I think the idea approved label is one to pay attention to. We put that one on issues that we've triaged and we think are worth working on, where we see no significant problems. We don't have many of those that also have a language label at this time. Maybe that's something the Nix team could focus on a bit more.

[polykernel]

@roberth Thanks for the encouragement. I would be happy to help with reusing the code to write property tests. As for the property tests specifically, would the tests be for checking roundtrips with builtins.fromTOML? Also, should I open a new PR for this new direction of work and close this one?

Regarding the situation, I feel bad that you've seemingly done the right thing, but it did not lead to the intended result.

Not to worry. I understand that design is difficult and that sometimes decisions change based on new inputs and considerations. Regardless of the result, I considered this to be a good learning opportunity to get more familiar with the Nix codebase.

I think the idea approved label is one to pay attention to. We put that one on issues that we've triaged and we think are worth working on, where we see no significant problems.

I will definitely look out for issues with the idea approved label and see if I can help.

[roberth]

would the tests be for checking roundtrips with builtins.fromTOML?

Yes, basically fromTOML (toTOML x) == x for all generated x.
I should warn btw that I've seen a broken arbitrary implementation in the tests that I haven't managed to fix yet, and there might be more broken ones, so for creating Gen<>, I would recommend to use the upstream documentation and not look at examples in the codebase for now.

should I open a new PR for this new direction of work and close this one?

I think it's a bit cleaner to create a new one, mostly because of the branch name, but a fresh discussion thread is also nice imo.

[polykernel]

Yes, basically fromTOML (toTOML x) == x for all generated x. I should warn btw that I've seen a broken arbitrary implementation in the tests that I haven't managed to fix yet, and there might be more broken ones, so for creating Gen<>, I would recommend to use the upstream documentation and not look at examples in the codebase for now.

I am currently looking at the rapidcheck and gtest documentation, it may take some time since I am not familiar with either framework. I will try to get a preliminary implementation as soon as possible and close this PR then.

Edit: This did not age well, some real life stuff got in the way. I am restarting work on the PR after some obstacles. I hope to get it ready soon.