diff mbox

openvpn: adjust iproute2 logic

Message ID 1365094499-21046-1-git-send-email-gustavo@zacarias.com.ar
State Superseded
Headers show

Commit Message

Gustavo Zacarias April 4, 2013, 4:54 p.m. UTC
As reported in bug #6164 when using busybox iproute2 openvpn can fail to
locate the 'ip' utility.
This was correct until busybox 1.21.0 was released since it changed the
location of /bin/ip to /sbin/ip (what iproute2 proper does).
So adjust the ip path accordingly.

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/openvpn/openvpn.mk | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Peter Korsgaard April 4, 2013, 8:51 p.m. UTC | #1
>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

 Gustavo> As reported in bug #6164 when using busybox iproute2 openvpn can fail to
 Gustavo> locate the 'ip' utility.
 Gustavo> This was correct until busybox 1.21.0 was released since it changed the
 Gustavo> location of /bin/ip to /sbin/ip (what iproute2 proper does).
 Gustavo> So adjust the ip path accordingly.

 Gustavo> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
 Gustavo> ---
 Gustavo>  package/openvpn/openvpn.mk | 10 +++++++---
 Gustavo>  1 file changed, 7 insertions(+), 3 deletions(-)

 Gustavo> diff --git a/package/openvpn/openvpn.mk b/package/openvpn/openvpn.mk
 Gustavo> index c911465..28f7238 100644
 Gustavo> --- a/package/openvpn/openvpn.mk
 Gustavo> +++ b/package/openvpn/openvpn.mk
 Gustavo> @@ -17,10 +17,14 @@ OPENVPN_CONF_OPT += --enable-small --disable-plugins \
 Gustavo>  	--disable-debug --disable-eurephia
 Gustavo>  endif
 
 Gustavo> -ifeq ($(BR2_PACKAGE_IPROUTE2),y)
 Gustavo> -OPENVPN_CONF_ENV += IPROUTE=/sbin/ip
 Gustavo> -else
 Gustavo> +# Busybox 1.21+ places the ip applet in the "correct" place
 Gustavo> +# but previous versions didn't.
 Gustavo> +ifneq ($(BR2_PACKAGE_IPROUTE2),y)
 Gustavo> +ifneq ($(BR2_PACKAGE_BUSYBOX_VERSION_1_21_X),y)

We'll most likely forget to update this next time we bump the busybox
version, and not notice it as it's a runtime issue.

Could you invert the logic so you explicitly check for the old busybox
versions instead?
Gustavo Zacarias April 4, 2013, 9:33 p.m. UTC | #2
On 04/04/2013 05:51 PM, Peter Korsgaard wrote:

> We'll most likely forget to update this next time we bump the busybox
> version, and not notice it as it's a runtime issue.
> 
> Could you invert the logic so you explicitly check for the old busybox
> versions instead?

True.
We'll likely forget to remove dead code too later on, but it doesn't hurt :)
Regards.
Peter Korsgaard April 7, 2013, 7:43 a.m. UTC | #3
>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

 >> Could you invert the logic so you explicitly check for the old busybox
 >> versions instead?

 Gustavo> True.

 Gustavo> We'll likely forget to remove dead code too later on, but it
 Gustavo> doesn't hurt :) Regards.

Indeed, that's a lot less harmful.
diff mbox

Patch

diff --git a/package/openvpn/openvpn.mk b/package/openvpn/openvpn.mk
index c911465..28f7238 100644
--- a/package/openvpn/openvpn.mk
+++ b/package/openvpn/openvpn.mk
@@ -17,10 +17,14 @@  OPENVPN_CONF_OPT += --enable-small --disable-plugins \
 	--disable-debug --disable-eurephia
 endif
 
-ifeq ($(BR2_PACKAGE_IPROUTE2),y)
-OPENVPN_CONF_ENV += IPROUTE=/sbin/ip
-else
+# Busybox 1.21+ places the ip applet in the "correct" place
+# but previous versions didn't.
+ifneq ($(BR2_PACKAGE_IPROUTE2),y)
+ifneq ($(BR2_PACKAGE_BUSYBOX_VERSION_1_21_X),y)
 OPENVPN_CONF_ENV += IPROUTE=/bin/ip
+else
+OPENVPN_CONF_ENV += IPROUTE=/sbin/ip
+endif
 endif
 
 ifeq ($(BR2_PACKAGE_OPENVPN_LZO),y)