Skip to content

nixos/botamusique: init#121626

Merged
mweinelt merged 5 commits intoNixOS:masterfrom
mweinelt:botamusique
May 23, 2021
Merged

nixos/botamusique: init#121626
mweinelt merged 5 commits intoNixOS:masterfrom
mweinelt:botamusique

Conversation

@mweinelt
Copy link
Member

@mweinelt mweinelt commented May 3, 2021

Motivation for this change

Proper setup of botamusique, a mumble music bot.

I did look into certificate creation, the upstreams example looks like this:

openssl req -x509 -nodes -days 3650 -newkey rsa:2048 -keyout botamusique.pem -out botamusique.pem -subj "/CN=botamusique"

Something users could do themselves, or we could handle it in ExecStartPre, not sure.

I did not look into what giving the bot tokens would do, I guess allow them to register?

Both could be exposed at the top-level, certificate is actually a path, tokens are literal strings. Nothing really security-critical ig.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@mweinelt mweinelt requested a review from infinisil May 3, 2021 21:28
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 3, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels May 3, 2021
@mweinelt
Copy link
Member Author

mweinelt commented May 11, 2021

Added a nixos test and configured it as a passthru test. I think feature-wise this is as far as I would go in this iteration. No certificate handling.

@mweinelt mweinelt force-pushed the botamusique branch 2 times, most recently from 7c93558 to 7755b85 Compare May 11, 2021 21:28
@mweinelt
Copy link
Member Author

And pushed an update for botamusique that allows setting the bandwidth to something more reasonable than 200 kbit/s.

@mweinelt mweinelt added this to the 21.05 milestone May 11, 2021
@ofborg ofborg bot requested a review from infinisil May 11, 2021 23:28
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels May 11, 2021
@mweinelt mweinelt requested review from FRidh and jonringer as code owners May 17, 2021 22:09
@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label May 17, 2021
@ofborg ofborg bot requested a review from thelegy May 17, 2021 22:19
@ofborg ofborg bot removed the 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. label May 17, 2021
@mweinelt mweinelt force-pushed the botamusique branch 2 times, most recently from 34a7487 to 96b9635 Compare May 22, 2021 15:57
@mweinelt
Copy link
Member Author

mweinelt commented May 22, 2021

Updated to the latest version, that disables fetching the changelog, when autoupdates are disabled. This allows me to drop the patch I applied to the package in the nixos test.

Test currently doesn't complete, due to murmur being unable to find grpc.
Fixed in #124030.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind expanding this description, usually nice to give a concise description of the package and what enabling the service does

Suggested change
enable = mkEnableOption "botamusique";
enable = mkEnableOption "Enable botamusique service to play youtube or sound on mumble.";

Copy link
Member

Choose a reason for hiding this comment

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

That is redundant: https://github.com/NixOS/nixpkgs/blob/master/lib/options.nix#L92-L99

Usually you just pass the service name to mkEnableOption

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh didn't realize that it constructed the description. sorry!

@mweinelt mweinelt merged commit 84f649f into NixOS:master May 23, 2021
@mweinelt mweinelt deleted the botamusique branch May 23, 2021 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants