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
Conversation
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`.
@GrahamcOfBorg test avahi |
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. |
There was a problem hiding this 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"; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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)
@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. :) |
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 |
I wouldn't want to risk that ^^ |
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 avahiservice definitions, which are then placed in
/etc/avahi/services
.Manual:
Added a section mentioning the changes for avahi.
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)fixes #60939