Re: [Buildroot] [PATCH 1/1] package/mdnsd: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello,

On Mon, 23 Jan 2023 06:52:58 +0100
Joachim Wiberg <troglobit@gmail.com> wrote:

> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Thanks, I've applied with those changes. See below.

> diff --git a/package/mdnsd/Config.in b/package/mdnsd/Config.in
> new file mode 100644
> index 0000000000..095a924e98
> --- /dev/null
> +++ b/package/mdnsd/Config.in
> @@ -0,0 +1,50 @@
> +config BR2_PACKAGE_MDNSD
> +	bool "mdnsd"
> +	depends on BR2_USE_MMU # fork()
> +	help
> +	  Small mDNS-SD daemon for advertising services and device discovery,
> +	  similar to Avahi and Bonjour.
> +
> +	  By default, mdnsd runs on all interfaces that support multicast.
> +	  It reads services to announce from /etc/mdns.d/*.service, a few
> +	  common services are included below.  To override the defaults,
> +	  e.g., path to services, TTL of multicast frames, or the default
> +	  interface, set MDNSD_ARGS in /etc/default/mdnsd
> +
> +	  Note: currently no NSS integration with GLIBC.

Some of the lines here were too long, causing a check-package warning.
Also the upstream URL of the project was missing as a last line in the
help text.

> +
> +if BR2_PACKAGE_MDNSD
> +
> +config BR2_PACKAGE_MDNSD_MQUERY
> +	bool "mquery"
> +	default n

Not needed, being disabled is the default.

> +	help
> +  	  Scan a LAN for mDNS capable devices, or query specific records,
> +	  similar to the mdns-scan tool.  Useful for verifying multicast
> +	  connectivity or locating neighbors with link-local address.

Some of the lines here were too long, causing a check-package warning.

> +comment "Services to advertise"
> +
> +config BR2_PACKAGE_MDNSD_FTP_SERVICE
> +	bool "FTP service"
> +	default n
> +
> +config BR2_PACKAGE_MDNSD_HTTP_SERVICE
> +	bool "HTTP service"
> +	default n
> +
> +config BR2_PACKAGE_MDNSD_IPP_SERVICE
> +	bool "IPP service"
> +	default n
> +
> +config BR2_PACKAGE_MDNSD_PRINTER_SERVICE
> +	bool "Printer service"
> +	default n

Same comment as above about "default n".

> diff --git a/package/mdnsd/mdnsd.hash b/package/mdnsd/mdnsd.hash
> new file mode 100644
> index 0000000000..2fa7552d85
> --- /dev/null
> +++ b/package/mdnsd/mdnsd.hash
> @@ -0,0 +1,6 @@
> +# Upstream sha256 from GitHub
> +sha256  1af8742ab82a0af88d99d0b15508358ad4305879ab039631bea889138f5c87e8  mdnsd-0.12.tar.gz
> +
> +# Locally computed
> +sha256  2969546227b58ce1b431cc4c36c9a9b45d604e6b94fb8b787ea5d3696f3eee3b  LICENSE
> +

This final empty line was causing a check-package warning.

> diff --git a/package/mdnsd/mdnsd.mk b/package/mdnsd/mdnsd.mk
> new file mode 100644
> index 0000000000..90a4d4c051
> --- /dev/null
> +++ b/package/mdnsd/mdnsd.mk
> @@ -0,0 +1,75 @@
> +################################################################################
> +#
> +# mdnsd
> +#
> +################################################################################
> +
> +MDNSD_VERSION = 0.12
> +MDNSD_SITE = https://github.com/troglobit/mdnsd/releases/download/v$(MDNSD_VERSION)
> +MDNSD_LICENSE = BSD-3-Clause
> +MDNSD_LICENSE_FILES = LICENSE
> +MDNSD_DEPENDENCIES = host-pkgconf
> +
> +ifeq ($(BR2_PACKAGE_MDNSD_MQUERY),y)
> +MDNSD_CONF_OPTS += --with-mquery
> +else
> +MDNSD_CONF_OPTS += --without-mquery
> +endif
> +
> +ifeq ($(BR2_PACKAGE_SYSTEMD),y)
> +MDNSD_DEPENDENCIES += systemd
> +MDNSD_CONF_OPTS += --with-systemd
> +else
> +MDNSD_CONF_OPTS += --without-systemd
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MDNSD_FTP_SERVICE),y)
> +define MDNSD_INSTALL_FTP_SERVICE
> +	$(INSTALL) -D -m 0644 package/mdnsd/ftp.service \
> +		$(TARGET_DIR)/etc/mdns.d/
> +endef
> +MDNSD_POST_INSTALL_TARGET_HOOKS += MDNSD_INSTALL_FTP_SERVICE
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MDNSD_HTTP_SERVICE),y)
> +define MDNSD_INSTALL_HTTP_SERVICE
> +	$(INSTALL) -D -m 0644 package/mdnsd/http.service \
> +		$(TARGET_DIR)/etc/mdns.d/
> +endef
> +MDNSD_POST_INSTALL_TARGET_HOOKS += MDNSD_INSTALL_HTTP_SERVICE
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MDNSD_IPP_SERVICE),y)
> +define MDNSD_INSTALL_IPP_SERVICE
> +	$(INSTALL) -D -m 0644 package/mdnsd/ipp.service \
> +		$(TARGET_DIR)/etc/mdns.d/
> +endef
> +MDNSD_POST_INSTALL_TARGET_HOOKS += MDNSD_INSTALL_IPP_SERVICE
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MDNSD_PRINTER_SERVICE),y)
> +define MDNSD_INSTALL_PRINTER_SERVICE
> +	$(INSTALL) -D -m 0644 package/mdnsd/printer.service \
> +		$(TARGET_DIR)/etc/mdns.d/
> +endef
> +MDNSD_POST_INSTALL_TARGET_HOOKS += MDNSD_INSTALL_PRINTER_SERVICE
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MDNSD_SSH_SERVICE),y)
> +define MDNSD_INSTALL_SSH_SERVICE
> +	$(INSTALL) -D -m 0644 package/mdnsd/ssh.service \
> +		$(TARGET_DIR)/etc/mdns.d/
> +endef
> +MDNSD_POST_INSTALL_TARGET_HOOKS += MDNSD_INSTALL_SSH_SERVICE
> +endif

All of this service handling stuff was very verbose. I've refactored it
like this:

MDNSD_SERVICES_$(BR2_PACKAGE_MDNSD_FTP_SERVICE) += ftp
MDNSD_SERVICES_$(BR2_PACKAGE_MDNSD_HTTP_SERVICE) += http
MDNSD_SERVICES_$(BR2_PACKAGE_MDNSD_IPP_SERVICE) += ipp
MDNSD_SERVICES_$(BR2_PACKAGE_MDNSD_PRINTER_SERVICE) += printer
MDNSD_SERVICES_$(BR2_PACKAGE_MDNSD_SSH_SERVICE) += ssh

define MDNSD_INSTALL_SERVICES
        $(foreach service,$(MDNSD_SERVICES_y),\
                $(INSTALL) -D -m 0644 package/mdnsd/$(service).service \
                        $(TARGET_DIR)/etc/mdns.d/$(service).service
        )
endef
MDNSD_POST_INSTALL_TARGET_HOOKS += MDNSD_INSTALL_SERVICES

> +define MDNSD_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 755 -D package/mdnsd/S50mdnsd $(TARGET_DIR)/etc/init.d/

You need a full destination path here.

As said above: I fixed those various small things when applying.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot