diff mbox series

[ovs-dev,v2] checkpatch: Normalize exit code for Windows

Message ID 20190211155727.12168-1-aserdean@ovn.org
State Superseded
Headers show
Series [ovs-dev,v2] checkpatch: Normalize exit code for Windows | expand

Commit Message

Alin-Gabriel Serdean Feb. 11, 2019, 3:57 p.m. UTC
Using python `sys.exit(-1)` on Windows produces mixed results.
Let's take the following results from different shells:
CMD
>python -c "import sys; sys.exit(-1)" & echo %errorlevel%
1
MSYS
$ python -c "import sys; sys.exit(-1)" && echo $?
0
WSL
$ python -c "import sys; sys.exit(-1)"; echo $?
255

this results in the following tests to fail:
checkpatch

 10: checkpatch - sign-offs                          FAILED (checkpatch.at:32)
 11: checkpatch - parenthesized constructs           FAILED (checkpatch.at:32)
 12: checkpatch - parenthesized constructs - for     FAILED (checkpatch.at:32)
 13: checkpatch - comments                           FAILED (checkpatch.at:32)

because of:
 ./checkpatch.at:32: exit code was 0, expected 255

This patch introduces a positive constant for the default exit code.

Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
---
v2: factor out result from `ovs_checkpatch_print_result as suggested by
    <aconole@redhat.com>
---
 utilities/checkpatch.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Comments

Ben Pfaff Feb. 11, 2019, 4:26 p.m. UTC | #1
On Mon, Feb 11, 2019 at 05:57:27PM +0200, Alin Gabriel Serdean wrote:
> Using python `sys.exit(-1)` on Windows produces mixed results.
> Let's take the following results from different shells:
> CMD
> >python -c "import sys; sys.exit(-1)" & echo %errorlevel%
> 1
> MSYS
> $ python -c "import sys; sys.exit(-1)" && echo $?
> 0
> WSL
> $ python -c "import sys; sys.exit(-1)"; echo $?
> 255
> 
> this results in the following tests to fail:
> checkpatch
> 
>  10: checkpatch - sign-offs                          FAILED (checkpatch.at:32)
>  11: checkpatch - parenthesized constructs           FAILED (checkpatch.at:32)
>  12: checkpatch - parenthesized constructs - for     FAILED (checkpatch.at:32)
>  13: checkpatch - comments                           FAILED (checkpatch.at:32)
> 
> because of:
>  ./checkpatch.at:32: exit code was 0, expected 255
> 
> This patch introduces a positive constant for the default exit code.
> 
> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
> Acked-by: Ben Pfaff <blp@ovn.org>
> Acked-by: Aaron Conole <aconole@redhat.com>

I'm pretty sure that this is just for exit on failure, so probably
EXIT_FAILURE (or similar) would be a better name.

I'm not sure why we're using -1 (or 255).  Most OVS utilities, and most
Unix-like software in general, use exit status 1 on failure.
Alin-Gabriel Serdean March 13, 2019, 2:05 p.m. UTC | #2
> > because of:
> >  ./checkpatch.at:32: exit code was 0, expected 255
> >
> > This patch introduces a positive constant for the default exit code.
> >
> > Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
> > Acked-by: Ben Pfaff <blp@ovn.org>
> > Acked-by: Aaron Conole <aconole@redhat.com>
> 
> I'm pretty sure that this is just for exit on failure, so probably
EXIT_FAILURE
> (or similar) would be a better name.
[Alin Serdean] Updated here: https://patchwork.ozlabs.org/patch/1056056/
> 
> I'm not sure why we're using -1 (or 255).  Most OVS utilities, and most
Unix-
> like software in general, use exit status 1 on failure.
[Alin Serdean] TBH I just followed the exit code from:
https://github.com/openvswitch/ovs/blob/master/tests/checkpatch.at#L33
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 8eab31f60..9b5822324 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -24,6 +24,7 @@  import sys
 RETURN_CHECK_INITIAL_STATE = 0
 RETURN_CHECK_STATE_WITH_RETURN = 1
 RETURN_CHECK_AWAITING_BRACE = 2
+EXIT_CODE = 255
 __errors = 0
 __warnings = 0
 empty_return_check_state = 0
@@ -837,7 +838,7 @@  def ovs_checkpatch_parse(text, filename, author=None, committer=None):
 
     run_file_checks(text)
     if __errors or __warnings:
-        return -1
+        return EXIT_CODE
     return 0
 
 
@@ -863,10 +864,10 @@  Check options:
           % sys.argv[0])
 
 
-def ovs_checkpatch_print_result(result):
+def ovs_checkpatch_print_result():
     global quiet, __warnings, __errors, total_line
 
-    if result < 0:
+    if __errors or __warnings:
         print("Lines checked: %d, Warnings: %d, Errors: %d\n" %
               (total_line, __warnings, __errors))
     elif not quiet:
@@ -886,7 +887,7 @@  def ovs_checkpatch_file(filename):
     result = ovs_checkpatch_parse(part.get_payload(decode=False), filename,
                                   mail.get('Author', mail['From']),
                                   mail['Commit'])
-    ovs_checkpatch_print_result(result)
+    ovs_checkpatch_print_result()
     return result
 
 
@@ -920,7 +921,7 @@  if __name__ == '__main__':
                                        "quiet"])
     except:
         print("Unknown option encountered. Please rerun with -h for help.")
-        sys.exit(-1)
+        sys.exit(EXIT_CODE)
 
     for o, a in optlist:
         if o in ("-h", "--help"):
@@ -946,7 +947,7 @@  if __name__ == '__main__':
             quiet = True
         else:
             print("Unknown option '%s'" % o)
-            sys.exit(-1)
+            sys.exit(EXIT_CODE)
 
     if sys.stdout.isatty():
         colors = True
@@ -972,17 +973,17 @@  Subject: %s
             if not quiet:
                 print('== Checking %s ("%s") ==' % (revision[0:12], name))
             result = ovs_checkpatch_parse(patch, revision)
-            ovs_checkpatch_print_result(result)
+            ovs_checkpatch_print_result()
             if result:
-                status = -1
+                status = EXIT_CODE
         sys.exit(status)
 
     if not args:
         if sys.stdin.isatty():
             usage()
-            sys.exit(-1)
+            sys.exit(EXIT_CODE)
         result = ovs_checkpatch_parse(sys.stdin.read(), '-')
-        ovs_checkpatch_print_result(result)
+        ovs_checkpatch_print_result()
         sys.exit(result)
 
     status = 0
@@ -991,5 +992,5 @@  Subject: %s
             print('== Checking "%s" ==' % filename)
         result = ovs_checkpatch_file(filename)
         if result:
-            status = -1
+            status = EXIT_CODE
     sys.exit(status)