Re: [Buildroot] [PATCH v1] package/wget: bump version to 1.21.3

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

Hello Peter,

On Fri, 11 Mar 2022 07:50:42 +0100
Peter Seiderer <ps.report@gmx.net> wrote:

> - explicit set some default options (--without-metalink, --enable-opie,
>   --enable-digest, --enable-ntlm, --disable-debug, --disable-valgrind-tests,
>   --disable-assert)
> 
> - add optional libpsl dependency
> 
> - remove legacy --with-libidn option (see [1]), replace with
>   new --enable-iri option in case libiconv and libidn2 are available
> 
> - use explicit --with-libuuid option
> 
> - add optional c-ares dependency
> 
> For details see [2].
> 
> [1] https://git.savannah.gnu.org/cgit/wget.git/commit/configure.ac?id=a24e67e239ef949cc77a4c4e5a0beb703026a296
> [2] https://lists.gnu.org/archive/html/info-gnu/2022-02/msg00017.html
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>

You're mixing up a minor version bump with a significant rework of the
package, which is unrelated to the version bump. This really needs
separate commit. Also, see below.


> +WGET_CONF_OPTS = \
> +	--without-metalink \
> +	--enable-opie \
> +	--enable-digest \
> +	--enable-ntlm \

So opie, digest and ntlm enabled is the default?

> +	--disable-debug \
> +	--disable-valgrind-tests \
> +	--disable-assert
> +
> +ifeq ($(BR2_PACKAGE_LIBPSL),y)
> +WGET_CONF_OPTS += --with-libpsl
> +WGET_DEPENDENCIES += libpsl
> +else
> +WGET_CONF_OPTS += --without-libpsl
> +endif
> +
>  ifeq ($(BR2_PACKAGE_GNUTLS),y)
>  WGET_CONF_OPTS += --with-ssl=gnutls
>  WGET_DEPENDENCIES += gnutls
> @@ -22,19 +38,18 @@ else
>  WGET_CONF_OPTS += --without-ssl
>  endif
>  
> -ifeq ($(BR2_PACKAGE_LIBICONV),y)
> -WGET_DEPENDENCIES += libiconv
> -endif
> -
> -ifeq ($(BR2_PACKAGE_LIBIDN2),y)
> -WGET_CONF_OPTS += --with-libidn
> -WGET_DEPENDENCIES += libidn2
> +ifeq ($(BR2_PACKAGE_LIBICONV)$(BR2_PACKAGE_LIBIDN2),yy)

This is very likely wrong. Indeed BR2_PACKAGE_LIBICONV=y is only
possible when the toolchain does *not* have locale support. When the
toolchain has locale support, iconv support is provided by the
toolchain itself.

Could you revisit this, and also clarify the interaction between
--enable-iri/--disable-iri on one side and
--with-libidn/--without-libidn on the other side?

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