| Message ID | 20201204174507.8483-1-fl@n621.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] package/avahi: allow disabling default services | expand |
Florian, All On Friday, December 04, 2020 18:45 CET, Florian Larysch <fl@n621.de> wrote: > By default, Avahi installs service definitions for SSH and SFTP, but > those might not be present on all systems. Add an option for disabling > them. > I have this removal in all my post-build scripts. So it would be great to have it as an option. I would even suggest to always remove the .service files and make it optional. I think an embedded system should explicitly enable services. > > +config BR2_PACKAGE_AVAHI_DEFAULT_SERVICES > + bool "install default service definitions" > + depends on BR2_PACKAGE_AVAHI_DAEMON > + default y If kept as an Option: default this to n? Regards, Michael
On 15/04/2021 11:34, Michael Nosthoff via buildroot wrote: > Florian, All > > On Friday, December 04, 2020 18:45 CET, Florian Larysch <fl@n621.de> wrote: > >> By default, Avahi installs service definitions for SSH and SFTP, but >> those might not be present on all systems. Add an option for disabling >> them. >> > > I have this removal in all my post-build scripts. So it would be great to have it as an option. > I would even suggest to always remove the .service files and make it optional. I think an embedded system should > explicitly enable services. > >> >> +config BR2_PACKAGE_AVAHI_DEFAULT_SERVICES >> + bool "install default service definitions" >> + depends on BR2_PACKAGE_AVAHI_DAEMON >> + default y > > If kept as an Option: default this to n? To make it easy to upgrade Buildroot, we want as much as possible to make sure that existing configurations still work. If you have an existing configuration that actually relies on these services to be installed, then it would be very annoying if they just disappear under your feet... So in general, we want the default to reflect the old situation. That means that adding an option that removes something will generally have to default y. This policy is far from ideal, because it means people will often have to explicitly unset this option. In many cases, even for existing configs, unsetting it is in fact the right thing to do. So maybe we should consider changing this policy. I think we should only do that if we also add a prominent file in the root, e.g. UPGRADING, that makes explicit note of such changes. Some time ago, I tried to keep something like that going by editing CHANGES, but it didn't amount to much, and that file is so full that it's hard to find the relevant bits. I've added a bunch of random people in Cc whom I think might have a relevant opinion about this. I hope nobody will feel offended because I didn't include them :-) If this idea gains traction, I volunteer to write a skeleton UPGRADING file, and refer to it from the manual. I've been thinking for a long time already that I really should add a section to the manual about what to do when you upgrade Buildroot, and this would be the ideal push for me. Regards, Arnout
Arnout, All, On 2021-04-15 21:02 +0200, Arnout Vandecappelle spake thusly: > On 15/04/2021 11:34, Michael Nosthoff via buildroot wrote: > > On Friday, December 04, 2020 18:45 CET, Florian Larysch <fl@n621.de> wrote: > >> By default, Avahi installs service definitions for SSH and SFTP, but > >> those might not be present on all systems. Add an option for disabling > >> them. [--SNIP--] > >> +config BR2_PACKAGE_AVAHI_DEFAULT_SERVICES > >> + bool "install default service definitions" > >> + depends on BR2_PACKAGE_AVAHI_DAEMON > >> + default y > > If kept as an Option: default this to n? > > To make it easy to upgrade Buildroot, we want as much as possible to make sure > that existing configurations still work. If you have an existing configuration > that actually relies on these services to be installed, then it would be very > annoying if they just disappear under your feet... So in general, we want the > default to reflect the old situation. That means that adding an option that > removes something will generally have to default y. An option that defaults to 'y' will have to do so forever and ever... > This policy is far from ideal, because it means people will often have to > explicitly unset this option. In many cases, even for existing configs, > unsetting it is in fact the right thing to do. And for new, from-scratch configurations, this is not nice either, because most options default to 'n', while only a bunch default to 'y', and this is not very consistent... > So maybe we should consider changing this policy. I would very well be in favour of dropping the policy to be backward-compatible in such a situation. > I think we should only do that if we also add a prominent file in the root, Did you mean: s/root/top-directory of the Buildroot tree/ ? > e.g. UPGRADING, that makes explicit note of such changes. Some time ago, I tried > to keep something like that going by editing CHANGES, but it didn't amount to > much, and that file is so full that it's hard to find the relevant bits. This is going to bit-rot eventually... :-/ > I've added a bunch of random people in Cc whom I think might have a relevant > opinion about this. I hope nobody will feel offended because I didn't include > them :-) > > If this idea gains traction, I volunteer to write a skeleton UPGRADING file, > and refer to it from the manual. I've been thinking for a long time already that > I really should add a section to the manual about what to do when you upgrade > Buildroot, and this would be the ideal push for me. What about the existing section, then: https://buildroot.org/downloads/manual/manual.html#migrating-from-ol-versions (notice the typo in the anchor, giving away who wrote it ;-] And even then, that typo is still fitting my ol' little me... ;-] ) Regards, Yann E. MORIN.
On 15/04/2021 21:47, Yann E. MORIN wrote: > Arnout, All, > > On 2021-04-15 21:02 +0200, Arnout Vandecappelle spake thusly: [snip] >> e.g. UPGRADING, that makes explicit note of such changes. Some time ago, I tried >> to keep something like that going by editing CHANGES, but it didn't amount to >> much, and that file is so full that it's hard to find the relevant bits. > > This is going to bit-rot eventually... :-/ It's similar to Config.in.legacy, for all the things that can't be expressed in Kconfig. So how is it going to bitrot? It could also be put in the manual directly, but I actually think it's harder to find there. > >> I've added a bunch of random people in Cc whom I think might have a relevant >> opinion about this. I hope nobody will feel offended because I didn't include >> them :-) >> >> If this idea gains traction, I volunteer to write a skeleton UPGRADING file, >> and refer to it from the manual. I've been thinking for a long time already that >> I really should add a section to the manual about what to do when you upgrade >> Buildroot, and this would be the ideal push for me. > > What about the existing section, then: > https://buildroot.org/downloads/manual/manual.html#migrating-from-ol-versions Heh, I searched for variations of upgrade and update, not for migrate :-) Goes to show that it's not very discoverable! Regards, Arnout > > (notice the typo in the anchor, giving away who wrote it ;-] And even > then, that typo is still fitting my ol' little me... ;-] ) > > Regards, > Yann E. MORIN. >
Hello Florian, On Fri, 4 Dec 2020 18:45:08 +0100 Florian Larysch <fl@n621.de> wrote: > By default, Avahi installs service definitions for SSH and SFTP, but > those might not be present on all systems. Add an option for disabling > them. > > As there is no way to tell the Avahi package not to install the service > files in the first place, we have to manually remove them from the > target directory. > > Signed-off-by: Florian Larysch <fl@n621.de> > --- > package/avahi/Config.in | 8 ++++++++ > package/avahi/avahi.mk | 9 +++++++++ > 2 files changed, 17 insertions(+) I have finally applied this patch! With some changes: * I made the option default to disabled, following the discussion that took place * Fixed some formatting issues reported by "make check-package" * Used a make foreach loop instead of a shell for loop * Used $(...) to refer to the make variable instead of ${...} Thanks a lot, and sorry for the long delay! Thomas
diff --git a/package/avahi/Config.in b/package/avahi/Config.in index 5e303d044e..4a000dd676 100644 --- a/package/avahi/Config.in +++ b/package/avahi/Config.in @@ -42,6 +42,14 @@ config BR2_PACKAGE_AVAHI_LIBDNSSD_COMPATIBILITY Enable the libdns_sd (Bonjour) compatibility library support for legacy applications. +config BR2_PACKAGE_AVAHI_DEFAULT_SERVICES + bool "install default service definitions" + depends on BR2_PACKAGE_AVAHI_DAEMON + default y + help + Install the SSH/SFTP service definitions included with the Avahi + daemon by default. + endif comment "avahi needs a toolchain w/ threads" diff --git a/package/avahi/avahi.mk b/package/avahi/avahi.mk index eef05f0d2f..b8aea90fcc 100644 --- a/package/avahi/avahi.mk +++ b/package/avahi/avahi.mk @@ -184,4 +184,13 @@ endef AVAHI_POST_INSTALL_STAGING_HOOKS += AVAHI_STAGING_INSTALL_LIBDNSSD_LINK endif +ifeq (${BR2_PACKAGE_AVAHI_DEFAULT_SERVICES},) +define AVAHI_REMOVE_DEFAULT_SERVICES + for service in ssh sftp-ssh; do \ + $(RM) -f $(TARGET_DIR)/etc/avahi/services/$${service}.service; \ + done +endef +AVAHI_POST_INSTALL_TARGET_HOOKS += AVAHI_REMOVE_DEFAULT_SERVICES +endif + $(eval $(autotools-package))
By default, Avahi installs service definitions for SSH and SFTP, but those might not be present on all systems. Add an option for disabling them. As there is no way to tell the Avahi package not to install the service files in the first place, we have to manually remove them from the target directory. Signed-off-by: Florian Larysch <fl@n621.de> --- package/avahi/Config.in | 8 ++++++++ package/avahi/avahi.mk | 9 +++++++++ 2 files changed, 17 insertions(+)