Rfc htpdate #18818
Rfc htpdate #18818
Conversation
|
@Jookia, thanks for your PR! By analyzing the annotation information on this pull request, we identified @edolstra, @bjornfor and @offlinehacker to be potential reviewers |
|
(Oops, forgot to fix the title before posting. Sorry!) |
| Type = "forking"; | ||
| PIDFile = "/var/run/htpdate.pid"; | ||
| ExecStart = concatStringsSep " " [ | ||
| "${htpdate}/bin/htpdate" |
There was a problem hiding this comment.
Nix has multiline-strings via ''
There was a problem hiding this comment.
Multiline strings don't work for this, the alternate way is one very lone line.
There was a problem hiding this comment.
@Jookia Something like this should work:
ExecStart = ''
blah \
foo \
bar
''
| config = mkIf cfg.enable { | ||
|
|
||
| # Make tools such as htpdate available in the system path | ||
| environment.systemPackages = [ htpdate ]; |
There was a problem hiding this comment.
Is it necessary to make the package available to the user? It's generally better to put as little as possible there. Of course it's justified if the tool is expected to be called by the user (for example to control the daemon)
| meta = { | ||
| description = "Utility to fetch time and set the system clock over HTTP"; | ||
| homepage = http://www.vervest.org/htp/; | ||
| platforms = stdenv.lib.platforms.all; |
There was a problem hiding this comment.
The build fails on darwin because it calls out to gcc. You could try making it use cc instead, assuming the package doesn't actually depend on gcc specifics.
There was a problem hiding this comment.
Fixed, if you're on Darwin could you try building?
There was a problem hiding this comment.
Nevermind, Travis CI concludes that there's no sys/timex header which OS X doesn't have. Setting to just linux.
LnL7
added
8.has: package (new)
| }; | ||
|
|
||
| extraOptions = mkOption { | ||
| type = types.string; |
There was a problem hiding this comment.
types.str is more appropriate for plain strings, string has potentially surprising merge semantics and is deprecated.
There was a problem hiding this comment.
Changed them all to types.str.
|
Thank you |
Motivation for this change
This PR adds the htpdate utility as well as a NixOS service to run it as a daemon.
Things done
nix-shell -p nox --run "nox-review wip"./result/bin/)