Skip to content

avahi: set service directory and refactor module#61945

Closed
WilliButz wants to merge 4 commits intoNixOS:staging-nextfrom
WilliButz:avahi-refactor
Closed

avahi: set service directory and refactor module#61945
WilliButz wants to merge 4 commits intoNixOS:staging-nextfrom
WilliButz:avahi-refactor

Conversation

@WilliButz
Copy link
Member

Motivation for this change

Before this PR avahi used to look for service definitions in etc/avahi/services in its store path.
First this leads to the situation mentioned in #60939 and it also makes adding custom services quite user-unfriendly, because they would have to be added to the build output of avahi.

Things done
Package:

For the build AVAHI_SERVICE_DIR is now set to /etc/avahi/services.

Module:

Types are now specified for all options.
The fixed uid and gid for the avahi user have been removed
and the avahi user is now in the avahi group.
The the generic opening of the firewall for UDP port 5353 is
now optional, but still defaults to true.

The option extraServiceFiles was added to specify avahi
service definitions, which are then placed in /etc/avahi/services.

Manual:

Added a section mentioning the changes for avahi.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS (x86_64-linux & aarch64-linux
    • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

fixes #60939

WilliButz added 4 commits May 22, 2019 22:29
Avahi now uses `/etc/avahi/services` instead of its
store path to look for files with service definitions.
Types are now specified for all options.
The fixed uid and gid for the avahi user have been removed
and the user avahi is now in the group avahi.
The the generic opening of the firewall for UDP port 5353 is
now optional, but still defaults to true.

The option `extraServiceFiles` was added to specify avahi
service definitions, which are then placed in `/etc/avahi/services`.
@WilliButz
Copy link
Member Author

@GrahamcOfBorg test avahi

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels May 23, 2019
@ofborg ofborg bot requested a review from lovek323 May 23, 2019 13:20
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels May 23, 2019
@infinisil
Copy link
Member

New commits should go to the staging branch, not staging-next, see https://github.com/NixOS/rfcs/blob/master/rfcs/0026-staging-workflow.md#detailed-design.

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.

(Hiding whitespace changes highly recommended for looking at the changes in this PR)

hostName = mkOption {
type = types.str;
default = config.networking.hostName;
defaultText = "config.networking.hostName";
Copy link
Member

Choose a reason for hiding this comment

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

You can use literalExample "config.networking.hostName" in order for it not to render as a string, but the literal expression in the manual.

Copy link
Member Author

Choose a reason for hiding this comment

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

@infinisil isn't literalExample just for examples? I'm setting the defaultText for the option here because I want to avoid having the manual rebuilt for changing hostnames. I'm not sure how you meant that :)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I also thought literalExamples was just for examples, and clearly that would be intended from its name. But upon checking, it seems to work for defaultText as well, and makes the manual look more correct with this. Feel free to add or not

'';
};
extraServiceFiles = mkOption {
type = types.attrsOf types.str;
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 with types; attrsOf (either path str) would fit better. Will also show in the docs then.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used this prior to now but removed it, because I thought I wouldn't get any advantage from it, because there is no real difference between the types. But having it in the manual sounds like a good reason 👍

'';
description = ''
Specify custom service definitions which are placed in the avahi service directory.
See the avahi.service(5) manpage for detailed information.
Copy link
Member

Choose a reason for hiding this comment

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

Can also use <citerefentry><refentrytitle>avahi.service</refentrytitle><manvolnum>5</manvolnum></citerefentry> here

# Make NSS modules visible so that `avahi_nss_support ()' can
# return a sensible value.
export LD_LIBRARY_PATH="${config.system.nssModules.path}"
preStart = "mkdir -p /run/avahi-daemon";
Copy link
Member

Choose a reason for hiding this comment

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

Since you're refactoring this already, I suggest making this

{
  systemd.tmpfiles.rules = [ "d /run/avahi-daemon - avahi avahi -" ];
}

(untested)

@WilliButz
Copy link
Member Author

@infinisil I'll close this PR and open a new one with the proposed changes, to avoid spamming everyone with code-owner messages due to the diff between staging and staging-next. :)

@WilliButz WilliButz closed this May 28, 2019
@infinisil
Copy link
Member

I found out if you're fast enough and with some luck, you can switch the base branch on github and by force pushing at the same time without notifying everyone :P

@WilliButz
Copy link
Member Author

I wouldn't want to risk that ^^

@WilliButz WilliButz deleted the avahi-refactor branch May 28, 2019 12:09
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 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants