dnscrypt-proxy2: 2.0.15 -> 2.0.17, add NixOS module #48289

lukateras · 2018-10-12T21:41:11Z

Motivation for this change

Resolves #43298.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
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 nox --run "nox-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)
Fits CONTRIBUTING.md.

Mic92 · 2018-10-12T21:55:34Z

nixos/modules/services/networking/dnscrypt-proxy2.nix

+  json = pkgs.writeText "dnscrypt-proxy.json" (builtins.toJSON cfg.config);
+
+  toml = pkgs.runCommand "dnscrypt-proxy.toml" {} ''
+    ${pkgs.json-to-toml}/bin/json-to-toml < ${json} > $out


I think it make sense to just re-use remarshal instead. It does the same thing and if we agree on one tool, it can be re-used for different services. See also in the telegraf, influxdb and gitlab-runner module.

I know about remarshal, but concerned about build time and closure size. One of my targets for this service is a 1GHz single-core armv7l.

If I add more conversion types to that tool, will it perhaps be more appropriate?

I guess so. Both implementations are not terrible complex.

Btw. I can recommend nixops for you use case independent from your pr. Our nix builder also handles armv7l for that matter.

It would be nice if the output is pretty-printed with indentation to make it more readable.

GrahamcOfBorg · 2018-10-12T21:56:16Z

Success on aarch64-linux (full log)

Attempted: dnscrypt-proxy2, json-to-toml

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin
shrinking /nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin/bin/dnscrypt-proxy
strip is /nix/store/428gs2z4b8f9byvghzlpbjwjb3a7jwxx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin/bin
patching script interpreter paths in /nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin
checking for references to /build in /nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin...
strip is /nix/store/428gs2z4b8f9byvghzlpbjwjb3a7jwxx-binutils-2.30/bin/strip
/nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin
/nix/store/a7cr7acjqi27h8bhcb98iv9rmn9pvkjk-json-to-toml-20181012.210909-bin

GrahamcOfBorg · 2018-10-12T21:56:28Z

Success on x86_64-darwin (full log)

Attempted: dnscrypt-proxy2, json-to-toml

Partial log (click to expand)

find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
find: '/nix/store/4zlc83ihkqwyy3ccrznnqc7c7sqin0n2-dnscrypt-proxy-2.0.17': No such file or directory
/nix/store/56ih47j8avylwqm9h09s57j37ygj0wv9-dnscrypt-proxy-2.0.17-bin
/nix/store/n69h5q9m1n5g3h24jncsm2zfjn5qw7zl-json-to-toml-20181012.210909-bin

GrahamcOfBorg · 2018-10-12T21:56:40Z

Success on x86_64-linux (full log)

Attempted: dnscrypt-proxy2, json-to-toml

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin
shrinking /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin/bin/dnscrypt-proxy
strip is /nix/store/dxf1m7dhc4qb655bdljc1fsd74v1nag3-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin/bin
patching script interpreter paths in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin
checking for references to /build in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin...
strip is /nix/store/dxf1m7dhc4qb655bdljc1fsd74v1nag3-binutils-2.30/bin/strip
/nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin
/nix/store/l595f8fqv7bpln34f8qvc0w16bw8r6cb-json-to-toml-20181012.210909-bin

Mic92 · 2018-10-12T21:56:47Z

nixos/modules/services/networking/dnscrypt-proxy2.nix

+{ config, lib, pkgs, ... }: with lib;
+
+let
+  cfg = config.services.dnscrypt-proxy2;


I think we should deprecate dnscrypt-proxy v1 for the next release. It is no longer maintained afaik.

Agreed. v1 should be removed for 19.03

joachifm · 2018-10-12T22:49:12Z

LGTM

GrahamcOfBorg · 2018-10-13T01:31:49Z

Success on x86_64-darwin (full log)

Attempted: dnscrypt-proxy2

Partial log (click to expand)

/nix/store/56ih47j8avylwqm9h09s57j37ygj0wv9-dnscrypt-proxy-2.0.17-bin

GrahamcOfBorg · 2018-10-13T01:31:53Z

Success on aarch64-linux (full log)

Attempted: dnscrypt-proxy2

Partial log (click to expand)

/nix/store/xgzy551z9h9ray6pwm2lai81fagqvwk9-dnscrypt-proxy-2.0.17-bin

GrahamcOfBorg · 2018-10-13T01:33:22Z

Success on x86_64-linux (full log)

Attempted: dnscrypt-proxy2

Partial log (click to expand)

installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin
shrinking /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin/bin/dnscrypt-proxy
strip is /nix/store/dxf1m7dhc4qb655bdljc1fsd74v1nag3-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin/bin
patching script interpreter paths in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin
checking for references to /build in /nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin...
strip is /nix/store/dxf1m7dhc4qb655bdljc1fsd74v1nag3-binutils-2.30/bin/strip
/nix/store/p320bl3vql0lpngprisvl8rzqdpryyjb-dnscrypt-proxy-2.0.17-bin

Mic92 · 2018-10-13T08:12:12Z

pkgs/tools/networking/dnscrypt-proxy/2.x/default.nix

-    repo = "dnscrypt-proxy";
-    rev = "${version}";
-    sha256 = "0iwvndk1h550zmwhwablb0smv9n2l51hqbmzj354mcgi6frnx819";
+  src = fetchzip {


How is this different from fetchFromGitHub btw?

Not much, wanted to reuse goPackagePath.

Mic92 · 2018-10-13T09:04:02Z

nixos/modules/services/networking/dnscrypt-proxy2.nix

+      '';
+      example = literalExample ''
+        {
+          sources.public-resolvers = {


This could become the default value, no?

Mic92 · 2018-10-13T09:08:08Z

I currently see the the following errors on startup:

Oct 13 10:05:18 turingmachine dnscrypt-proxy[17383]: [2018-10-13 10:05:18] [WARNING] /nix/store/public-resolvers.md: open sf-tvatsszk7d7bpj5c.tmp: read-only file system
Oct 13 10:05:18 turingmachine dnscrypt-proxy[17383]: [2018-10-13 10:05:18] [WARNING] /nix/store/public-resolvers.md.minisig: open sf-avhbndvzjjmf2siu.tmp: read-only file system

Can dnscrypt-proxy be configured to store its data in /var/lib/dnscrypt for example?

Shados · 2018-10-13T15:29:42Z

@Mic92 absolutely. Currently:

The default cache_file here being a relative path means it is placed in the working directory of the dnscrypt-proxy process.
The service has DynamicUser enabled:
- No HOME has been specified for the dynamic user, so it defaults to /
- DynamicUser also implies ProtectSystem=strict, which means / (among other things) are mounted read-only for the process

So should just need to set WorkingDirectory somewhere appropriate... maybe something like:

{
  CacheDirectory = "dnscrypt";
  WorkingDirectory = "/var/cache/dnscrypt";
}

With DynamicUser, this should actually end up under /var/cache/private/dnscrypt.

Otherwise, looks OK. Personally I was taking the route of more manually constructing the configuration file from well-typed options, as I like the benefits that offers:

Can provide errors or warnings about configuration issues at evaluation time rather than at run time.
Potentially insulate against change in the underlying program's configuration file format / syntax / semantics.
Makes both the module and the underlying daemon/service more-or-less self-documenting; you can often figure out how to use services with modules written this way wholey by reading man configuration.nix.

But it's also a lot more work to implement, and presents some cognitive overhead to people already familiar with configuring the service directly.

I will most likely get around to finishing a version of the module done that way, but at the current rate it won't be for a couple of months (too many irons in the fire -- and too many fires, for that matter). I'm definitely happy to have something working in nixpkgs before then 👍.

joachifm · 2018-10-21T15:43:39Z

nixos/modules/services/networking/dnscrypt-proxy2.nix

+      type = types.attrs;
+    };
+
+    package = mkOption {


What is the rationale for making this configurable?

joachifm · 2018-10-23T14:46:19Z

I'd be happy to merge this as if, if not for the package option. I really dislike those ... Anybody have a usecase for it in this case?

lukateras · 2018-10-23T15:00:32Z

On the other hand, I think that all service modules should have package options. This significantly reduces coupling between Nixpkgs and NixOS (this is a kind of dependency injection), between implementation and interface.

Primary use case is freezing specific version in your overlay with a different attr name. Without package option, you're stuck with shadowing the original package, which might cause unnecessary rebuilds if something else depends on that same package.

Secondary use case is that it allows to maintain multiple versions/flavors of the package in Nixpkgs while having a single NixOS module. Assumption that it is not the case doesn't hold over time.

joachifm · 2018-10-23T16:10:48Z

My view is that options should be judged on their merit on a module-to-module basis. I fail to see how the general rationale for package options pertains to dnscrypt-proxy.

lukateras · 2018-10-23T16:26:28Z

I don't understand why it has to be judged on module-to-module basis. It has a few general benefits that I've listed above. Deciding on case-by-case basis will inevitably cause friction downstream, because we won't be able to predict all use cases, and the option itself is trivial and adds three lines of Nix code.

pkgs/tools/networking/dnscrypt-proxy/2.x/default.nix

dtzWill · 2019-03-21T22:22:14Z

Don't suppose updates such as in recent PR's (see above) resolve the outstanding issues? If any?

rnhmjoj · 2019-03-27T18:14:47Z

Can this make it into 19.03 or is it too late already?
Btw, I'm with @yegortimoshenko on the package option.

- Remove package option, which seems to have been the only blocker to merging NixOS#48289. - Add configFile option as an alternative to translating toml/nix.

mmahut · 2020-01-24T22:50:23Z

Any updates?

worldofpeace · 2020-01-26T23:29:18Z

There's new PRs inheriting this work.

dnscrypt-proxy2: 2.0.15 -> 2.0.17

7606c28

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) labels Oct 12, 2018

Mic92 reviewed Oct 12, 2018

View reviewed changes

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Oct 12, 2018

Mic92 reviewed Oct 12, 2018

View reviewed changes

nixos/dnscrypt-proxy2: init

1864f6b

Mic92 reviewed Oct 13, 2018

View reviewed changes

joachifm reviewed Oct 21, 2018

View reviewed changes

lukateras mentioned this pull request Oct 29, 2018

nixos/darkhttpd: module to enable darkhttpd #48036

Merged

9 tasks

worldofpeace reviewed Nov 11, 2018

View reviewed changes

pkgs/tools/networking/dnscrypt-proxy/2.x/default.nix Show resolved Hide resolved

infinisil mentioned this pull request Nov 16, 2018

NixOS services: When to have a package option #50476

Closed

dtzWill mentioned this pull request Mar 21, 2019

dnscrypt-proxy2: 2.0.15 -> 2.0.21 #57949

Merged

10 tasks

ryneeverett mentioned this pull request Nov 5, 2019

dnscrypt-proxy2 service: init #72815

Closed

10 tasks

worldofpeace closed this Jan 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dnscrypt-proxy2: 2.0.15 -> 2.0.17, add NixOS module #48289

dnscrypt-proxy2: 2.0.15 -> 2.0.17, add NixOS module #48289

lukateras commented Oct 12, 2018

Mic92 Oct 12, 2018 •

edited

lukateras Oct 12, 2018

Mic92 Oct 12, 2018

Mic92 Oct 12, 2018

Mic92 Oct 12, 2018

GrahamcOfBorg commented Oct 12, 2018

GrahamcOfBorg commented Oct 12, 2018

GrahamcOfBorg commented Oct 12, 2018

Mic92 Oct 12, 2018

c0bw3b Oct 21, 2018

joachifm commented Oct 12, 2018

GrahamcOfBorg commented Oct 13, 2018

GrahamcOfBorg commented Oct 13, 2018

GrahamcOfBorg commented Oct 13, 2018

Mic92 Oct 13, 2018

lukateras Oct 13, 2018

Mic92 Oct 13, 2018 •

edited

Mic92 commented Oct 13, 2018

Shados commented Oct 13, 2018

joachifm Oct 21, 2018

joachifm commented Oct 23, 2018

lukateras commented Oct 23, 2018 •

edited

joachifm commented Oct 23, 2018

lukateras commented Oct 23, 2018

dtzWill commented Mar 21, 2019

rnhmjoj commented Mar 27, 2019

mmahut commented Jan 24, 2020

worldofpeace commented Jan 26, 2020

dnscrypt-proxy2: 2.0.15 -> 2.0.17, add NixOS module #48289

dnscrypt-proxy2: 2.0.15 -> 2.0.17, add NixOS module #48289

Conversation

lukateras commented Oct 12, 2018

Motivation for this change

Things done

Mic92 Oct 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GrahamcOfBorg commented Oct 12, 2018

GrahamcOfBorg commented Oct 12, 2018

GrahamcOfBorg commented Oct 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joachifm commented Oct 12, 2018

GrahamcOfBorg commented Oct 13, 2018

GrahamcOfBorg commented Oct 13, 2018

GrahamcOfBorg commented Oct 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Oct 13, 2018 • edited

Choose a reason for hiding this comment

Mic92 commented Oct 13, 2018

Shados commented Oct 13, 2018

Choose a reason for hiding this comment

joachifm commented Oct 23, 2018

lukateras commented Oct 23, 2018 • edited

joachifm commented Oct 23, 2018

lukateras commented Oct 23, 2018

dtzWill commented Mar 21, 2019

rnhmjoj commented Mar 27, 2019

mmahut commented Jan 24, 2020

worldofpeace commented Jan 26, 2020

Mic92 Oct 12, 2018 •

edited

Mic92 Oct 13, 2018 •

edited

lukateras commented Oct 23, 2018 •

edited