[Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Florian Larysch]

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(+)

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))
-- 
2.29.2

[Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Michael Nosthoff]

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

[Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Arnout Vandecappelle]

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

[Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Arnout Vandecappelle]

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.
> 

Re: [Buildroot] [PATCH 1/1] package/avahi: allow disabling default services

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot