[v4] package/nfs-utils: enable IPv6 if libtirpc is selected

From: Carlos Santos <unixmania@gmail.com>

IPv6 support requires libtirpc, so enable it if libtirpc is selected.

In practice this always enables IPv6, since nfs-utils needs rpcbind,
which in its turn selects libtirpc.

Fixes: https://bugs.busybox.net/show_bug.cgi?id=10806

Signed-off-by: Carlos Santos <unixmania@gmail.com>
---
CC: nathan.renniewaldock@gmail.com
CC: Peter Seiderer <ps.report@gmx.net>
CC: Arnout Vandecappelle <arnout@mind.be>
CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
--
Changes v1->v2
- Select LIBTIRPC unconditionally.
Changes v2->v3
- Do not select libtirpc inconditionally
Changes v3->v4
- Restore v1 commit message
- Add comment about selecting libtirpc to Config.in

So we're back to v1 with a lot of discussion in between. I want the bike
shed purple.
---
 package/nfs-utils/Config.in    | 3 +++
 package/nfs-utils/nfs-utils.mk | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

Oh dear, we're running in circles here...


On 10/09/2019 12:59, unixmania@gmail.com wrote:
> From: Carlos Santos <unixmania@gmail.com>
> 
> IPv6 support requires libtirpc, so enable it if libtirpc is selected.
> 
> In practice this always enables IPv6, since nfs-utils needs rpcbind,
> which in its turn selects libtirpc.

 So somehow, Thomas and I completely missed this bit...

> 
> Fixes: https://bugs.busybox.net/show_bug.cgi?id=10806
> 
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> ---
> CC: nathan.renniewaldock@gmail.com
> CC: Peter Seiderer <ps.report@gmx.net>
> CC: Arnout Vandecappelle <arnout@mind.be>
> CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> --
> Changes v1->v2
> - Select LIBTIRPC unconditionally.
> Changes v2->v3
> - Do not select libtirpc inconditionally
> Changes v3->v4
> - Restore v1 commit message
> - Add comment about selecting libtirpc to Config.in
> 
> So we're back to v1 with a lot of discussion in between. I want the bike
> shed purple.
> ---
>  package/nfs-utils/Config.in    | 3 +++
>  package/nfs-utils/nfs-utils.mk | 6 +++---
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/package/nfs-utils/Config.in b/package/nfs-utils/Config.in
> index 04ea4db3ed..966fe1406a 100644
> --- a/package/nfs-utils/Config.in
> +++ b/package/nfs-utils/Config.in
> @@ -6,11 +6,14 @@ config BR2_PACKAGE_NFS_UTILS
>  	bool "nfs-utils"
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # libtirpc, rpcbind
>  	depends on BR2_USE_MMU # fork()
> +	# Just in case, since rpcbind already selects libtirpc
>  	select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC

 But this doesn't really make sense then... We can just put

	select BR2_PACKAGE_LIBTIRPC

Even the comment above is not really needed - we can get that from the git log.

>  	select BR2_PACKAGE_RPCBIND # runtime
>  	help
>  	  The NFS Linux kernel server.
>  
> +	  For IPv6 support, ensure that libtirpc is selected too.
> +
>  	  http://linux-nfs.org/
>  
>  if BR2_PACKAGE_NFS_UTILS
> diff --git a/package/nfs-utils/nfs-utils.mk b/package/nfs-utils/nfs-utils.mk
> index dc20942f71..3d5f5412cf 100644
> --- a/package/nfs-utils/nfs-utils.mk
> +++ b/package/nfs-utils/nfs-utils.mk
> @@ -19,7 +19,6 @@ NFS_UTILS_CONF_OPTS = \
>  	--disable-nfsv41 \
>  	--disable-gss \
>  	--disable-uuid \
> -	--disable-ipv6 \
>  	--without-tcp-wrappers \
>  	--with-statedir=/run/nfs \
>  	--with-rpcgen=internal
> @@ -51,11 +50,12 @@ else
>  NFS_UTILS_CONF_OPTS += --disable-caps
>  endif
>  
> +# IPv6 requires libtirpc
>  ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
> -NFS_UTILS_CONF_OPTS += --enable-tirpc
> +NFS_UTILS_CONF_OPTS += --enable-tirpc --enable-ipv6
>  NFS_UTILS_DEPENDENCIES += libtirpc

 Due to the select, all of this becomes unconditional.

 In other words, we should:

