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

avahi: set service directory and refactor module #61945

Closed
wants to merge 4 commits into from

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

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

@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants