Skip to content
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

mpd-mpris: init at 0.2.3 #65340

Merged
merged 1 commit into from Jul 28, 2019
Merged

Conversation

doronbehar
Copy link
Contributor

Motivation for this change
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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

One thing I'm not sure of:

I've add a postInstall script that copies the upstream provided user systemd service to the appropriate location in $out. I've installed the package and the service file appears in ~/.nix-profile/lib/systemd/user along with all other user installed packages' user systemd services. However, systemctl --user enable mpd-mpris.service fails with:

Failed to enable unit: Unit file mpd-mpris.service does not exist.

Maybe that's not such a big deal as one can manually link ~/.nix-profile/lib/systemd/user/mpd-mpris.service to ~/.config/systemd/user/default.target.wants/ but I'm pretty sure that's something other packages have handled somehow.

I couldn't find any mention of something to be done in such a case in the manual..

pkgs/tools/audio/mpd-mpris/default.nix Outdated Show resolved Hide resolved
substituteInPlace mpd-mpris.service \
--replace /usr/bin $out/bin
mkdir -p $out/lib/systemd/user
cp mpd-mpris.service $out/lib/systemd/user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a NixOS module instead? cc @samueldr @cleverca22 @infinisil.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah this is fine, NixOS supports service files in packages with systemd.packages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@infinisil could you explain why I'm not able to enable the systemd service with systemctl enable when the package is installed to my user environment? I've wrote about it in my first comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's a user service, missed that. I don't think I've ever seen those being used as you described with nix-env, so maybe nobody ever thought of making that work.

It also doesn't seem like home-manager supports running service files from packages, but that's definitely a place where you could add support for that without much trouble.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are saying in general that systemd's user services search path just doesn't include ~/.nix-profile/lib/systemd/user/? That's rather unfortunate.. In any case, I guess it would be find to leave it as ease, unless you wish to add a user module service like services.keybase for example..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't researched a lot, it might end up being pretty easy to add that to systemd's paths

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL after reading this that if I ln -s ~/.nix-profile/lib/systemd ~/.local/share/systemd, systemctl enable works.. However, trying out the following:

export SYSTEMD_UNIT_PATH="$HOME/.nix-profile/lib/systemd/user:"
systemctl --user enable mpd-mpris

Without the symlink, didn't work..

@kalbasit
Copy link
Member

@GrahamcOfBorg build mpd-mpris

@ofborg ofborg bot requested a review from kalbasit July 28, 2019 06:27
@kalbasit kalbasit merged commit a3ae2db into NixOS:master Jul 28, 2019
@doronbehar doronbehar deleted the package-mpd-mpris branch August 22, 2019 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants