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
Add spotifyd package and service #65092
Conversation
881a73f
to
b823708
Compare
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.
I've included some suggestions I hope will be helpful. Please let me know if you have any questions.
@anderslundstedt LGTM 👍 Please squash all but the first commit into one and then from my perspective we're good to merge. Thanks! |
Great! Thanks everyone for the swift review. Should we not let the package and the module be introduced by separate commits? I kept it like that for now, but let me know if I should squash them together. |
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.
👍 LGTM
Thanks! |
Motivation for this change
Spotifyd is a nice spotify connect server. This adds a spotifyd package and a spotifyd service.
I took over the abandoned #55923, addressing the comments to the best of my ability. I am not very experienced with adding packages or services to NixOS, so likely there are still things to address.
Additional comments
Here are the differences between this pull request and #55923:
In the package:
Version bumped to 0.2.11, which also eliminates the need for the
noavxcrypto.patch
patch.platforms.unix
instead ofplatforms.all
.In the service module:
As suggested by a comment, removed the
dataDir
option. As far as I can see, this option does indeed seem unnecessary.As suggested by a comment, use command line option
--cache_path /var/lib/spotifyd
instead of settingEnvironment = "HOME=/var/lib/spotifyd"
.Added
spotifyd
user belonging to theaudio
group. (Did not work for me without this.)Unresolved #55923 comments not addressed by the above changes:
Using
lib.generators.toINI
to generate the config line from theconfig
service option. I did not feel I had the competence to make such a change (and it is not clear to me why such a change would be preferable).Dropping the
--no-daemon
from the service start. I tried this which resulted in the service crashing.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)