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

nixos/display-managers: fix loading of module-x11-publish #43610

Merged
merged 1 commit into from Aug 3, 2018

Conversation

jfrankenau
Copy link
Member

Motivation for this change

Fix #11970

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.

${config.hardware.pulseaudio.package.out}/bin/pactl load-module module-x11-publish "display=$DISPLAY"
# Publish access credentials in the root window if supported by PulseAudio.
${optionalString (config.hardware.pulseaudio.package == pkgs.pulseaudioFull)
"${config.hardware.pulseaudio.package.out}/bin/pactl load-module module-x11-publish 'display=$DISPLAY'"
Copy link
Member

Choose a reason for hiding this comment

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

I think using || true is a better solution, because this here won't work when the user has an even slighly modified pulseaudio, because then the derivations won't match.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is a better way. Thank you.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, wait. This doesn't get rid of the error message. What about using pulseaudio --dump-modules to check if module-x11-publish is available?

@jfrankenau jfrankenau force-pushed the fix-pulse-module-x11-publish branch 2 times, most recently from 932c1d9 to 900957c Compare August 3, 2018 10:21
module-x11-publish is only provided by the pulseaudioFull package.
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Ah very neat, thanks!

@infinisil infinisil merged commit fcb4254 into NixOS:master Aug 3, 2018
@jfrankenau jfrankenau deleted the fix-pulse-module-x11-publish branch August 3, 2018 15:21
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