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
nixos: added navidrome service #91366
Conversation
"d '${cfg.musicFolder}' '${cfg.musicFolderPermissions}' '${cfg.user}' '${cfg.group}' - -" | ||
]; | ||
|
||
systemd.services.navidrome = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a good idea to reuse the upstream's systemd unit.
You should modify the package to install navidrome.service
in $out/lib/systemd/system/
, then to use the unit you can do
systemd.packages = [ pkgs.navidrome ];
in the NixOS configuration. Now you only need to set the properties like environment
and serviceConfig.User
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A number of these options seem like they wouldn't often be used. There is a configuration file format (toml
) as well. Maybe you could create a settings
option to represent the configuration file which could replace a few of the options you have included in this module. See NixOS/rfcs#42 for rationale and some linked examples if you're interested.
If you're interested and need a hand ping me 👍
@rnhmjoj I understand your idea and approve it, however at the moment the package is built downloading the compiled binary, so I can't copy the systemd config file from the repository.
@aanderse So do you suggest to fold all the settings inside a single Moreover, I was thinking to update the navidrome package too, can I simply add a commit to this pull request or I need to create a new one? |
Yes, it's ok. In general it's better to lower the maintenance cost by using upstream-provided configurations but it's not a hard requirement.
Yes, in fact this is one of the reasons for the push of |
@aanderse I'm trying your approach but I don't understand if the RFC you've cited is implemented. The examples in your link don't work.
Then I overwrite
Then in I could put a custom
I'll push my new commit so you can read the code, obviously it doesn't work but it's to make you understand better if what I wrote is not enough. @rnhmjoj Thank you for your answer, all clear. Obviously if you want to answer my last questions you're welcome too. |
@aciceri I hate to make suggestions and not follow through, I thought it was going to be pretty straight forward in this case. I guess I was wrong. I'm a bit tapped out for time at the moment... I'm really hoping @infinisil will be kind enough to donate some of his time and guidance here. Pretty please with a 🍒 on top? |
Hey @aciceri, Navidrome also supports JSON and YML config files, if it makes it easier for you. You just have to pass it as a parameter |
It has not been used since 2015: mozilla/gecko-dev@42c9261
It is only optionally dlopened by crash reporter to try to get system proxy settings but no system stores proxy in GConf these days so it is completely useless. https://github.com/mozilla/gecko-dev/blob/97c590ed5532a7b382b5b8500c109de8f1d831e3/toolkit/crashreporter/client/crashreporter_gtk_common.cpp#L121-L122
It is deprecated and has been removed ages ago: https://bugzilla.mozilla.org/show_bug.cgi?id=694570 It persists in the crash reporter for some reason but it is optional there: https://github.com/mozilla/gecko-dev/blob/f66f5a235e7d74c29b951316f73001126a056734/toolkit/crashreporter/client/crashreporter_linux.cpp#L366-L370
It has not been used for ages: https://bugzilla.mozilla.org/show_bug.cgi?id=687766
It has not been used for ages: https://bugzilla.mozilla.org/show_bug.cgi?id=687766
It is deprecated and has been removed ages ago: https://bugzilla.mozilla.org/show_bug.cgi?id=694570 It persists in the crash reporter for some reason but it is optional there: https://github.com/mozilla/gecko-dev/blob/f66f5a235e7d74c29b951316f73001126a056734/toolkit/crashreporter/client/crashreporter_linux.cpp#L366-L370
It is only optionally dlopened by crash reporter to try to get system proxy settings but no system stores proxy in GConf these days so it is completely useless. https://github.com/mozilla/gecko-dev/blob/97c590ed5532a7b382b5b8500c109de8f1d831e3/toolkit/crashreporter/client/crashreporter_gtk_common.cpp#L121-L122
First working commit Ops, was forgotting the module list Proof of concept, not working, code provided only to ask for help last
Every time I try to |
Motivation for this change
I recently added the navidrome support with #88928, now should be nice to have the corresponding module service.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)I was inspired by the
airsonic
and thetransmission
modules, I tested it and it works great.However since this is my first NixOS module I'm sure that something needs to be improved, even for for this reasons the commits name aren't nice.
When someone will say that's fine I'll do
git rebase
to squash the commits into a single one with the correct message.@rnhmjoj Since you have merged the
navidrome
package, maybe you are the most suitable for a review here. Thank you.