- select libtirpc unconditionally;
- add libtirpc to _DEPENDENCIES unconditionally;
- add --enable-tirpc --enable-ipv6 to CONF_OPTS unconditionally;
- leave the help text unchanged since libtirpc is anyway selected;
- make it clear in the commit message that libtirpc is anyway selected by
rpcbind, so we can just as well select it unconditionally in nfs-utils.

 Again, I'm very sorry that it takes us five iterations and several months to
come to this conclusion...

 Regards,
 Arnout


>  else
> -NFS_UTILS_CONF_OPTS += --disable-tirpc
> +NFS_UTILS_CONF_OPTS += --disable-tirpc --disable-ipv6
>  endif
>  
>  define NFS_UTILS_INSTALL_FIXUP
>

Answering from my phone and too tired to fight. 🙄

Carlos Santos <unixmania@gmail.com>

Em 10 de set de 2019, à(s) 09:23, Arnout Vandecappelle <arnout@mind.be> escreveu:

> Oh dear, we're running in circles here...
> 
> 
>> On 10/09/2019 12:59, unixmania@gmail.com wrote:
>> From: Carlos Santos <unixmania@gmail.com>
>> 
>> IPv6 support requires libtirpc, so enable it if libtirpc is selected.
>> 
>> In practice this always enables IPv6, since nfs-utils needs rpcbind,
>> which in its turn selects libtirpc.
> 
> So somehow, Thomas and I completely missed this bit...
> 
>> 
>> Fixes: https://bugs.busybox.net/show_bug.cgi?id=10806
>> 
>> Signed-off-by: Carlos Santos <unixmania@gmail.com>
>> ---
>> CC: nathan.renniewaldock@gmail.com
>> CC: Peter Seiderer <ps.report@gmx.net>
>> CC: Arnout Vandecappelle <arnout@mind.be>
>> CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> --
>> Changes v1->v2
>> - Select LIBTIRPC unconditionally.
>> Changes v2->v3
>> - Do not select libtirpc inconditionally
>> Changes v3->v4
>> - Restore v1 commit message
>> - Add comment about selecting libtirpc to Config.in
>> 
>> So we're back to v1 with a lot of discussion in between. I want the bike
>> shed purple.
>> ---
>> package/nfs-utils/Config.in    | 3 +++
>> package/nfs-utils/nfs-utils.mk | 6 +++---
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>> 
>> diff --git a/package/nfs-utils/Config.in b/package/nfs-utils/Config.in
>> index 04ea4db3ed..966fe1406a 100644
>> --- a/package/nfs-utils/Config.in
>> +++ b/package/nfs-utils/Config.in
>> @@ -6,11 +6,14 @@ config BR2_PACKAGE_NFS_UTILS
>>    bool "nfs-utils"
>>    depends on BR2_TOOLCHAIN_HAS_THREADS # libtirpc, rpcbind
>>    depends on BR2_USE_MMU # fork()
>> +    # Just in case, since rpcbind already selects libtirpc
>>    select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC
> 
> But this doesn't really make sense then... We can just put
> 
>    select BR2_PACKAGE_LIBTIRPC
> 
> Even the comment above is not really needed - we can get that from the git log.
> 
>>    select BR2_PACKAGE_RPCBIND # runtime
>>    help
>>      The NFS Linux kernel server.
>> 
>> +      For IPv6 support, ensure that libtirpc is selected too.
>> +
>>      http://linux-nfs.org/
>> 
>> if BR2_PACKAGE_NFS_UTILS
>> diff --git a/package/nfs-utils/nfs-utils.mk b/package/nfs-utils/nfs-utils.mk
>> index dc20942f71..3d5f5412cf 100644
>> --- a/package/nfs-utils/nfs-utils.mk
>> +++ b/package/nfs-utils/nfs-utils.mk
>> @@ -19,7 +19,6 @@ NFS_UTILS_CONF_OPTS = \
>>    --disable-nfsv41 \
>>    --disable-gss \
>>    --disable-uuid \
>> -    --disable-ipv6 \
>>    --without-tcp-wrappers \
>>    --with-statedir=/run/nfs \
>>    --with-rpcgen=internal
>> @@ -51,11 +50,12 @@ else
>> NFS_UTILS_CONF_OPTS += --disable-caps
>> endif
>> 
>> +# IPv6 requires libtirpc
>> ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
>> -NFS_UTILS_CONF_OPTS += --enable-tirpc
>> +NFS_UTILS_CONF_OPTS += --enable-tirpc --enable-ipv6
>> NFS_UTILS_DEPENDENCIES += libtirpc
> 
> Due to the select, all of this becomes unconditional.
> 
> In other words, we should:
> 
> - select libtirpc unconditionally;
> - add libtirpc to _DEPENDENCIES unconditionally;
> - add --enable-tirpc --enable-ipv6 to CONF_OPTS unconditionally;
> - leave the help text unchanged since libtirpc is anyway selected;
> - make it clear in the commit message that libtirpc is anyway selected by
> rpcbind, so we can just as well select it unconditionally in nfs-utils.
> 
> Again, I'm very sorry that it takes us five iterations and several months to
> come to this conclusion...
> 
> Regards,
> Arnout
> 
> 
>> else
>> -NFS_UTILS_CONF_OPTS += --disable-tirpc
>> +NFS_UTILS_CONF_OPTS += --disable-tirpc --disable-ipv6
>> endif
>> 
>> define NFS_UTILS_INSTALL_FIXUP
>>

