~andir/nixpkgs-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
7 3

[PATCH v2 1/2] nixos/gmnisrv: init

Details
Message ID
<20210409155009.3663-1-ben@bsima.me>
DKIM signature
missing
Download raw message
Patch: +58 -0
---
 nixos/modules/module-list.nix                 |  1 +
 .../modules/services/web-servers/gmnisrv.nix  | 57 +++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 nixos/modules/services/web-servers/gmnisrv.nix

diff --git a/nixos/modules/module-list.nix b/nixos/modules/module-list.nix
index 509bccb1ec7..75de48e557c 100644
--- a/nixos/modules/module-list.nix
+++ b/nixos/modules/module-list.nix
@@ -944,6 +944,7 @@
  ./services/web-servers/caddy.nix
  ./services/web-servers/darkhttpd.nix
  ./services/web-servers/fcgiwrap.nix
  ./services/web-servers/gmnisrv.nix
  ./services/web-servers/hitch/default.nix
  ./services/web-servers/hydron.nix
  ./services/web-servers/jboss/default.nix
diff --git a/nixos/modules/services/web-servers/gmnisrv.nix b/nixos/modules/services/web-servers/gmnisrv.nix
new file mode 100644
index 00000000000..eb4f99c3772
--- /dev/null
+++ b/nixos/modules/services/web-servers/gmnisrv.nix
@@ -0,0 +1,57 @@
{ lib
, options
, config
, pkgs
, modulesPath
}:

let
  cfg = config.services.gmnisrv;
  cfgFormat = pkgs.formats.ini { };
in {
  options.services.gmnisrv = {
    enable = lib.mkEnableOption "Enable the gmnisrv service";
    settings = lib.mkOption {
      type = cfgFormat.type;
      description = ''
        Configuration for gmnisrv. See gmnisrv.ini(5) for supported settings.
      '';
    };
    package = lib.mkOption {
      type = lib.types.package;
      default = pkgs.gmnisrv;
      description = "gmnisrv package to use";
    };
    listen = lib.mkOption {
      type = lib.types.str;
      default = "0.0.0.0:1965 [::]:1965";
      description = ''
        Host address and port to listen on. See gmnisrv.ini(5) for syntax
        details.
      '';
    };
    dataDir = lib.mkOption {
      type = lib.types.str;
      default = "/var/lib/gemini";
      description = "Where gmnisrv should store certs and other data.";
    };
  };
  config = lib.mkIf cfg.enable {
    services.gmnisrv.settings = {
      "listen" = cfg.listen;
      ":tls" = {
        "store" = lib.mkDefault "${cfg.dataDir}/certs";
      };
    };
    environment.etc."gmnisrv.ini" = {
      enable = true;
      source = cfgFormat.generate "gmnisrv.ini" cfg.settings;
    };
    systemd.services.gmnisrv = {
      description = "gmnisrv service";
      wantedBy = [ "multi-user.target" ];
      after = [ "network-online.target" ];
      script = "${cfg.package}/bin/gmnisrv -C /etc/gmnisrv.ini";
    };
  };
}
-- 
2.31.0

[PATCH v2 2/2] generators: support toplevel keys in toINI

Details
Message ID
<20210409155009.3663-2-ben@bsima.me>
In-Reply-To
<20210409155009.3663-1-ben@bsima.me> (view parent)
DKIM signature
missing
Download raw message
Patch: +25 -3
This was ported from sternenseemann's code:

https://github.com/openlab-aux/vuizvui/blob/1576e1025d570851449f6668e0bda2b1b9b21e06/modules/programs/foot/default.nix#L15-L48
---
 lib/generators.nix        | 12 ++++++++++--
 lib/tests/misc.nix        | 14 ++++++++++++++
 pkgs/pkgs-lib/formats.nix |  2 +-
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lib/generators.nix b/lib/generators.nix
index c8144db50ac..63d188b0f01 100644
--- a/lib/generators.nix
+++ b/lib/generators.nix
@@ -92,10 +92,13 @@ rec {
   * attrset of sections to an attrset of key-value pairs.
   *
   * generators.toINI {} {
   *   "toplevel" = "value";
   *   foo = { hi = "${pkgs.hello}"; ciao = "bar"; };
   *   baz = { "also, integers" = 42; };
   * }
   *
   *> toplevel=value
   *>
   *> [baz]
   *> also, integers=42
   *>
@@ -106,7 +109,7 @@ rec {
   * The mk* configuration attributes can generically change
   * the way sections and key-value strings are generated.
   *
   * For more examples see the test cases in ./tests.nix.
   * For more examples see the test cases in ./tests/misc.nix.
   */
  toINI = {
    # apply transformations (e.g. escapes) to section names
@@ -117,6 +120,9 @@ rec {
    listsAsDuplicateKeys ? false
  }: attrsOfAttrs:
    let
        isSection = builtins.isAttrs;
        topLevel = lib.filterAttrs (_: v: !(isSection v)) attrsOfAttrs;
        sections = lib.filterAttrs (_: v: isSection v) attrsOfAttrs;
        # map function to string for each key val
        mapAttrsToStringsSep = sep: mapFn: attrs:
          libStr.concatStringsSep sep
@@ -125,8 +131,10 @@ rec {
          [${mkSectionName sectName}]
        '' + toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } sectValues;
    in
      # construct toplevel keys
      toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } topLevel
      # map input to ini sections
      mapAttrsToStringsSep "\n" mkSection attrsOfAttrs;
      + mapAttrsToStringsSep "\n" mkSection sections;

  /* Generate a git-config file from an attrset.
   *
diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
index 0d249968402..e6a6676a89b 100644
--- a/lib/tests/misc.nix
+++ b/lib/tests/misc.nix
@@ -456,6 +456,20 @@ runTests {
    '';
  };

  testToINIToplevelKeys = {
    expr = generators.toINI {} {
      "toplevel" = "bar";
      "toplevel2" = "baz";
      "foo" = { "bar" = "baz"; };
    };
    expected = ''
      toplevel=bar
      toplevel2=baz
      [foo]
      bar=baz
    '';
  };

  /* right now only invocation check */
  testToJSONSimple =
    let val = {
diff --git a/pkgs/pkgs-lib/formats.nix b/pkgs/pkgs-lib/formats.nix
index 14589f8ecdc..b944c37bd59 100644
--- a/pkgs/pkgs-lib/formats.nix
+++ b/pkgs/pkgs-lib/formats.nix
@@ -77,7 +77,7 @@ rec {
        else
          singleIniAtom;

    in attrsOf (attrsOf iniAtom);
    in attrsOf (either iniAtom (attrsOf iniAtom));

    generate = name: value: pkgs.writeText name (lib.generators.toINI args value);

-- 
2.31.0

Re: [PATCH v2 2/2] generators: support toplevel keys in toINI

Details
Message ID
<130d8142-2f6c-642d-febe-fc87418fa60f@systemli.org>
In-Reply-To
<20210409155009.3663-2-ben@bsima.me> (view parent)
DKIM signature
missing
Download raw message
I'll go ahead and PR this upstream.

On 4/9/21 5:50 PM, Ben Sima wrote:
> This was ported from sternenseemann's code:
> 
> https://github.com/openlab-aux/vuizvui/blob/1576e1025d570851449f6668e0bda2b1b9b21e06/modules/programs/foot/default.nix#L15-L48
> ---
>  lib/generators.nix        | 12 ++++++++++--
>  lib/tests/misc.nix        | 14 ++++++++++++++
>  pkgs/pkgs-lib/formats.nix |  2 +-
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/generators.nix b/lib/generators.nix
> index c8144db50ac..63d188b0f01 100644
> --- a/lib/generators.nix
> +++ b/lib/generators.nix
> @@ -92,10 +92,13 @@ rec {
>     * attrset of sections to an attrset of key-value pairs.
>     *
>     * generators.toINI {} {
> +   *   "toplevel" = "value";
>     *   foo = { hi = "${pkgs.hello}"; ciao = "bar"; };
>     *   baz = { "also, integers" = 42; };
>     * }
>     *
> +   *> toplevel=value
> +   *>
>     *> [baz]
>     *> also, integers=42
>     *>
> @@ -106,7 +109,7 @@ rec {
>     * The mk* configuration attributes can generically change
>     * the way sections and key-value strings are generated.
>     *
> -   * For more examples see the test cases in ./tests.nix.
> +   * For more examples see the test cases in ./tests/misc.nix.
>     */
>    toINI = {
>      # apply transformations (e.g. escapes) to section names
> @@ -117,6 +120,9 @@ rec {
>      listsAsDuplicateKeys ? false
>    }: attrsOfAttrs:
>      let
> +        isSection = builtins.isAttrs;
> +        topLevel = lib.filterAttrs (_: v: !(isSection v)) attrsOfAttrs;
> +        sections = lib.filterAttrs (_: v: isSection v) attrsOfAttrs;
>          # map function to string for each key val
>          mapAttrsToStringsSep = sep: mapFn: attrs:
>            libStr.concatStringsSep sep
> @@ -125,8 +131,10 @@ rec {
>            [${mkSectionName sectName}]
>          '' + toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } sectValues;
>      in
> +      # construct toplevel keys
> +      toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } topLevel
>        # map input to ini sections
> -      mapAttrsToStringsSep "\n" mkSection attrsOfAttrs;
> +      + mapAttrsToStringsSep "\n" mkSection sections;
>  
>    /* Generate a git-config file from an attrset.
>     *
> diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
> index 0d249968402..e6a6676a89b 100644
> --- a/lib/tests/misc.nix
> +++ b/lib/tests/misc.nix
> @@ -456,6 +456,20 @@ runTests {
>      '';
>    };
>  
> +  testToINIToplevelKeys = {
> +    expr = generators.toINI {} {
> +      "toplevel" = "bar";
> +      "toplevel2" = "baz";
> +      "foo" = { "bar" = "baz"; };
> +    };
> +    expected = ''
> +      toplevel=bar
> +      toplevel2=baz
> +      [foo]
> +      bar=baz
> +    '';
> +  };
> +
>    /* right now only invocation check */
>    testToJSONSimple =
>      let val = {
> diff --git a/pkgs/pkgs-lib/formats.nix b/pkgs/pkgs-lib/formats.nix
> index 14589f8ecdc..b944c37bd59 100644
> --- a/pkgs/pkgs-lib/formats.nix
> +++ b/pkgs/pkgs-lib/formats.nix
> @@ -77,7 +77,7 @@ rec {
>          else
>            singleIniAtom;
>  
> -    in attrsOf (attrsOf iniAtom);
> +    in attrsOf (either iniAtom (attrsOf iniAtom));
>  
>      generate = name: value: pkgs.writeText name (lib.generators.toINI args value);
>  
> 

Re: [PATCH v2 2/2] generators: support toplevel keys in toINI

Details
Message ID
<ed765483-893d-fe26-339e-b8c06660e659@systemli.org>
In-Reply-To
<20210409155009.3663-2-ben@bsima.me> (view parent)
DKIM signature
missing
Download raw message
Now live at https://github.com/NixOS/nixpkgs/pull/118925!

I've made a few smaller changes:

On 4/9/21 5:50 PM, Ben Sima wrote:
>  lib/generators.nix        | 12 ++++++++++--
>  lib/tests/misc.nix        | 14 ++++++++++++++
>  pkgs/pkgs-lib/formats.nix |  2 +-
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/generators.nix b/lib/generators.nix
> index c8144db50ac..63d188b0f01 100644
> --- a/lib/generators.nix
> +++ b/lib/generators.nix
> @@ -92,10 +92,13 @@ rec {
>     * attrset of sections to an attrset of key-value pairs.
>     *
>     * generators.toINI {} {
> +   *   "toplevel" = "value";

* Don't quote this for consistency.

>     *   foo = { hi = "${pkgs.hello}"; ciao = "bar"; };
>     *   baz = { "also, integers" = 42; };
>     * }
>     *
> +   *> toplevel=value
> +   *>
>     *> [baz]
>     *> also, integers=42
>     *>
> @@ -106,7 +109,7 @@ rec {
>     * The mk* configuration attributes can generically change
>     * the way sections and key-value strings are generated.
>     *
> -   * For more examples see the test cases in ./tests.nix.
> +   * For more examples see the test cases in ./tests/misc.nix.
>     */
>    toINI = {
>      # apply transformations (e.g. escapes) to section names
> @@ -117,6 +120,9 @@ rec {
>      listsAsDuplicateKeys ? false
>    }: attrsOfAttrs:
>      let
> +        isSection = builtins.isAttrs;
> +        topLevel = lib.filterAttrs (_: v: !(isSection v)) attrsOfAttrs;
> +        sections = lib.filterAttrs (_: v: isSection v) attrsOfAttrs;
>          # map function to string for each key val
>          mapAttrsToStringsSep = sep: mapFn: attrs:
>            libStr.concatStringsSep sep
> @@ -125,8 +131,10 @@ rec {
>            [${mkSectionName sectName}]
>          '' + toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } sectValues;
>      in
> +      # construct toplevel keys
> +      toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } topLevel
>        # map input to ini sections
> -      mapAttrsToStringsSep "\n" mkSection attrsOfAttrs;
> +      + mapAttrsToStringsSep "\n" mkSection sections;
>  
>    /* Generate a git-config file from an attrset.
>     *
> diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
> index 0d249968402..e6a6676a89b 100644
> --- a/lib/tests/misc.nix
> +++ b/lib/tests/misc.nix
> @@ -456,6 +456,20 @@ runTests {
>      '';
>    };
>  

* Added a test for list values and toINI in this file as well.

> +  testToINIToplevelKeys = {
> +    expr = generators.toINI {} {
> +      "toplevel" = "bar";
> +      "toplevel2" = "baz";
> +      "foo" = { "bar" = "baz"; };
> +    };
> +    expected = ''
> +      toplevel=bar
> +      toplevel2=baz

* Removed the second toplevel thing here so the test doesn't depend on
attribute set ordering which is stable de facto, but I'm not sure it
will be forever (probably…).

> +      [foo]
> +      bar=baz
> +    '';
> +  };
> +
>    /* right now only invocation check */
>    testToJSONSimple =
>      let val = {
> diff --git a/pkgs/pkgs-lib/formats.nix b/pkgs/pkgs-lib/formats.nix
> index 14589f8ecdc..b944c37bd59 100644
> --- a/pkgs/pkgs-lib/formats.nix
> +++ b/pkgs/pkgs-lib/formats.nix
> @@ -77,7 +77,7 @@ rec {
>          else
>            singleIniAtom;
>  
> -    in attrsOf (attrsOf iniAtom);
> +    in attrsOf (either iniAtom (attrsOf iniAtom));

* Added a description for this type to make it a bit more readable in
the manual.

>  
>      generate = name: value: pkgs.writeText name (lib.generators.toINI args value);

Thanks a lot for saving me the work of porting it!


Cheers,

Lukas
Details
Message ID
<660bcac0-3456-e52e-a41f-0eb84b487edb@systemli.org>
In-Reply-To
<20210409155009.3663-1-ben@bsima.me> (view parent)
DKIM signature
missing
Download raw message

On 4/9/21 5:50 PM, Ben Sima wrote:
> ---
>  nixos/modules/module-list.nix                 |  1 +
>  .../modules/services/web-servers/gmnisrv.nix  | 57 +++++++++++++++++++
>  2 files changed, 58 insertions(+)
>  create mode 100644 nixos/modules/services/web-servers/gmnisrv.nix
> 
> diff --git a/nixos/modules/module-list.nix b/nixos/modules/module-list.nix
> index 509bccb1ec7..75de48e557c 100644
> --- a/nixos/modules/module-list.nix
> +++ b/nixos/modules/module-list.nix
> @@ -944,6 +944,7 @@
>    ./services/web-servers/caddy.nix
>    ./services/web-servers/darkhttpd.nix
>    ./services/web-servers/fcgiwrap.nix
> +  ./services/web-servers/gmnisrv.nix
>    ./services/web-servers/hitch/default.nix
>    ./services/web-servers/hydron.nix
>    ./services/web-servers/jboss/default.nix
> diff --git a/nixos/modules/services/web-servers/gmnisrv.nix b/nixos/modules/services/web-servers/gmnisrv.nix
> new file mode 100644
> index 00000000000..eb4f99c3772
> --- /dev/null
> +++ b/nixos/modules/services/web-servers/gmnisrv.nix
> @@ -0,0 +1,57 @@
> +{ lib
> +, options
> +, config
> +, pkgs
> +, modulesPath

Insert a `, ...` here, in case downstream users pass extra arguments to
the module.

> +}:
> +
> +let
> +  cfg = config.services.gmnisrv;
> +  cfgFormat = pkgs.formats.ini { };
> +in {
> +  options.services.gmnisrv = {
> +    enable = lib.mkEnableOption "Enable the gmnisrv service";
> +    settings = lib.mkOption {
> +      type = cfgFormat.type;
> +      description = ''
> +        Configuration for gmnisrv. See gmnisrv.ini(5) for supported settings.
> +      '';
> +    };
> +    package = lib.mkOption {
> +      type = lib.types.package;
> +      default = pkgs.gmnisrv;
> +      description = "gmnisrv package to use";
> +    };
> +    listen = lib.mkOption {
> +      type = lib.types.str;
> +      default = "0.0.0.0:1965 [::]:1965";
> +      description = ''
> +        Host address and port to listen on. See gmnisrv.ini(5) for syntax
> +        details.
> +      '';
> +    };

Since we have top level INI keys now, you can remove this altogether in
favor of services.gmnisrv.settings.listen. You can add documentation to
values under settings via the freeformType as well.

> +    dataDir = lib.mkOption {
> +      type = lib.types.str;
> +      default = "/var/lib/gemini";
> +      description = "Where gmnisrv should store certs and other data.";
> +    };
> +  };
> +  config = lib.mkIf cfg.enable {
> +    services.gmnisrv.settings = {
> +      "listen" = cfg.listen;

Instead of this, use

  cfgFormat.generate "gmnisrv.ini" ({
    "listen" = cfg.listen;
  } // cfg.settings);

Should save on eval time as well, not having to merge the modules' settings.

> +      ":tls" = {
> +        "store" = lib.mkDefault "${cfg.dataDir}/certs";

Is this a value that always needs to be set? Does it require user
intervention to generate the certs?

Overall it'd be preferrable to have

  default = {
    ":tls" = {

    "store" = lib.mkDefault "${cfg.dataDir}/certs";
  };

for services.gmnisrv.settings over this which would mean that the
service works out of the box

> +      };
> +    };
> +    environment.etc."gmnisrv.ini" = {
> +      enable = true;
> +      source = cfgFormat.generate "gmnisrv.ini" cfg.settings;
> +    };

Overall I'd say it is preferred in nixpkgs to pass the config file as a
derivation directory to the binary instead of generating some global
state outside of the nix store (albeit declaratively). The previous
solution without environment.etc was better.

> +    systemd.services.gmnisrv = {
> +      description = "gmnisrv service";
> +      wantedBy = [ "multi-user.target" ];
> +      after = [ "network-online.target" ];

Isn't this network.target normally? Could be wrong though.

> +      script = "${cfg.package}/bin/gmnisrv -C /etc/gmnisrv.ini";
> +    };
> +  };
> +}
> 

Re: [PATCH v2 2/2] generators: support toplevel keys in toINI

William Casarin <jb55@jb55.com>
Details
Message ID
<20210412165353.42wzthkb2f3r2n5o@monad>
In-Reply-To
<ed765483-893d-fe26-339e-b8c06660e659@systemli.org> (view parent)
DKIM signature
missing
Download raw message
On Fri, Apr 09, 2021 at 10:08:54PM +0200, sternenseemann wrote:
>Now live at https://github.com/NixOS/nixpkgs/pull/118925!

are we blocked on this? is there another way to get the gmnisrv module into
nixpkgs in the meantimne?

Re: [PATCH v2 2/2] generators: support toplevel keys in toINI

Details
Message ID
<87mtu25q9l.fsf@bsima.me>
In-Reply-To
<20210412165353.42wzthkb2f3r2n5o@monad> (view parent)
DKIM signature
missing
Download raw message
William Casarin <jb55@jb55.com> writes:

> are we blocked on this? is there another way to get the gmnisrv module into
> nixpkgs in the meantimne?

We could just copy the relevant generator functions into the module as
local let bindings. I'll try to get a v3 patch for this together soon
this week so we at least have it as an option.
Details
Message ID
<87fszp6aek.fsf@bsima.me>
In-Reply-To
<660bcac0-3456-e52e-a41f-0eb84b487edb@systemli.org> (view parent)
DKIM signature
missing
Download raw message
sternenseemann <sternenseemann@systemli.org> writes:

> On 4/9/21 5:50 PM, Ben Sima wrote:
>> +      ":tls" = {
>> +        "store" = lib.mkDefault "${cfg.dataDir}/certs";
>
> Is this a value that always needs to be set? Does it require user
> intervention to generate the certs?
>
> Overall it'd be preferrable to have
>
>   default = {
>     ":tls" = {
>
>     "store" = lib.mkDefault "${cfg.dataDir}/certs";
>   };
>
> for services.gmnisrv.settings over this which would mean that the
> service works out of the box

Yes it always need to be set, but the user doesn't need to do anything,
the certs are generated automatically.

I'm making this change and the other changes mentioned in the v3 patch.
Reply to thread Export thread (mbox)