Skip to content
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

pkgs-lib/formats: support top level INI keys #118925

Closed
wants to merge 1 commit into from

Conversation

sternenseemann
Copy link
Member

@sternenseemann sternenseemann commented Apr 9, 2021

lib/generators: support top level keys in toINI

Many packages using INI as their configuration format allow top-level
keys that don't belong to a section before the first section, for
example gmnisrv and foot. However lib.generators.toINI and
pkgs.formats.ini don't support this currently. This commit introduces
support for this.

All keys in the attribute set passed are considered as top-level entries
if they are not attribute sets (that is don't represent a section).
Introducing a switch for turning this behavior on or off doesn't seem
necessary as there is no valid representation for such values in the
input attribute set for the use case without top-level attributes.

Now such things are possible:

lib.generators.toINI {} {
  foo = 13;
  bar = {
    baz = 12;
    hello = "world";
  };
} == ''
  foo=13
  [bar]
  baz=12
  hello=world
''

Thanks to Ben Sima for cleaning up and porting a snippet I had for
proper nixpkgs!

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

lib/generators: support top level keys in toINI

Many packages using INI as their configuration format allow top-level
keys that don't belong to a section before the first section, for
example gmnisrv and foot. However lib.generators.toINI and
pkgs.formats.ini don't support this currently. This commit introduces
support for this.

All keys in the attribute set passed are considered as top-level entries
if they are not attribute sets (that is don't represent a section).
Introducing a switch for turning this behavior on or off doesn't seem
necessary as there is no valid representation for such values in the
input attribute set for the use case without top-level attributes.

Now such things are possible:

```nix
lib.generators.toINI {} {
  foo = 13;
  bar = {
    baz = 12;
    hello = "world";
  };
} == ''
  foo=13
  [bar]
  baz=12
  hello=world
''
```

Thanks to Ben Sima for cleaning up and porting a snippet I had for
proper nixpkgs!

Co-authored-by: Ben Sima <ben@bsima.me>
@infinisil
Copy link
Member

See the discussion in #107491

@sternenseemann
Copy link
Member Author

Oh yeah, that hadn't crossed my mind yet. Pretty unfortunate.

Under the assumptions that a) we can check beforehand if such a collision could occur (in the service's documentation) and b) in practice these collisions are somewhat rare, I would propose the following solution:

We make it configurable to turn this feature on and off and add the option to bind it to a special section (whose name is configurable) instead in case there may be collisions. By default, this would be turned off.

This would ensure that:

  • Module authors have to actively configure/enable the feature and will check if there may be collisions or not and decide accordingly.
  • In case a collision may be happening, module authors will have to choose a name for the global section and then also (hopefully) document it via the freeformType submodule.
  • We don't pollute the type description of module options unnecessarily with global sections support it is arguably pretty bulky.

The downside of this is of course that it would have an unusual interface for users in case the global section is named instead of top-level due to possible collisions. Essentially we would choose convenience (in a lot of cases) over consistency (with a few cases).

What do you think? I'm happy to implement either solution pretty much.

@Profpatsch
Copy link
Member

Hm, a lot of fancy encodings have been proposed, but I kinda didn’t see the most obvious one:

Add a generators.toINIWithGlobalSection that takes an attrset like

{ 
  globalSection = {
    a = "foo";
    … more non-attrset keys
  };
  sections = {
    … what can be passed to `generators.toINI`
  };

@aanderse
Copy link
Member

That proposal isn't confusing 👍 Most others have been confusing.

@stale
Copy link

stale bot commented Oct 22, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 22, 2021
Profpatsch added a commit to Profpatsch/nixpkgs that referenced this pull request Mar 14, 2022
As discussed in
NixOS#118925 (comment),
this is the best way of adding global sections to `toINI` without
employing heuristics (i.e. checking whether something is an attrset).
github-actions bot pushed a commit to arcnmx/nixpkgs-lib that referenced this pull request Apr 7, 2022
As discussed in
NixOS/nixpkgs#118925 (comment),
this is the best way of adding global sections to `toINI` without
employing heuristics (i.e. checking whether something is an attrset).
github-actions bot pushed a commit to nix-community/nixpkgs.lib that referenced this pull request Apr 10, 2022
As discussed in
NixOS/nixpkgs#118925 (comment),
this is the best way of adding global sections to `toINI` without
employing heuristics (i.e. checking whether something is an attrset).
@sternenseemann
Copy link
Member Author

See #164088, although it still lacks a pkgs-lib formats wrapper.

@tim-tx
Copy link
Contributor

tim-tx commented Nov 17, 2022

@sternenseemann I attempted a pkgs.formats.ini wrapper in #201569

github-actions bot pushed a commit to nix-community/nixpkgs.lib that referenced this pull request Dec 4, 2022
As discussed in
NixOS/nixpkgs#118925 (comment),
this is the best way of adding global sections to `toINI` without
employing heuristics (i.e. checking whether something is an attrset).
github-actions bot pushed a commit to nix-community/nixpkgs.lib that referenced this pull request May 31, 2023
As discussed in
NixOS/nixpkgs#118925 (comment),
this is the best way of adding global sections to `toINI` without
employing heuristics (i.e. checking whether something is an attrset).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants