[dpdk-dev] [PATCH] devtools: check_symbol_change requires bash

[dpdk-dev] [PATCH] devtools: check_symbol_change requires bash

[Stephen Hemminger]

The syntax of check_symbol_change uses some bash syntax.
It does not run correctly on Debian where /bin/sh is not the
same as /bin/bash.

Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 devtools/check-symbol-change.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 40b72073a975..19035a8d40e4 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Neil Horman <nhorman@tuxdriver.com>
 
-- 
2.18.0

Re: [dpdk-dev] [PATCH] devtools: check_symbol_change requires bash

[Mcnamara, John]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Tuesday, July 31, 2018 4:14 PM
> To: nhorman@tuxdriver.com
> Cc: dev@dpdk.org; Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH] devtools: check_symbol_change requires bash
> 
> The syntax of check_symbol_change uses some bash syntax.
> It does not run correctly on Debian where /bin/sh is not the same as
> /bin/bash.

I think this fix may be required for devtools/checkpatches.sh as well
since it seems to use bash specific call to trap().

I saw this in an internal bug report but didn't have a system to check
the fix on. Could you do a quick check.

John
-- 

Re: [dpdk-dev] [PATCH] devtools: check_symbol_change requires bash

[Stephen Hemminger]

On Tue, 31 Jul 2018 16:01:19 +0000
"Mcnamara, John" <john.mcnamara@intel.com> wrote:

> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Tuesday, July 31, 2018 4:14 PM
> > To: nhorman@tuxdriver.com
> > Cc: dev@dpdk.org; Stephen Hemminger <stephen@networkplumber.org>
> > Subject: [dpdk-dev] [PATCH] devtools: check_symbol_change requires bash
> > 
> > The syntax of check_symbol_change uses some bash syntax.
> > It does not run correctly on Debian where /bin/sh is not the same as
> > /bin/bash.  
> 
> I think this fix may be required for devtools/checkpatches.sh as well
> since it seems to use bash specific call to trap().
> 
> I saw this in an internal bug report but didn't have a system to check
> the fix on. Could you do a quick check.

Yes, checkpatches also needs to be bashifyed.

Re: [dpdk-dev] [PATCH] devtools: check_symbol_change requires bash

[Thomas Monjalon]

31/07/2018 17:14, Stephen Hemminger:
> The syntax of check_symbol_change uses some bash syntax.
> It does not run correctly on Debian where /bin/sh is not the
> same as /bin/bash.

I would prefer we fix the root cause.
What is the error?

I see there are some == bashisms. Is there something else?

[dpdk-dev] [PATCH] devtools: fix symbol check for dash

[Thomas Monjalon]

The script check-symbol-change.sh was not running when
/bin/sh redirects to dash.

Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
Cc: nhorman@tuxdriver.com

Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/check-symbol-change.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 40b72073a..daaf45e14 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -4,8 +4,8 @@
 
 build_map_changes()
 {
-	local fname=$1
-	local mapdb=$2
+	local fname="$1"
+	local mapdb="$2"
 
 	cat "$fname" | awk '
 		# Initialize our variables
@@ -80,7 +80,7 @@ build_map_changes()
 
 check_for_rule_violations()
 {
-	local mapdb=$1
+	local mapdb="$1"
 	local mname
 	local symname
 	local secname
@@ -89,10 +89,10 @@ check_for_rule_violations()
 
 	while read mname symname secname ar
 	do
-		if [ "$ar" == "add" ]
+		if [ "$ar" = "add" ]
 		then
 
-			if [ "$secname" == "unknown" ]
+			if [ "$secname" = "unknown" ]
 			then
 				# Just inform the user of this occurrence, but
 				# don't flag it as an error
-- 
2.17.1

Re: [dpdk-dev] [PATCH] devtools: fix symbol check for dash

[Ferruh Yigit]

On 8/5/2018 10:38 AM, Thomas Monjalon wrote:
> The script check-symbol-change.sh was not running when
> /bin/sh redirects to dash.
> 
> Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
> Cc: nhorman@tuxdriver.com
> 
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH] devtools: fix symbol check for dash

[Neil Horman]

On Thu, Aug 09, 2018 at 01:14:23PM +0100, Ferruh Yigit wrote:
> On 8/5/2018 10:38 AM, Thomas Monjalon wrote:
> > The script check-symbol-change.sh was not running when
> > /bin/sh redirects to dash.
> > 
> > Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
> > Cc: nhorman@tuxdriver.com
> > 
> > Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [dpdk-dev] [PATCH] devtools: fix symbol check for dash

[Thomas Monjalon]

09/08/2018 17:14, Neil Horman:
> On Thu, Aug 09, 2018 at 01:14:23PM +0100, Ferruh Yigit wrote:
> > On 8/5/2018 10:38 AM, Thomas Monjalon wrote:
> > > The script check-symbol-change.sh was not running when
> > > /bin/sh redirects to dash.
> > > 
> > > Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
> > > Cc: nhorman@tuxdriver.com
> > > 
> > > Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > 
> > Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Applied