From c68aa2ae8f981bcf20f83299f77b540480e95437 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 13:47:05 -0700 Subject: [PATCH 1/8] Modify patchcheck to accept a --travis flag --- Tools/scripts/patchcheck.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/Tools/scripts/patchcheck.py b/Tools/scripts/patchcheck.py index 33a9fead879325..dff201edcb56bd 100755 --- a/Tools/scripts/patchcheck.py +++ b/Tools/scripts/patchcheck.py @@ -1,4 +1,5 @@ #!/usr/bin/env python3 +"""Check proposed changes for common issues.""" import re import sys import shutil @@ -212,6 +213,28 @@ def regenerated_pyconfig_h_in(file_paths): else: return "not needed" +def travis(pull_request): + if pull_request == 'false': + print('Not a pull request; skipping') + return + base_branch = get_base_branch() + file_paths = changed_files(base_branch) + python_files = [fn for fn in file_paths if fn.endswith('.py')] + c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] + doc_files = [fn for fn in file_paths if fn.startswith('Doc') and + fn.endswith(('.rst', '.inc'))] + print(f'Checking {sum(map(len, [python_files, c_files, doc_files]))} files') + fixed = [] + fixed.extend(normalize_whitespace(python_files)) + fixed.extend(normalize_c_whitespace(c_files)) + fixed.extend(normalize_docs_whitespace(doc_files)) + if not fixed: + print('No whitespace issues found') + else: + print('The following files have whitespace issues:') + for file_path in fixed: + print(' ', file_path) + def main(): base_branch = get_base_branch() file_paths = changed_files(base_branch) @@ -246,4 +269,12 @@ def main(): if __name__ == '__main__': - main() + import argparse + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--travis', + help='Perform pass/fail checks') + args = args = parser.parse_args() + if args.travis: + travis(args.travis) + else: + main() From 7ffc9d7e246cb95cf54121428cab5a861e4fbe9b Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 14:16:21 -0700 Subject: [PATCH 2/8] Support an external Python for checking whitespace --- Tools/scripts/patchcheck.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Tools/scripts/patchcheck.py b/Tools/scripts/patchcheck.py index dff201edcb56bd..03194d4b9c1419 100755 --- a/Tools/scripts/patchcheck.py +++ b/Tools/scripts/patchcheck.py @@ -136,7 +136,7 @@ def report_modified_files(file_paths): return "\n".join(lines) -@status("Fixing whitespace", info=report_modified_files) +@status("Fixing Python file whitespace", info=report_modified_files) def normalize_whitespace(file_paths): """Make sure that the whitespace for .py files have been normalized.""" reindent.makebackup = False # No need to create backups. @@ -223,7 +223,6 @@ def travis(pull_request): c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] doc_files = [fn for fn in file_paths if fn.startswith('Doc') and fn.endswith(('.rst', '.inc'))] - print(f'Checking {sum(map(len, [python_files, c_files, doc_files]))} files') fixed = [] fixed.extend(normalize_whitespace(python_files)) fixed.extend(normalize_c_whitespace(c_files)) @@ -231,9 +230,8 @@ def travis(pull_request): if not fixed: print('No whitespace issues found') else: - print('The following files have whitespace issues:') - for file_path in fixed: - print(' ', file_path) + print(f'Please fix the {len(fixed)} file(s) with whitespace issues') + sys.exit(1) def main(): base_branch = get_base_branch() @@ -273,7 +271,11 @@ def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument('--travis', help='Perform pass/fail checks') - args = args = parser.parse_args() + parser.add_argument('--srcdir', + help='Path to the checkout') + args = parser.parse_args() + if args.srcdir: + SRCDIR = args.srcdir if args.travis: travis(args.travis) else: From b6fbb9a8bd97d29df76065ccd97a44eba14d4424 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 14:17:55 -0700 Subject: [PATCH 3/8] Have Travis check for whitespace issues --- .travis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index d30de21e82197d..c41d0f322e7971 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,6 +32,12 @@ matrix: - brew install openssl xz - export CPPFLAGS="-I$(brew --prefix openssl)/include" - export LDFLAGS="-L$(brew --prefix openssl)/lib" + - os: linux + language: python + python: 3.6 + env: TESTING=whitespace + script: + - python Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST --srcdir . - os: linux language: python # Build the docs against a stable version of Python so code bugs don't hold up doc-related PRs. From 98a11fd1b47752ccda06b646ad6662a0df9fed7b Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 14:40:09 -0700 Subject: [PATCH 4/8] Drop the idea of a srcdir and go back to assuming the built Python is used --- Tools/scripts/patchcheck.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Tools/scripts/patchcheck.py b/Tools/scripts/patchcheck.py index 03194d4b9c1419..c70f072e8a751e 100755 --- a/Tools/scripts/patchcheck.py +++ b/Tools/scripts/patchcheck.py @@ -271,11 +271,7 @@ def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument('--travis', help='Perform pass/fail checks') - parser.add_argument('--srcdir', - help='Path to the checkout') args = parser.parse_args() - if args.srcdir: - SRCDIR = args.srcdir if args.travis: travis(args.travis) else: From 5a7e6e69ab1fdc9ef6fa8ee66e2bab9d841b93df Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 14:40:20 -0700 Subject: [PATCH 5/8] Move the check to be part of the normal build --- .travis.yml | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index c41d0f322e7971..899b7fe3bbbf13 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,12 +32,6 @@ matrix: - brew install openssl xz - export CPPFLAGS="-I$(brew --prefix openssl)/include" - export LDFLAGS="-L$(brew --prefix openssl)/lib" - - os: linux - language: python - python: 3.6 - env: TESTING=whitespace - script: - - python Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST --srcdir . - os: linux language: python # Build the docs against a stable version of Python so code bugs don't hold up doc-related PRs. @@ -95,6 +89,17 @@ before_script: fi script: + # Using the built Python as patchcheck.py is built around the idea of using + # a checkout-build of CPython to know things like what base branch the changes + # should be compared against. + - | + set -e + if [[ $TRAVIS_OS_NAME == 'osx' ]]; then + PYTHON_EXE = "python.exe" + else + PYTHON_EXE = "python" + fi + ./$PYTHON_EXE Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST # `-r -w` implicitly provided through `make buildbottest`. - make buildbottest TESTOPTS="-j4 -uall,-cpu,-tzdata" From 7d5dbeb90f03dda3f024dfbf87ec86163e4b4cd5 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 14:42:38 -0700 Subject: [PATCH 6/8] Just run under Linux --- .travis.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 899b7fe3bbbf13..1234fc69939c9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -92,14 +92,12 @@ script: # Using the built Python as patchcheck.py is built around the idea of using # a checkout-build of CPython to know things like what base branch the changes # should be compared against. + # Only run on Linux as the check only needs to be run once. - | set -e - if [[ $TRAVIS_OS_NAME == 'osx' ]]; then - PYTHON_EXE = "python.exe" - else - PYTHON_EXE = "python" + if [[ $TRAVIS_OS_NAME == 'linux' ]]; then + ./python Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST fi - ./$PYTHON_EXE Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST # `-r -w` implicitly provided through `make buildbottest`. - make buildbottest TESTOPTS="-j4 -uall,-cpu,-tzdata" From 1dc29ec8332315c84e8b1bdad7d990e6c48735fa Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Jun 2017 14:45:57 -0700 Subject: [PATCH 7/8] Tighten up whitespace check shell command --- .travis.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1234fc69939c9b..4be6e4c2154079 100644 --- a/.travis.yml +++ b/.travis.yml @@ -93,11 +93,7 @@ script: # a checkout-build of CPython to know things like what base branch the changes # should be compared against. # Only run on Linux as the check only needs to be run once. - - | - set -e - if [[ $TRAVIS_OS_NAME == 'linux' ]]; then - ./python Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST - fi + - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then ./python Tools/scripts/patchcheck.py --travis $TRAVIS_PULL_REQUEST; fi # `-r -w` implicitly provided through `make buildbottest`. - make buildbottest TESTOPTS="-j4 -uall,-cpu,-tzdata" From acabdec0d855f2f2abeadf20ebfd29bc2efaed6e Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Sat, 24 Jun 2017 16:38:10 -0700 Subject: [PATCH 8/8] Mention `make patchcheck` --- Tools/scripts/patchcheck.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/scripts/patchcheck.py b/Tools/scripts/patchcheck.py index c70f072e8a751e..436b277b6ae866 100755 --- a/Tools/scripts/patchcheck.py +++ b/Tools/scripts/patchcheck.py @@ -231,6 +231,7 @@ def travis(pull_request): print('No whitespace issues found') else: print(f'Please fix the {len(fixed)} file(s) with whitespace issues') + print('(on UNIX you can run `make patchcheck` to make the fixes)') sys.exit(1) def main():