On Tue, 10 Sep 2019 14:23:15 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>  Oh dear, we're running in circles here...

Absolutely.

>  Due to the select, all of this becomes unconditional.
> 
>  In other words, we should:
> 
> - select libtirpc unconditionally;
> - add libtirpc to _DEPENDENCIES unconditionally;
> - add --enable-tirpc --enable-ipv6 to CONF_OPTS unconditionally;
> - leave the help text unchanged since libtirpc is anyway selected;
> - make it clear in the commit message that libtirpc is anyway selected by
> rpcbind, so we can just as well select it unconditionally in nfs-utils.

Fully agreed.

>  Again, I'm very sorry that it takes us five iterations and several months to
> come to this conclusion...

Yes, indeed. But sometimes that what it takes to finally understand
each other and find what the good solution is.

Thanks,

Thomas

Hello Carlos,

On Tue, 10 Sep 2019 10:07:53 -0300
Carlos Santos <unixmania@gmail.com> wrote:

> Answering from my phone and too tired to fight. 🙄
> 
> Carlos Santos <unixmania@gmail.com>

Unless I missed it, I don't see any specific answer from you on this
e-mail. Was there an issue answering from your phone ?

Also, I don't think there's any need to "fight", we're simply trying to
find what is the best solution in Buildroot, and sometimes it takes a
while. Your contribution and persistence is very appreciated.

Best regards,

Thomas

On Tue, Sep 10, 2019 at 1:04 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Carlos,
>
> On Tue, 10 Sep 2019 10:07:53 -0300
> Carlos Santos <unixmania@gmail.com> wrote:
>
> > Answering from my phone and too tired to fight.
> >
> > Carlos Santos <unixmania@gmail.com>
>
> Unless I missed it, I don't see any specific answer from you on this
> e-mail. Was there an issue answering from your phone ?
>
> Also, I don't think there's any need to "fight", we're simply trying to
> find what is the best solution in Buildroot, and sometimes it takes a
> while. Your contribution and persistence is very appreciated.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

s/fight/talk about/

Just apply v2, fixing the "inconditionally" typo. I can't waste more
time on this, sorry.