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

Message ID 20190910105910.32098-1-unixmania@gmail.com
State Rejected
Headers show
Series
  • [v4] package/nfs-utils: enable IPv6 if libtirpc is selected
Related show

Commit Message

Carlos Santos Sept. 10, 2019, 10:59 a.m. UTC
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(-)

Comments

Arnout Vandecappelle Sept. 10, 2019, 12:23 p.m. UTC | #1
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
>
Carlos Santos Sept. 10, 2019, 1:07 p.m. UTC | #2
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
>>
Thomas Petazzoni Sept. 10, 2019, 4:03 p.m. UTC | #3
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
Thomas Petazzoni Sept. 10, 2019, 4:04 p.m. UTC | #4
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
Carlos Santos Sept. 10, 2019, 4:42 p.m. UTC | #5
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.

Patch
diff mbox series

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
 	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
 else
-NFS_UTILS_CONF_OPTS += --disable-tirpc
+NFS_UTILS_CONF_OPTS += --disable-tirpc --disable-ipv6
 endif
 
 define NFS_UTILS_INSTALL_FIXUP