[OpenWrt-Devel] [PATCH] dnsmasq: allow de-selecting features from -full variant.

[Frank Schäfer]

Hi yousong,

thanks for the patch.
A few issues/comments:

Am 17.12.2014 um 02:21 schrieb Yousong Zhou:
> Signed-off-by: Yousong Zhou <yszhou4tech at gmail.com>
> ---
>  package/network/services/dnsmasq/Makefile          |   27 ++++++++++++++++++--
>  .../network/services/dnsmasq/files/dnsmasq.init    |    5 ++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/package/network/services/dnsmasq/Makefile b/package/network/services/dnsmasq/Makefile
> index a530225..2da593d 100644
> --- a/package/network/services/dnsmasq/Makefile
> +++ b/package/network/services/dnsmasq/Makefile
> @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
>  
>  PKG_NAME:=dnsmasq
>  PKG_VERSION:=2.72
> -PKG_RELEASE:=1
> +PKG_RELEASE:=2
>  
>  PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
>  PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq
> @@ -72,6 +72,26 @@ define Package/dnsmasq/conffiles
>  /etc/dnsmasq.conf
>  endef
>  
> +define Package/dnsmasq/config/Default
> +  if PACKAGE_$(1)-$(2)
> +  config PACKAGE_dnsmasq_$(2)_dhcpv6
> +    bool "Build with DHCPv6 support."
> +    default y
This config needs to depend on IPV6

> +  config PACKAGE_dnsmasq_$(2)_dnssec
> +    bool "Build with DNSSEC support."
> +    default y
This config needs to select PACKAGE_libnettle

> +  config PACKAGE_dnsmasq_$(2)_auth
> +    bool "Build with the facility to act as an authoritative DNS server."
> +    default y
> +  config PACKAGE_dnsmasq_$(2)_ipset
> +    bool "Build with ipset support."
> +    select PACKAGE_kmod-ipt-ipset
> +    default y
> +  endif
> +endef
It would be nice if this would appear in a submenu and only if the
dnsmasq package is enabled.

> +
> +Package/dnsmasq-full/config=$(call Package/dnsmasq/config/Default,dnsmasq,full)
> +
>  Package/dnsmasq-dhcpv6/conffiles = $(Package/dnsmasq/conffiles)
>  Package/dnsmasq-full/conffiles = $(Package/dnsmasq/conffiles)
>  
> @@ -85,7 +105,10 @@ ifeq ($(BUILD_VARIANT),nodhcpv6)
>  endif
>  
>  ifeq ($(BUILD_VARIANT),full)
> -	COPTS += -DHAVE_DNSSEC
> +	COPTS += $(if $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dhcpv6),,-DNO_DHCP6) \
> +		$(if $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dnssec),-DHAVE_DNSSEC) \
> +		$(if $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_auth),,-DNO_AUTH) \
> +		$(if $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_ipset),,-DNO_IPSET)
>  	COPTS += $(if $(CONFIG_LIBNETTLE_MINI),-DNO_GMP,)
>  else
>  	COPTS += -DNO_AUTH -DNO_IPSET
The installation section Package/dnsmasq-full/install should be
modified, too:
if DNSSEC is not enabled, there's no need to install trust-anchors.conf

---

The patch leaves the main problem unsolved:
you still have to enable the whole IPv6 stuff for DNS Auth / DNSSEC /
IPSET support.

I personally have no problems with migrating from package variants to
the "configure at build time" approach.
But I don't think they should be mixed together.
The "dnsmasq-full" package with only dhcpv6 enabled is exactly the same
as the "dnsmasq-dhcpv6" package variant.
And as soon as you deselect one of the config options, "dnsmasq-full"
actually isn't a "full" package anymore, right ? ;-)
IMHO too much inconsistencies.
If we go this way, I would prefer having just a single (fully
configurable) dnsmasq package.

I'll send a patch later this evening.

Regards,
Frank

> diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init
> index 942acd7..209952b 100644
> --- a/package/network/services/dnsmasq/files/dnsmasq.init
> +++ b/package/network/services/dnsmasq/files/dnsmasq.init
> @@ -85,6 +85,10 @@ append_address() {
>  	xappend "--address=$1"
>  }
>  
> +append_ipset() {
> +	xappend "--ipset=$1"
> +}
> +
>  append_interface() {
>  	local ifname=$(uci_get_state network "$1" ifname "$1")
>  	xappend "--interface=$ifname"
> @@ -135,6 +139,7 @@ dnsmasq() {
>  	append_parm "$cfg" "local" "--server"
>  	config_list_foreach "$cfg" "server" append_server
>  	config_list_foreach "$cfg" "address" append_address
> +	config_list_foreach "$cfg" "ipset" append_ipset
>  	config_list_foreach "$cfg" "interface" append_interface
>  	config_list_foreach "$cfg" "notinterface" append_notinterface
>  	config_list_foreach "$cfg" "addnhosts" append_addnhosts
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel