diff mbox series

[v1] devtools: fix error propagation from check-forbidden-tokens.awk

Message ID 1545141782-31841-1-git-send-email-arnon@qwilt.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v1] devtools: fix error propagation from check-forbidden-tokens.awk |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Arnon Warshavsky Dec. 18, 2018, 2:03 p.m. UTC 
  Bugzilla ID: 165
Fixes: 4d4c612e6a30 ("devtools: check wrong svg include in guides")
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>

Explicitly collect the output and result of the
multiple awk script calls, print and return error
if any of them fails
---
 devtools/checkpatches.sh | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

David Marchand Dec. 18, 2018, 2:12 p.m. UTC | #1 
On Tue, Dec 18, 2018 at 3:03 PM Arnon Warshavsky <arnon@qwilt.com> wrote:

> Bugzilla ID: 165
> Fixes: 4d4c612e6a30 ("devtools: check wrong svg include in guides")
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
>
> Explicitly collect the output and result of the
> multiple awk script calls, print and return error
> if any of them fails
> ---
>  devtools/checkpatches.sh | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index ee8debe..df4336c 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -44,22 +44,35 @@ print_usage () {
>  }
>
>  check_forbidden_additions() { # <patch>
> +       res=0
> +
>         # refrain from new additions of rte_panic() and rte_exit()
>         # multiple folders and expressions are separated by spaces
> -       awk -v FOLDERS="lib drivers" \
> +       result=$(awk -v FOLDERS="lib drivers" \
>                 -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
>                 -v RET_ON_FAIL=1 \
>                 -v MESSAGE='Using rte_panic/rte_exit' \
>                 -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
> \
> -               "$1"
> +               "$1")
> +       if [ $? -ne 0 ] ; then
> +               echo $result
> +               res=1
> +       fi
> +
>         # svg figures must be included with wildcard extension
>         # because of png conversion for pdf docs
> -       awk -v FOLDERS='doc' \
> +       result=$(awk -v FOLDERS='doc' \
>                 -v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
>                 -v RET_ON_FAIL=1 \
>                 -v MESSAGE='Using explicit .svg extension instead of .*' \
>                 -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
> \
> -               "$1"
> +               "$1")
> +        if [ $? -ne 0 ] ; then
> +                echo $result
> +                res=1
> +        fi
> +
> +       return $res
>  }
>
>  number=0
> --
> 1.8.3.1
>

How about just this change ?

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index ee8debe..8385384 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -51,7 +51,7 @@ check_forbidden_additions() { # <patch>
                -v RET_ON_FAIL=1 \
                -v MESSAGE='Using rte_panic/rte_exit' \
                -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
-               "$1"
+               "$1" &&
        # svg figures must be included with wildcard extension
        # because of png conversion for pdf docs
        awk -v FOLDERS='doc' \
David Marchand Dec. 18, 2018, 2:16 p.m. UTC | #2 
On Tue, Dec 18, 2018 at 3:12 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Tue, Dec 18, 2018 at 3:03 PM Arnon Warshavsky <arnon@qwilt.com> wrote:
>
>> Bugzilla ID: 165
>> Fixes: 4d4c612e6a30 ("devtools: check wrong svg include in guides")
>> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
>>
>> Explicitly collect the output and result of the
>> multiple awk script calls, print and return error
>> if any of them fails
>> ---
>>  devtools/checkpatches.sh | 21 +++++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>> index ee8debe..df4336c 100755
>> --- a/devtools/checkpatches.sh
>> +++ b/devtools/checkpatches.sh
>> @@ -44,22 +44,35 @@ print_usage () {
>>  }
>>
>>  check_forbidden_additions() { # <patch>
>> +       res=0
>> +
>>         # refrain from new additions of rte_panic() and rte_exit()
>>         # multiple folders and expressions are separated by spaces
>> -       awk -v FOLDERS="lib drivers" \
>> +       result=$(awk -v FOLDERS="lib drivers" \
>>                 -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
>>                 -v RET_ON_FAIL=1 \
>>                 -v MESSAGE='Using rte_panic/rte_exit' \
>>                 -f $(dirname $(readlink -e
>> $0))/check-forbidden-tokens.awk \
>> -               "$1"
>> +               "$1")
>> +       if [ $? -ne 0 ] ; then
>> +               echo $result
>> +               res=1
>> +       fi
>> +
>>         # svg figures must be included with wildcard extension
>>         # because of png conversion for pdf docs
>> -       awk -v FOLDERS='doc' \
>> +       result=$(awk -v FOLDERS='doc' \
>>                 -v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
>>                 -v RET_ON_FAIL=1 \
>>                 -v MESSAGE='Using explicit .svg extension instead of .*' \
>>                 -f $(dirname $(readlink -e
>> $0))/check-forbidden-tokens.awk \
>> -               "$1"
>> +               "$1")
>> +        if [ $? -ne 0 ] ; then
>> +                echo $result
>> +                res=1
>> +        fi
>> +
>> +       return $res
>>  }
>>
>>  number=0
>> --
>> 1.8.3.1
>>
>
> How about just this change ?
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index ee8debe..8385384 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -51,7 +51,7 @@ check_forbidden_additions() { # <patch>
>                 -v RET_ON_FAIL=1 \
>                 -v MESSAGE='Using rte_panic/rte_exit' \
>                 -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
> \
> -               "$1"
> +               "$1" &&
>         # svg figures must be included with wildcard extension
>         # because of png conversion for pdf docs
>         awk -v FOLDERS='doc' \
>

Ah ok, nevermind, two issues.
David Marchand Dec. 18, 2018, 2:27 p.m. UTC | #3 
On Tue, Dec 18, 2018 at 3:16 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Tue, Dec 18, 2018 at 3:12 PM David Marchand <david.marchand@redhat.com>
> wrote:
>
>>
>>
>> On Tue, Dec 18, 2018 at 3:03 PM Arnon Warshavsky <arnon@qwilt.com> wrote:
>>
>>> Bugzilla ID: 165
>>> Fixes: 4d4c612e6a30 ("devtools: check wrong svg include in guides")
>>> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
>>>
>>> Explicitly collect the output and result of the
>>> multiple awk script calls, print and return error
>>> if any of them fails
>>> ---
>>>  devtools/checkpatches.sh | 21 +++++++++++++++++----
>>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>>> index ee8debe..df4336c 100755
>>> --- a/devtools/checkpatches.sh
>>> +++ b/devtools/checkpatches.sh
>>> @@ -44,22 +44,35 @@ print_usage () {
>>>  }
>>>
>>>  check_forbidden_additions() { # <patch>
>>> +       res=0
>>> +
>>>         # refrain from new additions of rte_panic() and rte_exit()
>>>         # multiple folders and expressions are separated by spaces
>>> -       awk -v FOLDERS="lib drivers" \
>>> +       result=$(awk -v FOLDERS="lib drivers" \
>>>                 -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
>>>                 -v RET_ON_FAIL=1 \
>>>                 -v MESSAGE='Using rte_panic/rte_exit' \
>>>                 -f $(dirname $(readlink -e
>>> $0))/check-forbidden-tokens.awk \
>>> -               "$1"
>>> +               "$1")
>>> +       if [ $? -ne 0 ] ; then
>>> +               echo $result
>>> +               res=1
>>> +       fi
>>> +
>>>         # svg figures must be included with wildcard extension
>>>         # because of png conversion for pdf docs
>>> -       awk -v FOLDERS='doc' \
>>> +       result=$(awk -v FOLDERS='doc' \
>>>                 -v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
>>>                 -v RET_ON_FAIL=1 \
>>>                 -v MESSAGE='Using explicit .svg extension instead of .*'
>>> \
>>>                 -f $(dirname $(readlink -e
>>> $0))/check-forbidden-tokens.awk \
>>> -               "$1"
>>> +               "$1")
>>> +        if [ $? -ne 0 ] ; then
>>> +                echo $result
>>> +                res=1
>>> +        fi
>>> +
>>> +       return $res
>>>  }
>>>
>>>  number=0
>>> --
>>> 1.8.3.1
>>>
>>
>> How about just this change ?
>>
>> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
>> index ee8debe..8385384 100755
>> --- a/devtools/checkpatches.sh
>> +++ b/devtools/checkpatches.sh
>> @@ -51,7 +51,7 @@ check_forbidden_additions() { # <patch>
>>                 -v RET_ON_FAIL=1 \
>>                 -v MESSAGE='Using rte_panic/rte_exit' \
>>                 -f $(dirname $(readlink -e
>> $0))/check-forbidden-tokens.awk \
>> -               "$1"
>> +               "$1" &&
>>         # svg figures must be included with wildcard extension
>>         # because of png conversion for pdf docs
>>         awk -v FOLDERS='doc' \
>>
>
> Ah ok, nevermind, two issues.
>

Ok, second attempt, and this time tested :-)

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index ee8debe..915ef67 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -51,7 +51,7 @@ check_forbidden_additions() { # <patch>
                -v RET_ON_FAIL=1 \
                -v MESSAGE='Using rte_panic/rte_exit' \
                -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
-               "$1"
+               "$1" && \
        # svg figures must be included with wildcard extension
        # because of png conversion for pdf docs
        awk -v FOLDERS='doc' \
Thomas Monjalon Dec. 18, 2018, 2:35 p.m. UTC | #4 
18/12/2018 15:27, David Marchand:
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -51,7 +51,7 @@ check_forbidden_additions() { # <patch>
>                 -v RET_ON_FAIL=1 \
>                 -v MESSAGE='Using rte_panic/rte_exit' \
>                 -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
> -               "$1"
> +               "$1" && \
>         # svg figures must be included with wildcard extension
>         # because of png conversion for pdf docs
>         awk -v FOLDERS='doc' \

+1
Arnon Warshavsky Dec. 18, 2018, 2:44 p.m. UTC | #5 
The reason I did not use the && approach is that if both checks have errors,
only the first will be reported and we want all errors to be reported at
once
without discovering them one by one after every fix.
Thomas Monjalon Dec. 18, 2018, 2:48 p.m. UTC | #6 
18/12/2018 15:44, Arnon Warshavsky:
> The reason I did not use the && approach is that if both checks have errors,
> only the first will be reported and we want all errors to be reported at
> once
> without discovering them one by one after every fix.

So please do a simple OR of $?
No need to capture the output.
David Marchand Dec. 18, 2018, 2:49 p.m. UTC | #7 
On Tue, Dec 18, 2018 at 3:45 PM Arnon Warshavsky <arnon@qwilt.com> wrote:

> The reason I did not use the && approach is that if both checks have
> errors,
> only the first will be reported and we want all errors to be reported at
> once
> without discovering them one by one after every fix.
>
>
Ok, then:

 diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index ee8debe..7457f01 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -46,12 +46,13 @@ print_usage () {
 check_forbidden_additions() { # <patch>
        # refrain from new additions of rte_panic() and rte_exit()
        # multiple folders and expressions are separated by spaces
+       ret=0
        awk -v FOLDERS="lib drivers" \
                -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
                -v RET_ON_FAIL=1 \
                -v MESSAGE='Using rte_panic/rte_exit' \
                -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
-               "$1"
+               "$1" || ret=1
        # svg figures must be included with wildcard extension
        # because of png conversion for pdf docs
        awk -v FOLDERS='doc' \
@@ -59,7 +60,9 @@ check_forbidden_additions() { # <patch>
                -v RET_ON_FAIL=1 \
                -v MESSAGE='Using explicit .svg extension instead of .*' \
                -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
-               "$1"
+               "$1" || ret=1
+
+       return $ret
 }

 number=0


No need for all those checks on $? and the output saving.
Arnon Warshavsky Dec. 18, 2018, 2:53 p.m. UTC | #8 
>
> No need for all those checks on $? and the output saving.
>

You blew my cover guys. I am not a shell scripts person :)
You are right. WIll change that
David Marchand Dec. 18, 2018, 2:55 p.m. UTC | #9 
On Tue, Dec 18, 2018 at 3:54 PM Arnon Warshavsky <arnon@qwilt.com> wrote:

> No need for all those checks on $? and the output saving.
>>
>
> You blew my cover guys. I am not a shell scripts person :)
> You are right. WIll change that
>

Eheh, I am looking at shell scripts at the same time... hence the quick
replies ;-)
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index ee8debe..df4336c 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -44,22 +44,35 @@  print_usage () {
 }
 
 check_forbidden_additions() { # <patch>
+	res=0
+
 	# refrain from new additions of rte_panic() and rte_exit()
 	# multiple folders and expressions are separated by spaces
-	awk -v FOLDERS="lib drivers" \
+	result=$(awk -v FOLDERS="lib drivers" \
 		-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
 		-v RET_ON_FAIL=1 \
 		-v MESSAGE='Using rte_panic/rte_exit' \
 		-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
-		"$1"
+		"$1")
+	if [ $? -ne 0 ] ; then
+		echo $result
+		res=1
+	fi
+
 	# svg figures must be included with wildcard extension
 	# because of png conversion for pdf docs
-	awk -v FOLDERS='doc' \
+	result=$(awk -v FOLDERS='doc' \
 		-v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
 		-v RET_ON_FAIL=1 \
 		-v MESSAGE='Using explicit .svg extension instead of .*' \
 		-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
-		"$1"
+		"$1")
+        if [ $? -ne 0 ] ; then
+                echo $result
+                res=1
+        fi
+
+	return $res
 }
 
 number=0