| Message ID | 20190205150519.16016-1-aserdean@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] checkpatch: Normalize exit code for Windows | expand |
On Tue, Feb 05, 2019 at 05:05:19PM +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> Curious! I did not know about this particular nonportable behavior. Acked-by: Ben Pfaff <blp@ovn.org>
Alin Gabriel Serdean <aserdean@ovn.org> writes: > 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: Aaron Conole <aconole@redhat.com> With one little fixup: 1. either we should eliminate the result argument to ovs_checkpatch_print_result 2. or the test for the result < 0 needs to change to if result: Otherwise, when an error is introduced, the exit code will be propagated, but the printout will be confusing: 03:30:50 aconole {(0a8c1ec04...)} ~/git/ovs$ git commit -m "dpdk: This is a test Just testing " ERROR: C99 style comment #10 FILE: lib/dpdk.c:17: // BREAK!! Lines checked: 15, no obvious problems found :-) > utilities/checkpatch.py | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py > index 8eab31f60..8b305f2ad 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 > > > @@ -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 > @@ -974,13 +975,13 @@ Subject: %s > result = ovs_checkpatch_parse(patch, revision) > ovs_checkpatch_print_result(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) > sys.exit(result) > @@ -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)
> De la: ovs-dev-bounces@openvswitch.org <ovs-dev- > bounces@openvswitch.org> În numele Aaron Conole > Trimis: Tuesday, February 5, 2019 10:39 PM > Către: Alin Gabriel Serdean <aserdean@ovn.org> > Cc: dev@openvswitch.org > Subiect: Re: [ovs-dev] [PATCH] checkpatch: Normalize exit code for Windows > > Alin Gabriel Serdean <aserdean@ovn.org> writes: > > > 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: Aaron Conole <aconole@redhat.com> > > With one little fixup: > > 1. either we should eliminate the result argument to > ovs_checkpatch_print_result > > 2. or the test for the result < 0 needs to change to if result: > > Otherwise, when an error is introduced, the exit code will be propagated, > but the printout will be confusing: > > 03:30:50 aconole {(0a8c1ec04...)} ~/git/ovs$ git commit -m "dpdk: This is a > test > Just testing > " > ERROR: C99 style comment > #10 FILE: lib/dpdk.c:17: > // BREAK!! > > Lines checked: 15, no obvious problems found > > :-) > [Alin Serdean] Thanks for the feedback. I sent an incremental: https://patchwork.ozlabs.org/patch/1039912/ . Do you mind taking another look?
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 8eab31f60..8b305f2ad 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 @@ -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 @@ -974,13 +975,13 @@ Subject: %s result = ovs_checkpatch_parse(patch, revision) ovs_checkpatch_print_result(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) sys.exit(result) @@ -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)